Bug 1482829 - Convert logging::Level into mozprofile::preferences::Pref. r=whimboo
authorAndreas Tolfsen <ato@sny.no>
Tue, 14 Aug 2018 15:38:43 +0100
changeset 487239 2db292e66641b911f3663e5a29fada5da5c62f16
parent 487238 845be8b8ba0574ad1252307d9d893759f5999c1d
child 487240 131559bd826a1ea9c58f63de32a62eb67d1000bf
push id9719
push userffxbld-merge
push dateFri, 24 Aug 2018 17:49:46 +0000
treeherdermozilla-beta@719ec98fba77 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerswhimboo
bugs1482829
milestone63.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 1482829 - Convert logging::Level into mozprofile::preferences::Pref. r=whimboo In order to facilitate the use of Log.jsm's Logger#manageLevelFromPref, geckodriver needs to ensure that the input value for the marionette.log.level preference conforms to the variants in the Log.Level enum. This patch implements the Into<T> conversion from geckodriver's logging::Level into mozprofile::preferences::Pref by way of a new function to_gecko(), that ensures the preference value is correctly formatted. Logger#manageLevelFromPref expects a string value such as "Info", which exactly matches Log.Level's own properties. It is in other words case sensitive, and this ensures that Marionette no longer has to case convert the input data.
testing/geckodriver/src/logging.rs
testing/geckodriver/src/marionette.rs
--- a/testing/geckodriver/src/logging.rs
+++ b/testing/geckodriver/src/logging.rs
@@ -22,24 +22,26 @@
 //! [`error!`]: https://docs.rs/log/newest/log/macro.error.html
 //! [`warn!`]: https://docs.rs/log/newest/log/macro.warn.html
 //! [`info!`]: https://docs.rs/log/newest/log/macro.info.html
 //! [`debug!`]: https://docs.rs/log/newest/log/macro.debug.html
 //! [`trace!`]: https://docs.rs/log/newest/log/macro.trace.html
 //! [`init`]: fn.init.html
 //! [`init_with_level`]: fn.init_with_level.html
 
-use chrono;
-use log;
 use std::fmt;
 use std::io;
 use std::io::Write;
 use std::str;
 use std::sync::atomic::{AtomicUsize, Ordering, ATOMIC_USIZE_INIT};
 
+use chrono;
+use log;
+use mozprofile::preferences::Pref;
+
 static MAX_LOG_LEVEL: AtomicUsize = ATOMIC_USIZE_INIT;
 const LOGGED_TARGETS: &'static [&'static str] = &[
     "geckodriver",
     "mozprofile",
     "mozrunner",
     "mozversion",
     "webdriver",
 ];
@@ -117,16 +119,31 @@ impl Into<log::Level> for Level {
             Warn => log::Level::Warn,
             Info => log::Level::Info,
             Config | Debug => log::Level::Debug,
             Trace => log::Level::Trace,
         }
     }
 }
 
