1390451 - All test harnesses should support a/the --binary flag

Status: NEW

(Testing :: General, enhancement, P3)

Description

[Mike Hommey [:glandium]]

Some test harnesses allow to pass a --binary argument to give the path of a firefox that doesn't come from a local build. All harnesses should have such an option. For instance, I wanted to run locally a crashtest with a try build... (under rr), but can't out of the box.

Comment 1

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

In case of some this applies to Marionette and Firefox UI tests right now. It's done like that:

https://dxr.mozilla.org/mozilla-central/source/testing/marionette/mach_commands.py#91-92

Greg, which mach file would we have to modify to make it globally available for test commands? Is that possible to do at a single location or would all the test harnesses need an update?

Comment 2

[Gregory Szorc [:gps]]

I'd put common code in python/mozbuild/mozbuild/base.py. That's already where we define many command conditions.

Also, if you do this, please make a custom decorator for adding common arguments to test commands. Right now, test command arguments vary in subtle ways. This hinders usability by introducing inconsistency.

Comment 3

[Gregory Szorc [:gps]]

For a concrete example of inconsistency between commands, see bug 1388117.

Comment 4

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

So as it looks like the --binary option as used by Marionette's mach command isn't defined there, but it's just forwarded to Marionette itself. This means if we want to have the same cli argument, each of our test harnesses would have to be updated to allow the --binary argument.

Ted, what do you think? In case of Marionette tests we clearly have a huge benefit for that all the last months.

Comment 5

[(not currently active) Ted Mielczarek]

I think we should support this in all test harnesses. Whatever you have to do behind the scenes to make that work is fine.

Comment 6

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

Mike, do you have any advice in where such an option would have to be added? If not who is the current expert on mach these days?

With that information it might be easier to get started on this bug. I kinda missed it myself again over the last days.

Comment 7

[Andrew Halberstadt [:ahal]]

You said it yourself in comment 4 :).

Each harness needs to be updated independently. I'd love if we had a place to define shared arguments across all harnesses (like how try selectors can share arguments with one another). I'd propose testing/mozbase/moztest/moztest/cli.py, but that is definitely out of scope for this bug.

Comment 8

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

So mochitest actually seems to use --appname here, which in-fact is also a bit misleading. Geoff, maybe you can give some background info here, and if it would be fine to change it to --binary?

Comment 9

[Geoff Brown [:gbrown]]

I am aware of some open bugs like bug 1351253.

On Android, --appname can be used to specify a package name, like org.mozilla.geckoview.test. In that context, "appname" might be more appropriate than "binary", although neither is accurate.

I am not very familiar with --appname on desktop, but know it is available for mochitest and reftest.

Ideally I would like to see just one option, --appname or --binary (or something else, --app-path??), but not both, universally available and accepted by 'mach test'.

Comment 10

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

(In reply to Geoff Brown [:gbrown] from comment #9)

On Android, --appname can be used to specify a package name, like org.mozilla.geckoview.test. In that context, "appname" might be more appropriate than "binary", although neither is accurate.

Maybe we should use a terminology what most modern harnesses in our tree make use of? web-platform-tests especially make use of --binary for desktop, and --package-name for Android. I think it should be fine to have such a distinction between these platforms. Also both describe very well what's expected as argument. Right now, I wouldn't expect that I have to add a path for --app-name.

Ideally I would like to see just one option, --appname or --binary (or something else, --app-path??), but not both, universally available and accepted by 'mach test'.

Shall we open up this discussion somewhere? Or who's agreement do we need?

Comment 11

[Geoff Brown [:gbrown]]

I don't think it needs a lot of discussion or agreement. Maybe just an announcement post once the changes land? And if any options are deprecated, let's be sure that they error out with a pointer to the new options.