Closed Bug 1567264 Opened 5 years ago Closed 4 years ago

Add `--enable-fission` options to `./mach test` and friends

Categories

(Testing :: General, enhancement, P2)

Version 3
enhancement

Tracking

(Fission Milestone:M6c)

RESOLVED FIXED
Fission Milestone M6c

People

(Reporter: nika, Assigned: gbrown)

References

(Blocks 1 open bug)

Details

(Whiteboard: [dev-prod-2020])

Attachments

(3 files)

Currently, tools like ./mach test, ./mach mochitest, ./mach web-platform-tests, etc. often support the --disable-e10s flag to turn off browser.tabs.remote.autostart. For fission testing right now it is currently necessary to manually configure the fission.autostart pref when running tests, and it would be much nicer if we could have an --enable-fission flag which does this for you.

This option would need to set the fission.autostart pref to true in the profile before running tests. This should be done before starting the browser, not dynamically.

Andrew, we chatted about this recently. We have more and more devs asking for this and we would appreciate your help in getting this option available.

Flags: needinfo?(ahal)

Sure, might not have a chance to look for awhile, I'll leave the needinfo for now.

I think we'll essentially want a shim that gets confined to the argument parsing code (i.e passing --enable-fission is equivalent to --setpref=fission.autostart=1). I'd want to avoid introducing fission specific logic/flags to the harness proper.

Priority: -- → P3

Note we should probably add a quick check that we never try to run with both --enable-fission and --disable-e10s (not that I'd really expect anyone to do that), and more importantly that we never try to run suites that disable e10s (e.g., chrome mochitests) with the fission flag enabled.

Andrew McCreight ran into that accidentally when running tests locally, which led to things failing in weird ways that it took a lot of time to diagnose. I don't really want more people running into that sort of situation...

I should have some time to look at this tomorrow.

Note it's a bit trickier than one might expect, as there are many edges cases. For example, mach mochitest <dir>, will run all mochitests under <dir>, including mochitest-chrome which only runs without e10s. So what do you do when you run ./mach mochitest <dir> --enable-fission and <dir> contains chrome mochitests? Similarly, not all test harnesses will be able to support --enable-fission, so what do you do when you run ./mach test <dir> --enable-fission and <dir> contains such harnesses? Not saying this is a hard problem, just pointing out it's more than a trivial fix.

Assignee: nobody → ahal
Status: NEW → ASSIGNED
Flags: needinfo?(ahal)
Priority: P3 → P2

This argument is an alias for '--setpref="fission.autostart=1"'.

Depends on D39547

I'll keep this open to tackle reftest, wpt, etc.

Keywords: leave-open
Pushed by ahalberstadt@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/9014816fb2f5
[mozlog] Support log errors in the errorsummary and mach formatter r=jgraham
https://hg.mozilla.org/integration/autoland/rev/d4061d6bc8c5
[mochitest] Add a convenience --enable-fission argument r=gbrown
Depends on: 1571529
Pushed by ahalberstadt@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/f4179666f9fc
[mochitest] Ensure --enable-fission sets the pref to 'true' instead of '1', r=kmag

Roll unfixed test bugs from Fission Milestone M4 to M4.1

Fission Milestone: M4 → M4.1

Andrew, any updates on when you will be able to resume work adding mach test --enable-fission?

Bug 1622436 asks for an --enable-fission option to be added to mach reftest and crashtest.

Bug 1622338 added an --enable-fission option to mach wpt.

Bug 1596770 added an --enable-fission option to mach marionette-test.

I'm moving this bug from Fission's M4.1 mochitest milestone to Fission Nightly M6c because adding a mach test --enable-fission option is not strictly needed to make Fission mochitests pass.

Fission Milestone: M4.1 → M6c
Depends on: 1622436, 1622338, 1596770
Flags: needinfo?(ahal)

Thanks for the reminder, this definitely slipped through the cracks.

Adding this to mach test will also involve adding it to all the other harnesses (otherwise, the mach test invocation will fail if a directory also contains, e.g xpcshell tests). It's not terribly hard, but not trivial either.

Assuming this isn't a top priority, I'm going to un-assign myself for now. Geoff Brown has been burning down a lot of the useability bugs like this lately, and the added whiteboard tag should get this into his queue.

Assignee: ahal → nobody
Status: ASSIGNED → NEW
Flags: needinfo?(ahal)
Whiteboard: [dev-prod-2020]

(In reply to Andrew Halberstadt [:ahal] from comment #16)

Adding this to mach test will also involve adding it to all the other harnesses (otherwise, the mach test invocation will fail if a directory also contains, e.g xpcshell tests). It's not terribly hard, but not trivial either.

Assuming this isn't a top priority, I'm going to un-assign myself for now. Geoff Brown has been burning down a lot of the useability bugs like this lately, and the added whiteboard tag should get this into his queue.

I don't think this is a high priority. Having --enable-fission on individual mach test commands (reftest and crashtest bug 1622436 and fixed wpt bug 1622338 and marionette bug 1596770) is probably more useful to developers trying to debug individual tests. In the meantime, developers can set the fission.autostart pref when running tests locally.

Eventually Fission will be enabled by default (some time in 2021) or be the only support configuration. --enable-fission won't be useful then, but --disable-fission might. I don't know.

Assignee: nobody → gbrown

Is there anything left to do in this bug?

I think --enable-fission is now supported by:

  • mochitest
  • reftest/crashtest/etc
  • marionette
  • web-platform tests
  • raptor
  • talos

'mach test --enable-fission' will work fine when run against any of those suites and will error out if run against a suite that does not support --enable-fission.

That seems roughly equivalent to existing support for --disable-e10s.

Flags: needinfo?(nika)
Flags: needinfo?(cpeterson)

Is there anything left to do in this bug?

Not that I know of. Nika may know of other work.

Flags: needinfo?(cpeterson)

We should also be sure to have --enable-fission support for mach run, where I don't think it's supported yet. (an --enable-fission argument seems to be passed directly to Firefox).

Other than that, I think you've covered all of the bases. Thanks!

Flags: needinfo?(nika)
Depends on: 1644876
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: