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
Use BiDi protocol for Chrome tests #17962
base: master
Are you sure you want to change the base?
Conversation
/botio integrationtest |
From: Bot.io (Linux m4)ReceivedCommand cmd_integrationtest from @calixteman received. Current queue size: 0 Live output at: http://54.241.84.105:8877/395e307c994cbca/output.txt |
From: Bot.io (Windows)ReceivedCommand cmd_integrationtest from @calixteman received. Current queue size: 0 Live output at: http://54.193.163.58:8877/aead5c6b2e4df46/output.txt |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll have a detailed look at this tomorrow, but in the meantime here are my first observations.
await Promise.all( | ||
pages.map(async ([browserName, page]) => { | ||
if (process.platform === "win32" && browserName === "firefox") { | ||
pending("Disabled in Firefox on Windows, because of bug 1662471."); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's really nice that we can now actually run this test everywhere now! Would it perhaps be good to keep a reference to the mentioned bug in the new comment you added, to prevent someone from changing this back later until the bug is fixed upstream?
@@ -2354,14 +2354,14 @@ describe("FreeText Editor", () => { | |||
`${getEditorSelector(1)} .overlay.enabled` | |||
); | |||
|
|||
rect = await page.$eval(getEditorSelector(0), el => { | |||
rect = await page.$eval(getEditorSelector(1), el => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you explain a bit how the changes in this file fix the issue? Looking at this test earlier I was a bit confused as to what this part of the test is achieving. Perhaps it could be resolved with more accompanying comments in the test to explain the intent of each "block" of logic. Because the test is quite lengthy that would also help with reading the test from top to bottom.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When taking the rect for editor 0 which now contains "A" and then clicking at rect.x+5rect.width,rect.y+5rect.height doesn't guarantee to be on the second editor.
I probably copy/paste some code without thinking about this.
From: Bot.io (Linux m4)SuccessFull output at http://54.241.84.105:8877/395e307c994cbca/output.txt Total script time: 7.67 mins
|
From: Bot.io (Windows)SuccessFull output at http://54.193.163.58:8877/aead5c6b2e4df46/output.txt Total script time: 18.50 mins
|
/botio test |
From: Bot.io (Linux m4)ReceivedCommand cmd_test from @calixteman received. Current queue size: 0 Live output at: http://54.241.84.105:8877/9e74d8b32fd9a8a/output.txt |
From: Bot.io (Windows)ReceivedCommand cmd_test from @calixteman received. Current queue size: 0 Live output at: http://54.193.163.58:8877/92a94d4274292a5/output.txt |
From: Bot.io (Windows)FailedFull output at http://54.193.163.58:8877/92a94d4274292a5/output.txt Total script time: 5.37 mins
Image differences available at: http://54.193.163.58:8877/92a94d4274292a5/reftest-analyzer.html#web=eq.log |
From: Bot.io (Linux m4)FailedFull output at http://54.241.84.105:8877/9e74d8b32fd9a8a/output.txt Total script time: 13.53 mins
Image differences available at: http://54.241.84.105:8877/9e74d8b32fd9a8a/reftest-analyzer.html#web=eq.log |
@OrKoN the ref tests failed on the linux bot because of an OOM crash. The bot has a 64 bits OS with 16Gb of RAM and afaik we never had this kind of issues with CDP in the last years. |
/botio test |
From: Bot.io (Linux m4)ReceivedCommand cmd_test from @calixteman received. Current queue size: 0 Live output at: http://54.241.84.105:8877/17e8528589e75e3/output.txt |
From: Bot.io (Windows)ReceivedCommand cmd_test from @calixteman received. Current queue size: 0 Live output at: http://54.193.163.58:8877/81255050207bcff/output.txt |
From: Bot.io (Linux m4)FailedFull output at http://54.241.84.105:8877/17e8528589e75e3/output.txt Total script time: 9.94 mins
Image differences available at: http://54.241.84.105:8877/17e8528589e75e3/reftest-analyzer.html#web=eq.log |
@calixteman do all tests use Puppeteer or only integration? |
I think I am not able to view the logs but I will give it a try later |
From: Bot.io (Windows)FailedFull output at http://54.193.163.58:8877/81255050207bcff/output.txt Total script time: 39.92 mins
Image differences available at: http://54.193.163.58:8877/81255050207bcff/reftest-analyzer.html#web=eq.log |
The unit/ref/integration tests are using Puppeteer. |
You should be able to just look at http://54.193.163.58:8877/81255050207bcff/output.txt |
@OrKoN The logs of the runs that crashed due to OOM can be found at http://54.241.84.105:8877/9e74d8b32fd9a8a/output.txt for the first try and at http://54.241.84.105:8877/17e8528589e75e3/output.txt for the second try. Both crashed during the reference tests. For completeness I'll also include the relevant bit here (the
|
/botio-linux test |
From: Bot.io (Linux m4)ReceivedCommand cmd_test from @calixteman received. Current queue size: 0 Live output at: http://54.241.84.105:8877/4340ac09990c704/output.txt |
From: Bot.io (Linux m4)FailedFull output at http://54.241.84.105:8877/4340ac09990c704/output.txt Total script time: 9.37 mins
Image differences available at: http://54.241.84.105:8877/4340ac09990c704/reftest-analyzer.html#web=eq.log |
Would it perhaps be an idea to pull the test improvements out into a separate PR so we can already land them while we're figuring out what to do with the Chrome/WebDriver BiDi OOM bot issue? It would make this PR smaller and we can then already benefit from the improved code, and run the previously disabled test on Windows. Together with the |
The Puppeteer update should in particular be helpful for us because it contains improved WebDriver BiDi compatibility, a newer Chrome version (both might help for mozilla#17962) and an official deprecation of CDP for Firefox. Note that the latter doesn't require changes on our end because we already use WebDriver BiDi unconditionally for Firefox since commit 4db0174. The full release notes can be found at https://github.com/puppeteer/puppeteer/releases/tag/puppeteer-core-v22.8.0.
No description provided.