Closed Bug 1603078 Opened 5 years ago Closed 5 years ago

Run remote(pup) with Fission enabled

Categories

(Remote Protocol :: Agent, task, P1)

task

Tracking

(Fission Milestone:M6, firefox74 fixed)

RESOLVED FIXED
Firefox 74
Fission Milestone M6
Tracking Status
firefox74 --- fixed

People

(Reporter: ato, Assigned: ato)

References

(Blocks 1 open bug)

Details

(Whiteboard: [puppeteer-beta-mvp])

Attachments

(3 files)

We should run the Puppeteer test suite (known on Treeherder as remote(pup)) with Fission enabled so we can track progress towards remote-fission.

This should just be a matter of adding a flag to ./mach puppeteer-test that sets the fission.autostart preference, and create a new task entry in taskcluster/ci/source-test/remote.yml.

Priority: -- → P3

Tracking for Fission Nightly (M6)

Fission Milestone: --- → M6
Assignee: nobody → ato
Status: NEW → ASSIGNED
Priority: P3 → P1
Whiteboard: [puppeteer-beta]

Puppeteer accepts an environmental variable EXTRA_LAUNCH_OPTIONS
that is a JSON encoded object as a string, containing additional
configuration to use when running tests.

This takes an extraPrefsFirefox key which is an object
mapping preference names to values. This is extracted in
remote/test/puppeteer/lib/Launcher.js:390 and later written to a
fresh profile in :515.

It appears remote/mach_commands.py has left out the "Firefox" appendix.

The "./mach puppeteer-test" command now takes a new flag,
--enable-fission, which will set the fission.autostart preference to true.

This changeset is not complete yet, but I’ve submitted a couple of patches to get us started.

You did some refactoring of how Fission test jobs are configured for TaskCluster recently. What do I have to do to set up a Fission variant for the remote(pup) job?

As you can see from D58512, I’ve already added an --enable-fission flag to ./mach puppeteer-test. I assume I need to include that flag somewhere?

Flags: needinfo?(ahal)
Whiteboard: [puppeteer-beta] → [puppeteer-beta-mvp]

Is remote(pup) a test task? If so you'll need to make sure your task:
A) Has a variants key that includes "fission"
B) Sets fission-run-on-projects (this is the same as run-on-projects but only applies to fission tasks)

Enabling the fission variant will pass:

            'mozharness': {
                'extra-options': ['--setpref=fission.autostart=true',
                                  '--setpref=dom.serviceWorkers.parent_intercept=true',
                                  '--setpref=browser.tabs.documentchannel=true'],
            },

Note this goes into mozharness not the mach command. If your harness supports --setpref I'd recommend making --enable-fission a simple alias to the above, and then updating your internal logic to look for the fission.autostart pref to determine whether or not it is running with fission.

If your harness doesn't support --setpref or it is not a test kind, you're off the beaten path and will need to figure out some other way of hooking it all up properly.

Flags: needinfo?(ahal)

Andrew, the puppeteer tests are source-tests, which simply fetch the binary via a fetch task. So I assume the alias thing would somewhat work then?

Yeah, though that's more for the harness side of things.

In the taskgraph side, you'll need to decide how to define fission or not. E.g, you could simply duplicate the entire task in the .yml files, or you could implement a fission: true key in the task definition that you then read from a transform that splits the task into two (one fission and one non-fission). If it's just a single task, might be easiest to just duplicate the whole thing in the configs and tweak the necessary bits.

Because the Puppeteer unit tests run as source tasks and not test
tasks, we can't rely on the generic technique for enabling Fission
available to other test tasks based on mozharness.

Since the puppeteer task is a simple, opt-in, non-scheduled job we
duplicate it as puppeteer-fis and pass --enable-fission to "./mach
puppeteer-test" instead. This achieves the same effect.

Just a quick follow-up question:

Are the dom.serviceWorkers.parent_intercept and browser.tabs.documentchannel preferences important to us?

Flags: needinfo?(ahal)

For the record I’ve filed bug 1606812 to employ mozharness for the Puppeteer tests. Not considering this a blocker, but it is something we should look into at some point.

I have no idea, 301 :kmag.

Flags: needinfo?(ahal) → needinfo?(kmaglione+bmo)
Blocks: 1606828

(In reply to Andreas Tolfsen ❲:ato❳ from comment #10)

Are the dom.serviceWorkers.parent_intercept and browser.tabs.documentchannel preferences important to us?

I think the plan is to enabled these prefs by default and ride the trains in 73.

@ Matt Woodrow, who is working on Document Channel and can answer definitively.

Flags: needinfo?(matt.woodrow)

(In reply to Chris Peterson [:cpeterson] from comment #13)

(In reply to Andreas Tolfsen ❲:ato❳ from comment #10)

Are the dom.serviceWorkers.parent_intercept and browser.tabs.documentchannel preferences important to us?

I think the plan is to enabled these prefs by default and ride the trains in 73.

@ Matt Woodrow, who is working on Document Channel and can answer definitively.

Both of these prefs are going to be required for fission to work properly, the latter will almost certainly affect these tests.

They both are true by default on Nightly, and the plan is for them to ride with 73. That said, there's probably no harm in explicitly setting them (like we do for other --enable-fission options), and we can remove them all at the same time.

Flags: needinfo?(matt.woodrow)

The remote agent only ships on Nightly which would mean that these preferences will become defaults before we start shipping on other release channels.

However, as Matt says there is probably little harm in setting them explicitly. This would allow us to run tests more reliably against other release branches if we would wish.

Flags: needinfo?(kmaglione+bmo)
Pushed by atolfsen@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/44d98b5ee706 remote: fix passing extra prefs to Puppeteer r=remote-protocol-reviewers,maja_zf https://hg.mozilla.org/integration/autoland/rev/74eea3cb0eac remote: add --enable-fission to "./mach puppeteer-test" r=remote-protocol-reviewers,whimboo https://hg.mozilla.org/integration/autoland/rev/54b9d2b12988 taskcluster: add remote(pup-fis) job; r=ahal
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 74

With Fission enabled we only run 267 out of 638 tests. In the case below it is due to navigation times out for Firefox Browser Page Page.waitForNavigation should work with history.pushState() (navigation.spec.js:374:5).

Ran 267 (ok - 56, failed - 177, skipped - 34) of 638 test(s)

https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=284595705&repo=mozilla-central&lineNumber=13422

I don't think we should investigate that right now until navigation is making use of the JSWindowActor implementation (bug 1565162).

You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: