Bug 1482829 - Track Marionette logger verbosity with Log#manageLevelFromPref. r=whimboo
authorAndreas Tolfsen <ato@sny.no>
Mon, 13 Aug 2018 13:59:39 +0100
changeset 487240 131559bd826a1ea9c58f63de32a62eb67d1000bf
parent 487239 2db292e66641b911f3663e5a29fada5da5c62f16
child 487241 f334098ab7bc805488005b3f09537e59a953cccb
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 - Track Marionette logger verbosity with Log#manageLevelFromPref. r=whimboo This patch adopts Logger#managerLevelFromPref from Log.jsm to set and keep track of the Marionette logger's verbosity. This has the advantage that we do not have to roll separate implementations of Log for the child- and parent processes. It also has the upside that the log level will be reflected when changed at runtime through the use of an observer.
layout/tools/reftest/runreftest.py
testing/geckodriver/README.md
testing/marionette/client/marionette_driver/geckoinstance.py
testing/marionette/doc/Prefs.md
testing/marionette/log.js
testing/marionette/prefs.js
testing/marionette/prefs/marionette.js
testing/mochitest/runtests.py
--- a/layout/tools/reftest/runreftest.py
+++ b/layout/tools/reftest/runreftest.py
@@ -391,17 +391,17 @@ class RefTest(object):
 
         if options.marionette:
             # options.marionette can specify host:port
             port = options.marionette.split(":")[1]
             prefs["marionette.port"] = int(port)
 
         # Enable tracing output for detailed failures in case of
         # failing connection attempts, and hangs (bug 1397201)
-        prefs["marionette.log.level"] = "TRACE"
+        prefs["marionette.log.level"] = "Trace"
 
         # Third, set preferences passed in via the command line.
         for v in options.extraPrefs:
             thispref = v.split('=')
             if len(thispref) < 2:
                 print "Error: syntax error in --setpref=" + v
                 sys.exit(1)
             prefs[thispref[0]] = thispref[1].strip()
--- a/testing/geckodriver/README.md
+++ b/testing/geckodriver/README.md
@@ -342,17 +342,17 @@ it will be removed once the interactabil
   <td><code>level</code>
   <td>string
   <td>Set the level of verbosity of geckodriver and Firefox.
    Available levels are <code>trace</code>,
    <code>debug</code>, <code>config</code>,
    <code>info</code>, <code>warn</code>,
    <code>error</code>, and <code>fatal</code>.
    If left undefined the default is <code>info</code>.
-   The value is treated case-insensitively.
+   The value is case-insensitive.
  </tr>
 </table>
 
 
 `prefs` object
 --------------
 
 <table>
--- a/testing/marionette/client/marionette_driver/geckoinstance.py
+++ b/testing/marionette/client/marionette_driver/geckoinstance.py
@@ -264,17 +264,17 @@ class GeckoInstance(object):
         args = {"preferences": deepcopy(self.required_prefs)}
         args["preferences"]["marionette.port"] = self.marionette_port
         args["preferences"]["marionette.defaultPrefs.port"] = self.marionette_port
 
         if self.prefs:
             args["preferences"].update(self.prefs)
 
         if self.verbose:
