Closed Bug 1540655 Opened 1 year ago Closed 8 months ago

Create TaskCluster job for the Puppeteer test suite

Categories

(Remote Protocol :: Agent, enhancement, P1)

enhancement

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: ato, Assigned: ato)

References

(Blocks 2 open bugs)

Details

Attachments

(11 files, 1 obsolete file)

47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review

To track our continued progress towards complete Puppeteer support
we need to create a new Tier-2/Tier-3 TaskCluster job running the
Puppeteer test suite on try.

The Puppeteer test suite requires Node.js, but since this is now a
build-time dependency we should be fine in that area. My current
thinking is to vendor the test suite in central, then have a mach
command for pulling in new changes from upstream.

Assignee: nobody → ato
Blocks: puppeteer
Status: NEW → ASSIGNED
Depends on: 1533831
Priority: -- → P2
Depends on: 1546945
Depends on: 1543115

I need to vendor the Puppeteer repository along with its dependencies
in central for this bug. My initial plan is to put the dependencies
under third_party/nodejs/ and annontate them with moz.yaml files,
and to put the Puppeteer repo under remote/test/puppeteer/.

I then intend to introduce a new mach command (something like
./mach remote vendor-puppeteer) to pull in the latest changes from
GitHub. This will allow the remote protocol to track progress towards
achieving full Puppeteer API support.

Does that all sound reasonable to you?

Flags: needinfo?(jlaster)

mach vendor already exists and seems like it should be the frontend here even if it requires a custom subcommand like mach vendor dav1d.

That seems reasonable. What third party dependencies do you have?
We meet tuesdays at 12:30pm est in my vidyo room to discuss node mc topics if you'd like to discuss in more depth.

CC Dan Mosedale

Flags: needinfo?(jlaster) → needinfo?(dmose)

We may start with a forked version of puppeteer. For example, to workaround bug 1543115. But I imagine we will hit some other similar blockers. I can easily see us having to put some special bits to run the tests for firefox.
What is your plan to accomodate that?

Also, is there an urge to support mach vendor to update puppeteer?
Wouldn't it be easier to clone it manually and rather focus on getting the test running?
I imagine we could do that and still flag the files correctly as vendor via moz.yaml?

I don’t think ./mach vendor supports Node.js quite yet, but my
thinking was to manage the third-party dependencies using that so
that they could live under third_party/.

As we need to lock the Puppeteer version to the remote agent and
have the ability to apply custom patches to it, I want to manually
export a git clone into remote/test/puppeteer/. I mentioned
./mach remote vendor-puppeteer because those steps can
be automated: not as a replacement for ./mach vendor.

Depends on: 1551188
No longer depends on: 1543115, 1546945
Flags: needinfo?(dmose)
Priority: P2 → P1
Depends on: 1553317

For info I made a fork of puppeteer 1.6.2 that works with the current Firefox Nightly and doesn't rely on Juggler. It normally sets the correct preferences for the profile. I'm not sure we want to use exactly that version here, but at least it should work: https://github.com/juliandescottes/pptr-ff

Yes, I did the same but with only the minimal amount of changes
required to make npm test run:
https://github.com/andreastt/puppeteer/tree/firefox

With my branch you still use output variables such as
PUPPETEER_SKIP_CHROMIUM_DOWNLOAD and CHROME.

Since we arrived at the conclusion that we probably need to fork
Puppeteer regardless in order to add support for Firefox, going
with your fork long-term seems like the right thing to do.

Introduces "./mach remote vendor-puppeteer" for vendoring the
Puppeteer client without dependencies into remote/test/puppeteer/.

The particular checkout of Puppeteer is
https://github.com/andreastt/puppeteer/tree/firefox, which contains a
couple of hotfixes we need for the client to work with the Firefox
implementation of CDP.

The remote agent targets a specific version of Puppeteer, so it is
not suitable for this to be vendored under third_party/. We also
wouldn't want other code in central to accidentally use a patched fork.

The vendoring process is not part of "./mach vendor" because it does
not yet have Node.js support, and implementing that for mach is outside
the scope of getting the Puppeteer tests running with the remote agent.

Puppeteer is licensed under the Apache-2.0 license.

No code from Puppeteer gets included in Firefox.

Introduces "./mach puppeteer-test" command for running the Puppeteer
tests against the remote agent. This has to be a top-level command
because the automatic test detection system in mach does not allow
us to delegate to a subcommand such as "./mach remote puppeteer-test".

The tests run against a fork of Puppeteer with hotfixes needed for
it to work with the CDP implementation in Firefox. This fork is
located at https://github.com/andreastt/puppeteer/tree/firefox, and
vendored under remote/test/puppeteer/ in a previous commit in this series.

We vendor the Puppeteer library, but not its dependencies.
When "npm install" is called in remote/test/puppeteer/, it puts
its dependencies under remote/test/puppeteer/node_modules/ and
generates a remote/test/puppeteer/package-lock.json file. We do
not want these to be checked in.

DONTBUILD

Adds a --symlink <PATH> flag that instead of checkout the Puppeteer
source code from a git repo, allows one to symlink a local directory
into remote/test/puppeteer. This is useful for local development.

The Puppeteer test flavour are functional tests for the CDP-based
Puppeteer library from Google, that we want to run against our
implementation of CDP for Firefox.

