Bug 1493149 - [geckodriver] "temporary" flag in AddonInstallParameters has to be optional. r=ato
authorHenrik Skupin <mail@hskupin.info>
Thu, 27 Sep 2018 08:37:29 +0000
changeset 438444 6155b2f3b9c12681fc4767cc5961ec2c1c199e6b
parent 438443 69fe889aa92a7dd642a586ebb53edb51b2db1428
child 438453 c58aecd60cf303a396d8d7fea6225cbf50f64a44
push id70017
push userhskupin@mozilla.com
push dateThu, 27 Sep 2018 09:14:27 +0000
treeherderautoland@6155b2f3b9c1 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersato
bugs1493149
milestone64.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1493149 - [geckodriver] "temporary" flag in AddonInstallParameters has to be optional. r=ato With the Serde refactoring we have made this flag mandatory. This patch reverts that change, and allows it to be skipped. Differential Revision: https://phabricator.services.mozilla.com/D7050
testing/geckodriver/src/command.rs
testing/geckodriver/src/marionette.rs
--- a/testing/geckodriver/src/command.rs
+++ b/testing/geckodriver/src/command.rs
@@ -134,36 +134,36 @@ impl WebDriverExtensionCommand for Gecko
             &GeckoExtensionCommand::XblAnonymousChildren(_) => None,
         }
     }
 }
 
 #[derive(Clone, Debug, PartialEq, Serialize)]
 pub struct AddonInstallParameters {
     pub path: String,
-    pub temporary: bool,
+    pub temporary: Option<bool>,
 }
 
 impl<'de> Deserialize<'de> for AddonInstallParameters {
     fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
     where
         D: Deserializer<'de>,
     {
         #[derive(Debug, Deserialize)]
         #[serde(deny_unknown_fields)]
         struct Base64 {
             addon: String,
-            temporary: bool,
+            temporary: Option<bool>,
         };
 
         #[derive(Debug, Deserialize)]
         #[serde(deny_unknown_fields)]
         struct Path {
             path: String,
-            temporary: bool,
+            temporary: Option<bool>,
         };
 
         #[derive(Debug, Deserialize)]
         #[serde(untagged)]
         enum Helper {
             Base64(Base64),
             Path(Path),
         };
@@ -249,17 +249,28 @@ mod tests {
         assert!(serde_json::from_str::<AddonInstallParameters>(&json).is_err());
     }
 
     #[test]
     fn test_json_addon_install_parameters_with_path() {
         let json = r#"{"path": "/path/to.xpi", "temporary": true}"#;
         let data = AddonInstallParameters {
             path: "/path/to.xpi".to_string(),
-            temporary: true,
+            temporary: Some(true),
+        };
+
+        check_deserialize(&json, &data);
+    }
+
+    #[test]
+    fn test_json_addon_install_parameters_with_path_only() {
+        let json = r#"{"path": "/path/to.xpi"}"#;
+        let data = AddonInstallParameters {
+            path: "/path/to.xpi".to_string(),
+            temporary: None,
         };
 
         check_deserialize(&json, &data);
     }
 
     #[test]
     fn test_json_addon_install_parameters_with_path_invalid_type() {
         let json = r#"{"path": true, "temporary": true}"#;
@@ -270,28 +281,33 @@ mod tests {
     #[test]
     fn test_json_addon_install_parameters_with_path_and_temporary_invalid_type() {
         let json = r#"{"path": "/path/to.xpi", "temporary": "foo"}"#;
 
         assert!(serde_json::from_str::<AddonInstallParameters>(&json).is_err());
     }
 
     #[test]
-    fn test_json_addon_install_parameters_with_path_only() {
-        let json = r#"{"path": "/path/to.xpi"}"#;
-
-        assert!(serde_json::from_str::<AddonInstallParameters>(&json).is_err());
-    }
-
-    #[test]
     fn test_json_addon_install_parameters_with_addon() {
         let json = r#"{"addon": "aGVsbG8=", "temporary": true}"#;
         let data = serde_json::from_str::<AddonInstallParameters>(&json).unwrap();
 
-        assert_eq!(data.temporary, true);
+        assert_eq!(data.temporary, Some(true));
+        let mut file = File::open(data.path).unwrap();
+        let mut contents = String::new();
+        file.read_to_string(&mut contents).unwrap();
+        assert_eq!(contents, "hello");
+    }
+
+    #[test]
+    fn test_json_addon_install_parameters_with_addon_only() {
+        let json = r#"{"addon": "aGVsbG8="}"#;
+        let data = serde_json::from_str::<AddonInstallParameters>(&json).unwrap();
+
+        assert_eq!(data.temporary, None);
         let mut file = File::open(data.path).unwrap();
         let mut contents = String::new();
         file.read_to_string(&mut contents).unwrap();
         assert_eq!(contents, "hello");
     }
 
     #[test]
     fn test_json_addon_install_parameters_with_addon_invalid_type() {
@@ -303,23 +319,16 @@ mod tests {
     #[test]
     fn test_json_addon_install_parameters_with_addon_and_temporary_invalid_type() {
         let json = r#"{"addon": "aGVsbG8=", "temporary": "foo"}"#;
 
         assert!(serde_json::from_str::<AddonInstallParameters>(&json).is_err());
     }
 
     #[test]
-    fn test_json_addon_install_parameters_with_addon_only() {
-        let json = r#"{"addon": "aGVsbG8="}"#;
-
-        assert!(serde_json::from_str::<AddonInstallParameters>(&json).is_err());
-    }
-
-    #[test]
     fn test_json_install_parameters_with_temporary_only() {
         let json = r#"{"temporary": true}"#;
 
         assert!(serde_json::from_str::<AddonInstallParameters>(&json).is_err());
     }
 
     #[test]
     fn test_json_addon_install_parameters_with_both_path_and_addon() {
--- a/testing/geckodriver/src/marionette.rs
+++ b/testing/geckodriver/src/marionette.rs
@@ -1287,18 +1287,20 @@ impl MarionetteConnection {
 
 trait ToMarionette {
     fn to_marionette(&self) -> WebDriverResult<Map<String, Value>>;
 }
 
 impl ToMarionette for AddonInstallParameters {
     fn to_marionette(&self) -> WebDriverResult<Map<String, Value>> {
         let mut data = Map::new();
-        data.insert("path".to_string(), Value::String(self.path.clone()));
-        data.insert("temporary".to_string(), Value::Bool(self.temporary));
+        data.insert("path".to_string(), serde_json::to_value(&self.path)?);
+        if self.temporary.is_some() {
+            data.insert("temporary".to_string(), serde_json::to_value(&self.temporary)?);
+        }
         Ok(data)
     }
 }
 
 impl ToMarionette for AddonUninstallParameters {
     fn to_marionette(&self) -> WebDriverResult<Map<String, Value>> {
         let mut data = Map::new();
         data.insert("id".to_string(), Value::String(self.id.clone()));