+impl Into<Pref> for Level {
+    fn into(self) -> Pref {
+        use self::Level::*;
+        Pref::new(match self {
+            Fatal => "Fatal",
+            Error => "Error",
+            Warn => "Warn",
+            Info => "Info",
+            Config => "Config",
+            Debug => "Debug",
+            Trace => "Trace",
+        })
+    }
+}
+
 impl From<log::Level> for Level {
     fn from(log_level: log::Level) -> Level {
         use log::Level::*;
         match log_level {
             Error => Level::Error,
             Warn => Level::Warn,
             Info => Level::Info,
             Debug => Level::Debug,
@@ -190,20 +207,23 @@ pub fn set_max_level(level: Level) {
 /// Produces a 13-digit Unix Epoch timestamp similar to Gecko.
 fn format_ts(ts: chrono::DateTime<chrono::Local>) -> String {
     format!("{}{:03}", ts.timestamp(), ts.timestamp_subsec_millis())
 }
 
 #[cfg(test)]
 mod tests {
     use super::{format_ts, init_with_level, max_level, set_max_level, Level};
+
+    use std::str::FromStr;
+    use std::sync::Mutex;
+
     use chrono;
     use log;
-    use std::str::FromStr;
-    use std::sync::Mutex;
+    use mozprofile::preferences::{Pref, PrefValue};
 
     lazy_static! {
         static ref LEVEL_MUTEX: Mutex<()> = Mutex::new(());
     }
 
     #[test]
     fn test_level_repr() {
         assert_eq!(Level::Fatal as usize, 70);
@@ -242,16 +262,34 @@ mod tests {
         assert_eq!(Into::<log::Level>::into(Level::Warn), log::Level::Warn);
         assert_eq!(Into::<log::Level>::into(Level::Info), log::Level::Info);
         assert_eq!(Into::<log::Level>::into(Level::Config), log::Level::Debug);
         assert_eq!(Into::<log::Level>::into(Level::Debug), log::Level::Debug);
         assert_eq!(Into::<log::Level>::into(Level::Trace), log::Level::Trace);
     }
 
     #[test]
+    fn test_level_into_pref() {
+        let tests = [
+            (Level::Fatal, "Fatal"),
+            (Level::Error, "Error"),
+            (Level::Warn, "Warn"),
+            (Level::Info, "Info"),
+            (Level::Config, "Config"),
+            (Level::Debug, "Debug"),
+            (Level::Trace, "Trace"),
+        ];
+
+        for &(lvl, s) in tests.iter() {
+            let expected = Pref { value: PrefValue::String(s.to_string()), sticky: false };
+            assert_eq!(Into::<Pref>::into(lvl), expected);
+        }
+    }
+
+    #[test]
     fn test_level_from_str() {
         assert_eq!(Level::from_str("fatal"), Ok(Level::Fatal));
         assert_eq!(Level::from_str("error"), Ok(Level::Error));
         assert_eq!(Level::from_str("warn"), Ok(Level::Warn));
         assert_eq!(Level::from_str("info"), Ok(Level::Info));
         assert_eq!(Level::from_str("config"), Ok(Level::Config));
         assert_eq!(Level::from_str("debug"), Ok(Level::Debug));
         assert_eq!(Level::from_str("trace"), Ok(Level::Trace));
--- a/testing/geckodriver/src/marionette.rs
+++ b/testing/geckodriver/src/marionette.rs
@@ -481,22 +481,29 @@ impl MarionetteHandler {
                                     format!("Failed to start browser {}: {}",
                                             binary.display(), e))
             })?;
         self.browser = Some(browser_proc);
 
         Ok(())
     }
 
-    pub fn set_prefs(&self, port: u16, profile: &mut Profile, custom_profile: bool,
-                     extra_prefs: Vec<(String, Pref)>)
-                 -> WebDriverResult<()> {
-        let prefs = try!(profile.user_prefs()
-                         .map_err(|_| WebDriverError::new(ErrorStatus::UnknownError,
-                                                          "Unable to read profile preferences file")));
+    pub fn set_prefs(
+        &self,
+        port: u16,
+        profile: &mut Profile,
+        custom_profile: bool,
+        extra_prefs: Vec<(String, Pref)>,
+    ) -> WebDriverResult<()> {
+        let prefs = profile.user_prefs().map_err(|_| {
+            WebDriverError::new(
+                ErrorStatus::UnknownError,
+                "Unable to read profile preferences file",
+            )
+        })?;
 
         for &(ref name, ref value) in prefs::DEFAULT.iter() {
             if !custom_profile || !prefs.contains_key(name) {
                 prefs.insert((*name).clone(), (*value).clone());
             }
         }
 
         prefs.insert_slice(&extra_prefs[..]);
@@ -504,21 +511,22 @@ impl MarionetteHandler {
         if self.settings.jsdebugger {
             prefs.insert("devtools.browsertoolbox.panel", Pref::new("jsdebugger".to_owned()));
             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(|_| WebDriverError::new(ErrorStatus::UnknownError,
-                                                      "Unable to write Firefox profile"))
+        prefs.write().map_err(|_| {
+            WebDriverError::new(ErrorStatus::UnknownError, "Unable to write Firefox profile")
+        })
     }
 }
 
 impl WebDriverHandler<GeckoExtensionRoute> for MarionetteHandler {
     fn handle_command(&mut self, _: &Option<Session>,
                       msg: WebDriverMessage<GeckoExtensionRoute>) -> WebDriverResult<WebDriverResponse> {
         let mut resolved_capabilities = None;
         {