Add -v flag to optionally increase Gecko verbosity

[andreastt]

Introduces a cumulative -v flag where a single instance mean INFO level
and above are shown and two mean TRACE. The log levels are defined in
Log.jsm in the Gecko toolkit.

MarionetteSettings gains a new `verbosity' property that if undefined
uses the defaults in Gecko. This means we are no longer requiring the
marionette.logging preference to be set.

The defaults in Gecko are INFO for optimised builds and DEBUG for
debug builds.

Fixes #89.

---

This change is

[jgraham]

Reviewed 2 of 2 files at r1.
Review status: all files reviewed at latest revision, 3 unresolved discussions, some commit checks failed.

---

src/main.rs, line 113 [r1] (raw file):

    // which are info for optimised builds
    // and debug for debug builds
    let log_level = if opts.verbosity == 1 {

Maybe rewrite as

match (opts.verbosity) {
    0 => None,
    1 => Some(Loglevel::Debug),
     _ => Some(LogLevel::Trace)
 }

---

src/marionette.rs, line 270 [r1] (raw file):

    /// level. The Gecko default is LogLevel::Info for optimised
    /// builds and LogLevel::Debug for debug builds.
    pub verbosity: Option<LogLevel>,

Why not call this log_level?

---

src/marionette.rs, line 279 [r1] (raw file):

    port: u16,
    e10s: bool,
    verbosity: Option<LogLevel>,

Also log_level here.

---

Comments from Reviewable

[jgraham]

Review status: all files reviewed at latest revision, 4 unresolved discussions, some commit checks failed.

---

src/main.rs, line 82 [r1] (raw file):

                        "Load Firefox with an e10s profile");
        parser.refer(&mut opts.verbosity)
            .add_option(&["-v"], IncrBy(1),

This needs a corresponding long form.

---

Comments from Reviewable

[andreastt]

Review status: 0 of 2 files reviewed at latest revision, 4 unresolved discussions.

---

src/main.rs, line 82 [r1] (raw file):

Previously, jgraham wrote…

This needs a corresponding long form.

Done through adding the `--log=LEVEL` long form we talked about.

---

Comments from Reviewable

[jgraham]

Review status: 0 of 2 files reviewed at latest revision, 4 unresolved discussions, some commit checks failed.

---

src/main.rs, line 49 [r2] (raw file):

        let prog = env::args().next().unwrap();
        println_stderr!("{}: error: {}", prog, format_args!($($arg)*));
        process::exit(64);

Why 64?

---

src/main.rs, line 137 [r2] (raw file):

    } else if opts.log_level.len() > 0 {
        match LogLevel::from_str(&opts.log_level) {
            Ok(l) => Some(l),

Don't use l as a single-letter variable.

---

Comments from Reviewable

[jgraham]

Reviewed 2 of 2 files at r2.
Review status: all files reviewed at latest revision, 4 unresolved discussions, some commit checks failed.

---

Comments from Reviewable

[andreastt]

Review status: all files reviewed at latest revision, 4 unresolved discussions, some commit checks failed.

---

src/main.rs, line 49 [r2] (raw file):

Previously, jgraham wrote…

Why 64?

Apparently 64 is defined in [sysexits.h](http://linux.die.net/include/sysexits.h) as command line usage error.

---

src/main.rs, line 137 [r2] (raw file):

Previously, jgraham wrote…

Don't use l as a single-letter variable.

Does `l` carry some special significance in Rust, like `s` and similar in Python (when you use pdb)?

---

src/marionette.rs, line 270 [r1] (raw file):

Previously, jgraham wrote…

Why not call this log_level?

Done.

---

src/marionette.rs, line 279 [r1] (raw file):

Previously, jgraham wrote…

Also log_level here.

Done.

---

Comments from Reviewable

[jgraham]

Review status: all files reviewed at latest revision, 1 unresolved discussion, some commit checks failed.

---

src/main.rs, line 49 [r2] (raw file):

Previously, andreastt (Andreas Tolfsen) wrote…

Apparently 64 is defined in sysexits.h as command line usage error.

OK, I guess. It's not like any consumer can rely on that without specific knowledge of the program but one non-zero code is as good as another.

---

src/main.rs, line 137 [r2] (raw file):

Previously, andreastt (Andreas Tolfsen) wrote…

Does l carry some special significance in Rust, like s and similar in Python (when you use pdb)?

It's just hard to read e.g. it looks like `1` in reviewable.

---

Comments from Reviewable

[andreastt]

Review status: 1 of 2 files reviewed at latest revision, 1 unresolved discussion.

---

src/main.rs, line 137 [r2] (raw file):

Previously, jgraham wrote…

It's just hard to read e.g. it looks like 1 in reviewable.

Done.

---

Comments from Reviewable

[andreastt]

Fixed the tests.

[jgraham]

Reviewed 1 of 1 files at r4.
Review status: all files reviewed at latest revision, all discussions resolved.

---

Comments from Reviewable

[timrsfo]

This does not appear to be working, using the latest Geckodriver as of today, 9/18/17
how would I change the default DEBUG to ERROR or FATAL
e.g. I don't want to see this:

1505858205404	Marionette	DEBUG	Received observer notification "outer-window-destroyed" for "2147483661"

Tried this:

#!/bin/bash
echo "GECKO COMMANDLINE ARGS: " $@

#geckodriver --log fatal "$@" > /dev/null 2>&1 &   // (1) fatal, redirect null, works but fatal is a no-op
#geckodriver --log fatal "$@" &                    // (2) fatal, no redirect, fatal arg not working
#geckodriver -vvvv "$@" &                          // (3) just -v option - not working yet

geckodriver "$@" > /dev/null 2>&1                // (4) This works

pid="$!"

echo "geckodriver.sh pid is: $$"
echo "child geckodriver  pid is: $pid"

trap 'echo I am going down, so killing off my processes..; kill $pid; exit' SIGHUP SIGINT SIGQUIT SIGTERM

wait

I still get all kinds of DEBUG logs

can you provide a commandline example of how to use this -v flag for geckodriver, plz ?

NOTE: The above shell script can be used for a temporary work-around until the '--log fatal' and '-v' options are fixed. Just set the following (on linux):

"webdriver.gecko.driver": "/usr/local/bin/geckodriver.sh",

NOTE: The above script turns off logging or at least redirects it to null.

[andreastt]

@timrsfo That is a bug in Firefox, see https://bugzilla.mozilla.org/show_bug.cgi?id=1384956.

[timrsfo]

@andreastt thanks for the feedback. I came here searching for an answer for too much log output from, geckodriver with webdriver 3.5+. I hope the fix includes a way to pass geckodriver args such as -v, and --log fatal to geckodriver from webdriver. this is off topic though so I will drop off.

[whimboo]

@timrsfo please note that issue #955 is what you want, which covers the geckodriver part beside the bug @andreastt mentioned above.