1719692 - Stop using MarionettePrefs.logLevel and use remote.log.level instead

Status: RESOLVED FIXED

(Remote Protocol :: Marionette, task, P2)

Description

[Julian Descottes [:jdescottes]]

We are still relying on MarionettePrefs.logLevel in MarionetteEventsChild.jsm

XPCOMUtils.defineLazyGetter(this, "isTraceLevel", () => {
  const StdLog = ChromeUtils.import("resource://gre/modules/Log.jsm").Log;

  return [StdLog.Level.All, StdLog.Level.Trace].includes(
    MarionettePrefs.logLevel
  );
});

However, all marionette modules now use the shared remote/shared/Log.jsm module, so we should use the same preference for consistency.

Note that Log.jsm can use either remote.log.level or marionette.log.level depending on which preference has the most verbose value. It would probably make sense to expose a isTraceLevel in Log.jsm that would use the correct preference.

Comment 1

[Julian Descottes [:jdescottes]]

Interesting to see that logLevel has a custom logic intended to make the pref case insensitive:
https://searchfox.org/mozilla-central/rev/da25888c4495585c532640f0e5efad07b1037621/remote/marionette/prefs.js#162-182

This feels inconsistent, since marionette.prefs.logLevel is only used in MarionetteEventsChild, and remote/marionette/log.js is reading the raw value of the preference. I can see that Bug 1482829 both added started using the raw preference in log.js and intentionally kept the case-insensitive logic until "geckodriver's minimum supported Firefox version reaches 62". I don't fully understand why this is needed.

Comment 2

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Via https://firefox-source-docs.mozilla.org/testing/geckodriver/Support.html we still support Firefox 60, but this will change with the 0.30.0 release where we want to limit it to the oldest supported ESR release - which is 78 right now. So I'm happy to get this custom code removed in Marionette and let the code use the real preference name.

Comment 3

[Julian Descottes [:jdescottes]]

(In reply to Henrik Skupin (:whimboo) [⌚️UTC+1] from comment #2)

Via https://firefox-source-docs.mozilla.org/testing/geckodriver/Support.html we still support Firefox 60, but this will change with the 0.30.0 release where we want to limit it to the oldest supported ESR release - which is 78 right now. So I'm happy to get this custom code removed in Marionette and let the code use the real preference name.

Ok, sounds good to me.

Comment 4

[Julian Descottes [:jdescottes]]

Attachment: Bug 1719692 - [remote] Stop using MarionettePrefs.logLevel and use remote.log.level instead

Depends on D120894

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/8dc95ec025f4
[remote] Stop using MarionettePrefs.logLevel and use remote.log.level instead r=webdriver-reviewers,whimboo

Comment 6

[Sandor Molnar[:smolnar]]

https://hg.mozilla.org/mozilla-central/rev/8dc95ec025f4