Closed Bug 1693763 Opened 7 months ago Closed 5 months ago

Re-enable CI runs for Windows on Puppeteer repository

Categories

(Remote Protocol :: CDP, enhancement, P2)

All
Windows
enhancement
Points:
2

Tracking

(firefox90 fixed)

RESOLVED FIXED
90 Branch
Tracking Status
firefox90 --- fixed

People

(Reporter: whimboo, Assigned: whimboo)

References

(Blocks 1 open bug)

Details

(Whiteboard: [bidi-m1-mvp])

Attachments

(1 file)

Currently Puppeteer doesn't run the unit tests on Windows due to problems with shutting down the browser. See bug 1656962. For getting out of the experimental status we should get the tests running again on Windows.

The following PR will make it happen:
https://github.com/puppeteer/puppeteer/pull/6895

Points: --- → 3
Priority: -- → P2
Points: 3 → 2

(In reply to Henrik Skupin (:whimboo) [⌚️UTC+1] from comment #0)

The following PR will make it happen:
https://github.com/puppeteer/puppeteer/pull/6895

The most recent patch works as expected and the PR is now under review.

Assignee: nobody → hskupin
Status: NEW → ASSIGNED
OS: Unspecified → Windows
Hardware: Unspecified → All
Version: Firefox 87 → unspecified

We won't need bug 1656962 to make it work.

No longer depends on: 1656962
Status: ASSIGNED → RESOLVED
Closed: 7 months ago
Resolution: --- → FIXED

I will reopen this bug to get the upstream change synced to mozilla-central given that it was not part of the v8.0.0 release.

Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Attachment #9212123 - Attachment description: WIP: Bug 1693763 - [remote] Downstream sync graceful browser shutdown. → Bug 1693763 - [remote] Downstream sync graceful browser shutdown.
Pushed by hskupin@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/4d29ba47da66
[remote] Downstream sync graceful browser shutdown. r=remote-protocol-reviewers,jdescottes
Status: REOPENED → RESOLVED
Closed: 7 months ago6 months ago
Resolution: --- → FIXED
Target Milestone: --- → 89 Branch
Regressions: 1701960
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Target Milestone: 89 Branch → ---

I investigated the failures for "Launcher specs Puppeteer Puppeteer.launch should be able to launch Firefox (launcher.spec.ts)"

From what I can tell, the test will pass as long as you first properly installed puppeteer, including the firefox dependency.
Run:

  • cd remote/test/puppeteer
  • PUPPETEER_PRODUCT=firefox npm i

After that the test passes successfully.
It's also something that puppeteer CI does: https://github.com/puppeteer/puppeteer/blob/main/.github/workflows/main.yml#L35

On try, we set the current path to the Firefox binary in executablePath to avoid downloading any binary.
But this test asserts await puppeteer.launch({ product: 'firefox' });, which is by definition not using executablePath (I think it needs to be explicitly passed via something like await puppeteer.launch({ executablePath });).

It seems to me that for the time being we should expect this launcher test to FAIL on Try.
Should we reland this patch, but change the expectation to FAIL?

Flags: needinfo?(hskupin)

There's a r+ patch which didn't land and no activity in this bug for 2 weeks.
:whimboo, could you have a look please?
For more information, please visit auto_nag documentation.

Flags: needinfo?(jdescottes)
Flags: needinfo?(hskupin)

(In reply to Julian Descottes [:jdescottes] from comment #9)

It seems to me that for the time being we should expect this launcher test to FAIL on Try.
Should we reland this patch, but change the expectation to FAIL?

Good find! And it sounds good. I will update the patch shortly. Would you mind to file a new bug for your findings?

Actually I made a mistake in the upstream patch:
https://github.com/puppeteer/puppeteer/blob/cf8c08d9919c5ab5247e23d55bbbc2ac4e48215f/test/launcher.spec.ts#L450

It should not be it but itOnlyRegularInstall similar to what is used for Chrome. In such a case this test will just be skipped.

Flags: needinfo?(jdescottes)
Flags: needinfo?(hskupin)
Pushed by hskupin@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/7d63f2fb2698
[remote] Downstream sync graceful browser shutdown. r=remote-protocol-reviewers,jdescottes
Status: REOPENED → RESOLVED
Closed: 6 months ago5 months ago
Resolution: --- → FIXED
Target Milestone: --- → 90 Branch
You need to log in before you can comment on or make changes to this bug.