-            level = "TRACE" if self.verbose >= 2 else "DEBUG"
+            level = "Trace" if self.verbose >= 2 else "Debug"
             args["preferences"]["marionette.log.level"] = level
             args["preferences"]["marionette.logging"] = level
 
         if "-jsdebugger" in self.app_args:
             args["preferences"].update({
                 "devtools.browsertoolbox.panel": "jsdebugger",
                 "devtools.debugger.remote-enabled": True,
                 "devtools.chrome.enabled": True,
--- a/testing/marionette/doc/Prefs.md
+++ b/testing/marionette/doc/Prefs.md
@@ -29,17 +29,17 @@ allow time for user to set breakpoints i
 
 `marionette.log.level`
 ----------------------
 
 Sets the verbosity level of the Marionette logger repository.  Note
 that this preference does not control the verbosity of other loggers
 used in Firefox or Fennec.
 
-The available levels are, in descending order of severity, `trace`,
+The available levels are, in descending order of severity, `Trace`,
 `debug`, `config`, `info`, `warn`, `error`, and `fatal`.  The value
 is treated case-insensitively.
 
 
 `marionette.port`
 -----------------
 
 Defines the port on which the Marionette server will listen.  Defaults
--- a/testing/marionette/log.js
+++ b/testing/marionette/log.js
@@ -1,94 +1,63 @@
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this file,
  * You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 "use strict";
 
-ChromeUtils.import("resource://gre/modules/Services.jsm");
 const StdLog = ChromeUtils.import("resource://gre/modules/Log.jsm", {}).Log;
 
-const {MarionettePrefs} = ChromeUtils.import("chrome://marionette/content/prefs.js", {});
-
 this.EXPORTED_SYMBOLS = ["Log"];
 
-const isChildProcess = Services.appinfo.processType ==
-    Services.appinfo.PROCESS_TYPE_CONTENT;
+const PREF_LOG_LEVEL = "marionette.log.level";
 
 /**
  * Shorthand for accessing the Marionette logging repository.
  *
  * Using this class to retrieve the `Log.jsm` repository for
  * Marionette will ensure the logger is set up correctly with the
  * appropriate stdout dumper and with the correct log level.
  *
  * Unlike `Log.jsm` this logger is E10s safe, meaning repository
- * configuration for appenders and logging levels are communicated
- * across processes.
- *
- * @name Log
+ * configuration is communicated across processes.
  */
-class MarionetteLog {
+class Log {
   /**
    * Obtain the `Marionette` logger.
    *
    * The returned {@link Logger} instance is shared among all
    * callers in the same process.
    *
    * @return {Logger}
    */
   static get() {
     let logger = StdLog.repository.getLogger("Marionette");
     if (logger.ownAppenders.length == 0) {
       logger.addAppender(new StdLog.DumpAppender());
+      logger.manageLevelFromPref(PREF_LOG_LEVEL);
     }
-    logger.level = MarionettePrefs.logLevel;
     return logger;
   }
 
   /**
    * Obtain a logger that logs all messages with a prefix.
    *
    * Unlike {@link LoggerRepository.getLoggerWithMessagePrefix()}
    * this function will ensure invoke {@link #get()} first to ensure
-   * the logger has been properly set up first.
+   * the logger has been properly set up.
    *
    * This returns a new object with a prototype chain that chains
    * up the original {@link Logger} instance.  The new prototype has
    * log functions that prefix `prefix` to each message.
    *
    * @param {string} prefix
    *     String to prefix each logged message with.
    *
    * @return {Proxy.<Logger>}
    */
   static getWithPrefix(prefix) {
     this.get();
     return StdLog.repository.getLoggerWithMessagePrefix("Marionette", `[${prefix}] `);
   }
 }
 
-class ParentProcessLog extends MarionetteLog {
-  static get() {
-    let logger = super.get();
-    Services.ppmm.initialProcessData["Marionette:Log"] = {level: logger.level};
-    return logger;
-  }
-}
-
-class ChildProcessLog extends MarionetteLog {
-  static get() {
-    let logger = super.get();
-
-    // Log.jsm is not e10s compatible (see https://bugzil.la/1411513)
-    // so loading it in a new child process will reset the repository config
-    logger.level = Services.cpmm.initialProcessData["Marionette:Log"] || StdLog.Level.Info;
-
-    return logger;
-  }
-}
-
-if (isChildProcess) {
-  this.Log = ChildProcessLog;
-} else {
-  this.Log = ParentProcessLog;
-}
+this.Log = Log;
--- a/testing/marionette/prefs.js
+++ b/testing/marionette/prefs.js
@@ -183,16 +183,18 @@ class MarionetteBranch extends Branch {
 
   /**
    * Fail-safe return of the current log level from preference
    * `marionette.log.level`.
    *
    * @return {Log.Level}
    */
   get logLevel() {
+    // TODO: when geckodriver's minimum supported Firefox version reaches 62,
+    // the lower-casing here can be dropped (https://bugzil.la/1482829)
     switch (this.get("log.level", "info").toLowerCase()) {
       case "fatal":
         return Log.Level.Fatal;
       case "error":
         return Log.Level.Error;
       case "warn":
         return Log.Level.Warn;
       case "config":
--- a/testing/marionette/prefs/marionette.js
+++ b/testing/marionette/prefs/marionette.js
@@ -14,17 +14,17 @@ pref("marionette.enabled", false);
 // allow time for user to set breakpoints in the Browser Toolbox.
 pref("marionette.debugging.clicktostart", false);
 
 // Verbosity of Marionette logger repository.
 //
 // Available levels are, in descending order of severity,
 // "trace", "debug", "config", "info", "warn", "error", and "fatal".
 // The value is treated case-insensitively.
-pref("marionette.log.level", "info");
+pref("marionette.log.level", "Info");
 
 // Certain log messages that are known to be long are truncated
 // before they are dumped to stdout.  The `marionette.log.truncate`
 // preference indicates that the values should not be truncated.
 pref("marionette.log.truncate", true);
 
 // Port to start Marionette server on.
 pref("marionette.port", 2828);
--- a/testing/mochitest/runtests.py
+++ b/testing/mochitest/runtests.py
@@ -1904,17 +1904,17 @@ toolbar#nav-bar {
 
         # Hardcoded prefs (TODO move these into a base profile)
         prefs = {
             "browser.tabs.remote.autostart": options.e10s,
             "dom.ipc.tabs.nested.enabled": options.nested_oop,
             "idle.lastDailyNotification": int(time.time()),
             # Enable tracing output for detailed failures in case of
             # failing connection attempts, and hangs (bug 1397201)
-            "marionette.log.level": "TRACE",
+            "marionette.log.level": "Trace",
         }
 
         if options.flavor == 'browser' and options.timeout:
             prefs["testing.browserTestHarness.timeout"] = options.timeout
 
         # browser-chrome tests use a fairly short default timeout of 45 seconds;
         # this is sometimes too short on asan and debug, where we expect reduced
         # performance.