They are distinct from the Firefox Puppeteer tests based on Marionette.

Makes it possible for mach to resolve test paths for Puppeteer,
so that individual tests can be run from the command line using
"./mach test", as such:

% ./mach test remote/test/puppeteer/test/screenshot.spec.js

As the Puppeteer test suite is imported from upstream and we cannot
change this directory at will (i.e. to add test manifest files),
we take the same approach as for WPT and populate the manifest by
recursively walking the remote/test/puppeteer/test/**/*.spec.js file tree.

This makes it possible to for mach to determine which test types
are likely to be relevant.

This adds a "remote(pup)" TaskCluster job for running the Google
Puppeteer tests against the new Firefox remote agent.

Earlier in this patchset we added a "./mach puppeteer-test" command
that wraps npm, and this being a source-test with access to the
full checkout, invokes this command directly instead of going
through mozharness.

Attachment #9076094 - Attachment description: bug 1540655: build, remote: add mach command for vendoring Puppeteer; → bug 1540655: mozbuild: add mach command for vendoring Puppeteer;
Attachment #9076098 - Attachment is obsolete: true
Blocks: 1571406
Attachment #9076094 - Attachment description: bug 1540655: mozbuild: add mach command for vendoring Puppeteer; → bug 1540655: build, remote: add mach command for vendoring Puppeteer;
Attachment #9076098 - Attachment is obsolete: false
Attachment #9076098 - Attachment is obsolete: true
Pushed by atolfsen@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/997d1568d944
build: sort MACH_MODULES; r=firefox-build-system-reviewers,mshal
https://hg.mozilla.org/integration/autoland/rev/b96dede008ad
build, remote: add mach command for vendoring Puppeteer; r=firefox-build-system-reviewers,chmanchester
https://hg.mozilla.org/integration/autoland/rev/3b3a2a9fbc8b
remote: vendor Puppeteer; r=remote-protocol-reviewers,jdescottes
https://hg.mozilla.org/integration/autoland/rev/939ce2afcf0b
remote: add mach command for running Puppeteer tests; r=remote-protocol-reviewers,jdescottes
https://hg.mozilla.org/integration/autoland/rev/c0a80d37576d
remote: ignore non-vendored Node.js assets; r=remote-protocol-reviewers,jdescottes
https://hg.mozilla.org/integration/autoland/rev/8c67b494e207
mozbuild: add puppeteer test flavor; r=firefox-build-system-reviewers,chmanchester
https://hg.mozilla.org/integration/autoland/rev/bb012df3018b
moztest: add test path resolution for Puppeteer tests; r=ahal
https://hg.mozilla.org/integration/autoland/rev/8d7bad30be46
remote: connect Puppeteer spec tests with mach test flavour; r=remote-protocol-reviewers,jdescottes
https://hg.mozilla.org/integration/autoland/rev/73236f81da44
ci: add test job for Puppeteer tests; r=ahal
https://hg.mozilla.org/integration/autoland/rev/641a7cb25298
remote: document how to run Puppeteer tests; r=remote-protocol-reviewers,jdescottes
https://hg.mozilla.org/integration/autoland/rev/1a23d770d8a1
remote: document how to vendor Puppeteer; r=remote-protocol-reviewers,jdescottes

This was a test failure in test_resolve.py caused by is_puppeteer_path()
trying to match on None values. I’ve rebased and fixed the test.

Flags: needinfo?(ato)
Pushed by atolfsen@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/f2b9955b0822
build: sort MACH_MODULES; r=firefox-build-system-reviewers,mshal
https://hg.mozilla.org/integration/autoland/rev/8f22cff02e35
build, remote: add mach command for vendoring Puppeteer; r=firefox-build-system-reviewers,chmanchester
https://hg.mozilla.org/integration/autoland/rev/5b769a365120
remote: vendor Puppeteer; r=remote-protocol-reviewers,jdescottes
https://hg.mozilla.org/integration/autoland/rev/15c8a10a3994
remote: add mach command for running Puppeteer tests; r=remote-protocol-reviewers,jdescottes
https://hg.mozilla.org/integration/autoland/rev/beab1be0f328
remote: ignore non-vendored Node.js assets; r=remote-protocol-reviewers,jdescottes
https://hg.mozilla.org/integration/autoland/rev/4da1734351e6
mozbuild: add puppeteer test flavor; r=firefox-build-system-reviewers,chmanchester
https://hg.mozilla.org/integration/autoland/rev/bf91c2f4f386
moztest: add test path resolution for Puppeteer tests; r=ahal
https://hg.mozilla.org/integration/autoland/rev/23931313b016
remote: connect Puppeteer spec tests with mach test flavour; r=remote-protocol-reviewers,jdescottes
https://hg.mozilla.org/integration/autoland/rev/7712e6870bd3
ci: add test job for Puppeteer tests; r=ahal
https://hg.mozilla.org/integration/autoland/rev/836e30244634
remote: document how to run Puppeteer tests; r=remote-protocol-reviewers,jdescottes
https://hg.mozilla.org/integration/autoland/rev/8a9e9189cd98
remote: document how to vendor Puppeteer; r=remote-protocol-reviewers,jdescottes
See Also: → 1581165
See Also: → 1610611
You need to log in before you can comment on or make changes to this bug.