bug 1494613: geckodriver: convert logging::Level to Pref r=whimboo
authorAndreas Tolfsen <ato@sny.no>
Thu, 27 Sep 2018 13:35:12 +0000
changeset 494627 0fcb85b4c2f2580df3de46af1c10b260dd156a44
parent 494626 baff8a950809384341075f414bbeddff204b731a
child 494628 3632445ae3db388e7b214b83acec5a042391a7bb
child 494646 ab5269bd3c218e44e98c31edee99cdd185d0beda
push id9984
push userffxbld-merge
push dateMon, 15 Oct 2018 21:07:35 +0000
treeherdermozilla-beta@183d27ea8570 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerswhimboo
bugs1494613, 12166, 1482829, 1396821
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 1494613: geckodriver: convert logging::Level to Pref r=whimboo The patch c1df1c2e46f6 contained a faulty rebase where the coercion of logging::max_level() changed from the Pref type to a string. The string representation of geckodriver::logging::Level is given in upper-case, e.g. "INFO", and the Pref representation is given as "Info" to be compatible with managing the log level from Log.jsm in Gecko. This inadvertently caused https://github.com/web-platform-tests/wpt/issues/12166 to reappear in almost the same way: before the problem was that Marionette’s frame script always included all log level entries. This was fixed with https://bugzilla.mozilla.org/show_bug.cgi?1482829, but then https://bugzilla.mozilla.org/show_bug.cgi?id=1396821 broke it so that log entries also from chrome space appeared.
testing/geckodriver/src/marionette.rs
--- a/testing/geckodriver/src/marionette.rs
+++ b/testing/geckodriver/src/marionette.rs
@@ -210,20 +210,17 @@ impl MarionetteHandler {
         if self.settings.jsdebugger {
             prefs.insert("devtools.browsertoolbox.panel", Pref::new("jsdebugger"));
             prefs.insert("devtools.debugger.remote-enabled", Pref::new(true));
             prefs.insert("devtools.chrome.enabled", Pref::new(true));
             prefs.insert("devtools.debugger.prompt-connection", Pref::new(false));
             prefs.insert("marionette.debugging.clicktostart", Pref::new(true));
         }
 
-        prefs.insert(
-            "marionette.log.level",
-            Pref::new(logging::max_level().to_string()),
-        );
+        prefs.insert("marionette.log.level", logging::max_level().into());
         prefs.insert("marionette.port", Pref::new(port));
 
         prefs.write().map_err(|e| {
             WebDriverError::new(
                 ErrorStatus::UnknownError,
                 format!("Unable to write Firefox profile: {}", e),
             )
         })
@@ -1481,9 +1478,37 @@ impl ToMarionette for WindowRectParamete
             serde_json::to_value(self)?.as_object(),
             ErrorStatus::UnknownError,
             "Expected an object"
         ).clone())
     }
 }
 
 #[cfg(test)]
-mod tests {}
+mod tests {
+    use super::{MarionetteHandler, MarionetteSettings};
+    use mozprofile::preferences::PrefValue;
+    use mozprofile::profile::Profile;
+
+    // This is not a pretty test, mostly due to the nature of
+    // mozprofile's and MarionetteHandler's APIs, but we have had
+    // several regressions related to marionette.log.level.
+    #[test]
+    fn test_marionette_log_level() {
+        let mut profile = Profile::new(None).unwrap();
+        let handler = MarionetteHandler::new(MarionetteSettings::default());
+        handler.set_prefs(2828, &mut profile, false, vec![]).ok();
+        let user_prefs = profile.user_prefs().unwrap();
+
+        let pref = user_prefs.get("marionette.log.level").unwrap();
+        let value = match pref.value {
+            PrefValue::String(ref s) => s,
+            _ => panic!(),
+        };
+        for (i, ch) in value.chars().enumerate() {
+            if i == 0 {
+                assert!(ch.is_uppercase());
+            } else {
+                assert!(ch.is_lowercase());
+            }
+        }
+    }
+}