Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Switch Puppeteer tests from CDP to WebDriver BiDi #17172

Merged
merged 2 commits into from
Dec 8, 2023

Conversation

whimboo
Copy link
Contributor

@whimboo whimboo commented Oct 25, 2023

Over the last months we have made great progress in implementing WebDriver BiDi in both Firefox and Chrome. Also Puppeteer uses already a lot of the new APIs when the webDriverBiDi protocol is enabled.

It would be good to see if pdf.js Puppeteer tests would already be in a position so that the underlying protocol can be changed to an officially spec'ed one. CC @marco-c.

All the testing commands by the bot can be found at #11851 (comment).

I'm going to use this PR to check which APIs might still be missing or failing.

Internally we are going to use this meta bug to track the remaining work on the WebDriver BiDi side:
https://bugzilla.mozilla.org/show_bug.cgi?id=1860992

@whimboo whimboo marked this pull request as draft October 25, 2023 08:40
@calixteman
Copy link
Contributor

/botio test

@moz-tools-bot
Copy link
Collaborator

From: Bot.io (Linux m4)


Received

Command cmd_test from @calixteman received. Current queue size: 0

Live output at: http://54.241.84.105:8877/c321a5329bf3279/output.txt

@moz-tools-bot
Copy link
Collaborator

From: Bot.io (Windows)


Received

Command cmd_test from @calixteman received. Current queue size: 1

Live output at: http://54.193.163.58:8877/cbf9641d3f42e20/output.txt

@moz-tools-bot
Copy link
Collaborator

From: Bot.io (Linux m4)


Failed

Full output at http://54.241.84.105:8877/c321a5329bf3279/output.txt

Total script time: 45.14 mins

  • Font tests: Passed
  • Unit tests: Passed
  • Integration Tests: FAILED
  • Regression tests: FAILED

Image differences available at: http://54.241.84.105:8877/c321a5329bf3279/reftest-analyzer.html#web=eq.log

@moz-tools-bot
Copy link
Collaborator

From: Bot.io (Windows)


Failed

Full output at http://54.193.163.58:8877/cbf9641d3f42e20/output.txt

Total script time: 60.00 mins

@whimboo
Copy link
Contributor Author

whimboo commented Oct 25, 2023

/botio test

@whimboo
Copy link
Contributor Author

whimboo commented Oct 25, 2023

/botio integrationtest

@moz-tools-bot
Copy link
Collaborator

From: Bot.io (Linux m4)


Received

Command cmd_integrationtest from @whimboo received. Current queue size: 0

Live output at: http://54.241.84.105:8877/4fba339dba81846/output.txt

@moz-tools-bot
Copy link
Collaborator

From: Bot.io (Windows)


Received

Command cmd_integrationtest from @whimboo received. Current queue size: 0

Live output at: http://54.193.163.58:8877/2fa84486562b274/output.txt

@moz-tools-bot
Copy link
Collaborator

From: Bot.io (Windows)


Failed

Full output at http://54.193.163.58:8877/2fa84486562b274/output.txt

Total script time: 37.89 mins

  • Integration Tests: FAILED

@timvandermeij
Copy link
Contributor

Thank you for doing this, and good to see that WebdriverBidi is making good progress!

@whimboo
Copy link
Contributor Author

whimboo commented Oct 30, 2023

I want to give a quick update. All the failures that I noticed should be fixed now and available via a recent Firefox release, and an up-to-date checkout of this repository. Only for Puppeteer we have to await a new release for the outstanding issue. Once that's done I'll try again if switching to BiDi will work across all platforms. I'm hoping for that this will happen during this week.

We should still check if the tests will work with Chrome as well over BiDi.

@timvandermeij
Copy link
Contributor

timvandermeij commented Nov 12, 2023

@whimboo FYI, in case you want to give this another try, the Puppeteer fixes for puppeteer/puppeteer#11259 and puppeteer/puppeteer#11298 should be available in version 21.5.1, and yesterday we updated to that version here in 8e2c9a3 so it can be used if this patch is rebased.

@calixteman
Copy link
Contributor

@whimboo I just tested on mac and few tests are failing with Firefox:

FTEST-UNEXPECTED-FAIL | must check that we've all the contents on copy/paste
FTEST-UNEXPECTED-FAIL | must check that text change can be undone/redone
FTEST-UNEXPECTED-FAIL | must update an existing annotation
FTEST-UNEXPECTED-FAIL | must move an annotation
FTEST-UNEXPECTED-FAIL | must not remove an empty annotation
FTEST-UNEXPECTED-FAIL | must open an existing annotation and check that the position are good
FTEST-UNEXPECTED-FAIL | must open an existing rotated annotation and check that the position are good
FTEST-UNEXPECTED-FAIL | must check the text can be selected with the mouse
FTEST-UNEXPECTED-FAIL | must check the keyboard event is limited to the input

@whimboo
Copy link
Contributor Author

whimboo commented Nov 14, 2023

To give a brief summary we have two more issues for dblclick and tripleclick in our BiDi implementation. We are going to work on both soon to get it properly supported.

@whimboo
Copy link
Contributor Author

whimboo commented Dec 1, 2023

The double and triple click issues are now fixed in Firefox Nightly. But there is now one remaining issue with a test of the page number input which fails to wait for the keyup event. We are working on it via https://bugzilla.mozilla.org/show_bug.cgi?id=1867523.

@whimboo
Copy link
Contributor Author

whimboo commented Dec 7, 2023

The double and triple click issues are now fixed in Firefox Nightly. But there is now one remaining issue with a test of the page number input which fails to wait for the keyup event. We are working on it via https://bugzilla.mozilla.org/show_bug.cgi?id=1867523.

Actually this is an issue with Puppeteer, which is fixed but hasn't shipped yet, and as such for now would require a workaround in the pdf.js tests. We will have to wait landing this change until #17387 has been merged.

Nevertheless I'll update this PR now so that it is ready for getting merged.

@whimboo
Copy link
Contributor Author

whimboo commented Dec 7, 2023

/botio integrationtest

@moz-tools-bot
Copy link
Collaborator

From: Bot.io (Linux m4)


Received

Command cmd_integrationtest from @whimboo received. Current queue size: 0

Live output at: http://54.241.84.105:8877/faef049b9ac2539/output.txt

@moz-tools-bot
Copy link
Collaborator

From: Bot.io (Windows)


Received

Command cmd_integrationtest from @whimboo received. Current queue size: 0

Live output at: http://54.193.163.58:8877/f4e0d8e9d9153c9/output.txt

@whimboo whimboo marked this pull request as ready for review December 7, 2023 09:50
@moz-tools-bot
Copy link
Collaborator

From: Bot.io (Linux m4)


Failed

Full output at http://54.241.84.105:8877/faef049b9ac2539/output.txt

Total script time: 39.09 mins

  • Integration Tests: FAILED

@moz-tools-bot
Copy link
Collaborator

From: Bot.io (Windows)


Failed

Full output at http://54.193.163.58:8877/f4e0d8e9d9153c9/output.txt

Total script time: 55.17 mins

  • Integration Tests: FAILED

@whimboo
Copy link
Contributor Author

whimboo commented Dec 7, 2023

There are lot of issues with timeouts calling browsingContext.close. I'm not able to reproduce that locally, so I pushed an update which enables trace logs for the remote agent. I hope it will help to investigate the problem.

@whimboo
Copy link
Contributor Author

whimboo commented Dec 7, 2023

/botio integrationtest

@moz-tools-bot
Copy link
Collaborator

From: Bot.io (Linux m4)


Received

Command cmd_integrationtest from @whimboo received. Current queue size: 0

Live output at: http://54.241.84.105:8877/33e8bee5181a907/output.txt

@moz-tools-bot
Copy link
Collaborator

From: Bot.io (Windows)


Received

Command cmd_integrationtest from @whimboo received. Current queue size: 1

Live output at: http://54.193.163.58:8877/f31a87d7136f189/output.txt

@moz-tools-bot
Copy link
Collaborator

From: Bot.io (Linux m4)


Success

Full output at http://54.241.84.105:8877/520c42f4365ee9d/output.txt

Total script time: 5.57 mins

  • Integration Tests: Passed

@moz-tools-bot
Copy link
Collaborator

From: Bot.io (Linux m4)


Success

Full output at http://54.241.84.105:8877/00e2160b3b4e762/output.txt

Total script time: 5.75 mins

  • Integration Tests: Passed

@moz-tools-bot
Copy link
Collaborator

From: Bot.io (Windows)


Failed

Full output at http://54.193.163.58:8877/fbec175849968b4/output.txt

Total script time: 15.80 mins

  • Integration Tests: FAILED

@moz-tools-bot
Copy link
Collaborator

From: Bot.io (Windows)


Failed

Full output at http://54.193.163.58:8877/f31a87d7136f189/output.txt

