[🚀 Feature]: Selenium Manager change outputs

[titusfortner]

Feature and motivation

@nvborisenko proposes:

if success - exit code should be 0, and the latest line in output should be exact the path to the binary".

@symonk proposes:

never output any logging level to stdout or stderr, just write errors to stderr and non errors to stdout, that way it's easier to pipe to other tools etc and not require every consumer who cares about the path output have to do parsing to get what they care about

My biggest take is that we shouldn't be putting error messages in stdout. I don't like having to remove INFO\t from results, especially since it is inconsistent with behavior of --version.

Maybe a compromise is to log the level when --debug is turned on, and leave it off otherwise?

Usage example

Current behavior

This is stdout with error code 0

$ ~/selenium-manager --browser firefox
INFO	/Users/titusfortner/.cache/selenium/geckodriver/mac64/0.32.0/geckodriver

This is also stdout with error code 0

$ ~/selenium-manager --version
selenium-manager 1.0.0-M2

This is stdout with error code > 0

$ ~/selenium-manager --browser firefoxes
Error: "Invalid browser/driver name"

This is stderr with error code > 0

$ ~/selenium-manager --what
error: Found argument '--what' which wasn't expected, or isn't valid in this context

  If you tried to supply '--what' as a value rather than a flag, use '-- --what'

Usage: selenium-manager [OPTIONS]

For more information try '--help'

stderr with error code > 0

$ ~/selenium-manager --browser
error: The argument '--browser <BROWSER>' requires a value but none was supplied

For more information try '--help'

[bonigarcia]

In my opinion, the log level (INFO, WARN. etc.) help to understand the log info, also when selenium-manager is executed directly from the shell (e.g. as an standalone app), so I would not remove this level. In the case of driver management, which is the big feature supported so far, the driver path is contained in a INFO log (which is unique, i.e., there is no more INFO logs). But even in that case, there could be WARN logs (see #11373) that contain warning messages that could be processed in the bindings to warn the user about some undesired behaviour (not and error, but something that can be potentially improved) to the final user.

[AutomatedTester]

Could we make the output computer consumable? E.g. Just print the messages within a JSON blob?

[titusfortner]

I was going to say that JSON seems like overkill, but then I got to thinking about how the bindings should convey potential additional information to the user.

Real world situation right now: Chrome dev channel doesn't have a compatible chromedriver released yet.
I think the most recent driver should be returned, but ideally the user should be notified that the versions aren't matched

The solution we have right now doesn't allow for anything that isn't either an error or the desired value. Maybe JSON is the right answer?

[titusfortner]

Out of curiosity, do we have examples of any other command line executable that adds log-like information to its sdout by default?

[AutomatedTester]

At Mozilla we had output that was machine readable. I only suggested JSON as it's simpler that writing our own consumer in each language. You can see an example of the output at Mozilla https://firefoxci.taskcluster-artifacts.net/H6oXKqcQTGy9xwcVmyFh0g/0/public/logs/live_backing.log

[titusfortner]

I agree, minimizing logic duplication across languages is the goal. So stdout would be JSON blob & stderr would be string with the issue?

The main problem with this, of course, is that it wouldn't look great to a user working with it on the command line directly. Were the tools at Mozilla only consumed by machine? Should there be a flag of some kind to toggle the kind of output?

[titusfortner]

Another advantage of JSON is that when we eventually add downloading the browser, we're going to want to output the location of the browser as well... JSON would be a good way to provide data about both driver & browser.

[bonigarcia]

We can implement a new flag in Selenium Manager (e.g., --json-output) to produce the JSON blob containing the output. By default, Selenium Manager displays the output using a more human-friendly (or at least developer-friendly) using different log levels (INFO, WARN, ...), like it is doing now. The bindings will invoke Selenium Manager with --json-output and parse the results.

To implement this, we need to define a format for this JSON. Maybe something like this?

{
  "warnings": [
    "This is a warning trace",
    "This is another warning trace"
  ],
  "debug": [
    "This is a debug trace",
    "This is another debug trace"
  ],
  "result" : "/home/boni/.cache/selenium/chromedriver/linux64/106.0.5249.61/chromedriver"
}

... or alternatively:

{
  "warnings": [
    {
      "timestamp": 1670343174,
      "message" : "This is a warning trace"
    },
    {
      "timestamp": 1670343483,
      "message" : "This is another warning trace"
    }
  ],
  "debug": [
    {
      "timestamp": 167034423,
      "message" : "This is a debug trace"
    },
    {
      "timestamp": 167034900,
      "message" : "This is another debug trace"
    },
  ],
  "result" : "/home/boni/.cache/selenium/chromedriver/linux64/106.0.5249.61/chromedriver"
}

Also, we need to decide what to do when unexpected errors (panic in Rust) happen. In that case (code results > 0), the JSON blob is not displayed, and the bindings get both sdtout and sdterr and use this info to throw an exception to the user. Right?

[bonigarcia]

Another attempt including output code:

{
  "warnings": [
    {
      "timestamp": 1670343174,
      "message" : "This is a warning trace"
    }
  ],
  "debug": [
    {
      "timestamp": 167034423,
      "message" : "This is a debug trace"
    }
  ],
  "result" : {
    "code" : 0,
    "message" : "/home/boni/.cache/selenium/chromedriver/linux64/106.0.5249.61/chromedriver"
  }
}

{
  "warnings": [
    {
      "timestamp": 1670343174,
      "message" : "This is a warning trace"
    }
  ],
  "debug": [
    {
      "timestamp": 167034423,
      "message" : "This is a debug trace"
    }
  ],
  "result" : {
    "code" : 65,
    "message" : "Wrong driver version"
  }
}

[AutomatedTester]

I would do something like

{
     "output": [
         {"type": "debug" 
            "timestamp": 167034423,
           "message" : "This is a debug trace"
         }
     ]
}

or just as each line needs to be output just do

 {"type": "debug" 
            "timestamp": 167034423,
           "message" : "This is a debug trace"
         }

so that as each line is output it can be consumed straight away.

[titusfortner]

There is no each line for consumption, really. Selenium sends the command and gets back a bunch of output.

Something like this is what would be easiest for bindings to parse:

{
code: 1,
error: "Bad things",
log: ["INFO\t1", "DEBUG\t2"]
warning: ["warning 1", "warning 2"]
output: {driver: "/path/to/driver",
         browser: "/path/to/browser",
         version: "v1.0"}
}

bindings pseudo code would be:

result = execute(selenium-manager, args)
if result.code > 0
  log result.log
  throw result.error

if debug-mode
  log result.log

log result.warning
return result.output 

[diemol]

Not 100% with @titusfortner on the contents, but with the format. Seems to me that a JSON with just one level is enough, why would we need to nest messages or an array of messages?