1797723 - Sync vendored puppeteer to v18.0.0

Status: RESOLVED FIXED

(Remote Protocol :: CDP, task, P1)

Description

[Alexandra Borovova [:Sasha]]

Sync vendored puppeteer to v17.1.3 (https://github.com/puppeteer/puppeteer/releases/tag/v17.1.3), which introduces changes to a test runner and a custom expectation file. Apart from regular changes, things which needs to be done here:

• update mach command to use the puppeteer expectation file,
• update the puppeteer expectation file to use FAIL, TIMEOUT and FAIL, PASS statuses instead of SKIP,
• update log parser to work with the puppeteer expectation file
• update documentation of the vendoring process: https://searchfox.org/mozilla-central/source/remote/doc/cdp/PuppeteerVendor.md

First two points were attempted to be addressed here: https://hg.mozilla.org/try/rev/d86001da13934ace1f4a5c8cc7e461c026c5cb6b

Comment 1

[Julian Descottes [:jdescottes]]

I was looking at this and I think there's at least one item we need to discuss internally.

update the puppeteer expectation file to use FAIL, TIMEOUT and FAIL, PASS statuses instead of SKIP,

As noted by Sasha, the current expectation file (remote/test/puppeteer/test/TestExpectations.json) skips a lot of tests for Firefox (around 300).
We want to convert some of them back to FAIL, TIMEOUT, FAIL, PASS or PASS, TIMEOUT or TIMEOUT. We can easily do that by comparing with our current expectation file https://searchfox.org/mozilla-central/source/remote/test/puppeteer-expected.json

However since this is now a shared expectation file, we need to discuss with puppeteer folks about the "TIMEOUT" tests. The expectation file from puppeteer does not seem to contain any TIMEOUT expectation, even though their runner understands it. (well there is one PASS,FAIL,TIMEOUT on the main branch, but nothing in tag v17.1.3).

Our puppeteer-expected.json contains 41 TIMEOUT expectations, so in the worst case scenario that means 41 x 25 seconds = 17 minutes of additional runtime just to assert that timeouts are still timeouts. It's understandable if puppeteer folks don't want to slow down their CI too much, so I'm not sure they would be happy with us switching SKIP tests to TIMEOUT.

We can maybe define some kind of convention, where puppeteer CI skips TIMEOUT tests, but mozilla CI runs those tests. Or we just agree to SKIP tests which consistently (or regularly timeout). This will make our test job much faster. In any case I think we should agree on this item before moving forward here.

Comment 2

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

I talked with Alex yesterday regarding the issue that we have seen on bug 1800257. He mentioned to me that they are using SKIP so that in CI these tests that would always timeout are not run. Each of them has a timeout of 25s which drastically would increase the overall execution time of the tests.

Given that we do not actively work on CDP maybe we want to keep the SKIP as well.

Comment 3

[Julian Descottes [:jdescottes]]

As discussed:

• we can SKIP timeout tests on our CI
• we will try to switch failing tests to FAIL and check that the runtime does not increase too much

Comment 4

[Alexandra Borovova [:Sasha]]

Attachment: Bug 1797723 - [puppeteer] Sync vendored puppeteer to v18.0.0.

Depends on D166650

Comment 5

[Alexandra Borovova [:Sasha]]

Attachment: Bug 1797723 - [puppeteer] Update vendor documentation.

Depends on D166651

Comment 6

[Alexandra Borovova [:Sasha]]

Decided to rather update puppeteer to version 18.0.0 to accommodate later changes to the custom mocha runner.

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/0fd435e2a18c
[puppeteer] Sync vendored puppeteer to v18.0.0. r=webdriver-reviewers,whimboo,jdescottes
https://hg.mozilla.org/integration/autoland/rev/6a92e4cb9e3d
[puppeteer] Update vendor documentation. r=webdriver-reviewers,whimboo

Comment 8

[Marian-Vasile Laza]

https://hg.mozilla.org/mozilla-central/rev/0fd435e2a18c
https://hg.mozilla.org/mozilla-central/rev/6a92e4cb9e3d