Total script time: 14.64 mins

  • Integration Tests: FAILED

@whimboo
Copy link
Contributor Author

whimboo commented Dec 8, 2023

@Snuffleupagus could you maybe help us with the upgrade of Puppeteer? Due to broken depencencies I'm not able to do that from a M1 MBP.

@whimboo
Copy link
Contributor Author

whimboo commented Dec 8, 2023

/botio integrationtest

@moz-tools-bot
Copy link
Collaborator

From: Bot.io (Linux m4)


Received

Command cmd_integrationtest from @whimboo received. Current queue size: 0

Live output at: http://54.241.84.105:8877/d4734d31a00af40/output.txt

@moz-tools-bot
Copy link
Collaborator

From: Bot.io (Windows)


Received

Command cmd_integrationtest from @whimboo received. Current queue size: 0

Live output at: http://54.193.163.58:8877/58d578eeb8d35c2/output.txt

@moz-tools-bot
Copy link
Collaborator

From: Bot.io (Linux m4)


Success

Full output at http://54.241.84.105:8877/d4734d31a00af40/output.txt

Total script time: 5.69 mins

  • Integration Tests: Passed

@moz-tools-bot
Copy link
Collaborator

From: Bot.io (Windows)


Failed

Full output at http://54.193.163.58:8877/58d578eeb8d35c2/output.txt

Total script time: 16.96 mins

  • Integration Tests: FAILED

@whimboo
Copy link
Contributor Author

whimboo commented Dec 8, 2023

/botio integrationtest

@moz-tools-bot
Copy link
Collaborator

From: Bot.io (Linux m4)


Received

Command cmd_integrationtest from @whimboo received. Current queue size: 0

Live output at: http://54.241.84.105:8877/921433b493e40e1/output.txt

@moz-tools-bot
Copy link
Collaborator

From: Bot.io (Windows)


Received

Command cmd_integrationtest from @whimboo received. Current queue size: 0

Live output at: http://54.193.163.58:8877/0bba05088306154/output.txt

@moz-tools-bot
Copy link
Collaborator

From: Bot.io (Linux m4)


Success

Full output at http://54.241.84.105:8877/921433b493e40e1/output.txt

Total script time: 5.75 mins

  • Integration Tests: Passed

@moz-tools-bot
Copy link
Collaborator

From: Bot.io (Windows)


Success

Full output at http://54.193.163.58:8877/0bba05088306154/output.txt

Total script time: 13.89 mins

  • Integration Tests: Passed

@calixteman
Copy link
Contributor

/botio test

@moz-tools-bot
Copy link
Collaborator

From: Bot.io (Linux m4)


Received

Command cmd_test from @calixteman received. Current queue size: 0

Live output at: http://54.241.84.105:8877/36c27c4b63b6553/output.txt

@moz-tools-bot
Copy link
Collaborator

From: Bot.io (Windows)


Received

Command cmd_test from @calixteman received. Current queue size: 0

Live output at: http://54.193.163.58:8877/89075c4b5130fef/output.txt

@@ -1056,9 +1056,6 @@ async function closeSession(browser) {
continue;
}
if (session.browser !== undefined) {
for (const page of await session.browser.pages()) {
await page.close();
}
await session.browser.close();
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@OrKoN this actually fixes an issue with browsingContext.close() in puppeteer with Firefox when the context as passed into the BiDi call is null. Could this be a bug in Puppeteer? Or will some internal state change for other tabs as well when trying to close them all via such a loop?

@moz-tools-bot
Copy link
Collaborator

From: Bot.io (Linux m4)


Failed

Full output at http://54.241.84.105:8877/36c27c4b63b6553/output.txt

Total script time: 24.23 mins

  • Unit tests: Passed
  • Integration Tests: Passed
  • Regression tests: FAILED
  different ref/snapshot: 16
  different first/second rendering: 2

Image differences available at: http://54.241.84.105:8877/36c27c4b63b6553/reftest-analyzer.html#web=eq.log

@moz-tools-bot
Copy link
Collaborator

From: Bot.io (Windows)


Failed

Full output at http://54.193.163.58:8877/89075c4b5130fef/output.txt

Total script time: 39.30 mins

  • Unit tests: Passed
  • Integration Tests: Passed
  • Regression tests: FAILED
  errors: 1
  different ref/snapshot: 25

Image differences available at: http://54.193.163.58:8877/89075c4b5130fef/reftest-analyzer.html#web=eq.log

Copy link
Contributor

@calixteman calixteman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thank you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants