-
Notifications
You must be signed in to change notification settings - Fork 10k
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
Fix integration test 'PageOpen and PageClose actions' #15308
Conversation
/botio integrationtest |
From: Bot.io (Windows)ReceivedCommand cmd_integrationtest from @calixteman received. Current queue size: 0 Live output at: http://54.193.163.58:8877/c5e61c1b0739c07/output.txt |
From: Bot.io (Linux m4)ReceivedCommand cmd_integrationtest from @calixteman received. Current queue size: 0 Live output at: http://54.241.84.105:8877/c2b62f85a32a7ba/output.txt |
From: Bot.io (Windows)FailedFull output at http://54.193.163.58:8877/c5e61c1b0739c07/output.txt Total script time: 0.00 mins |
From: Bot.io (Linux m4)SuccessFull output at http://54.241.84.105:8877/c2b62f85a32a7ba/output.txt Total script time: 4.83 mins
|
/botio-windows integrationtest |
1 similar comment
/botio-windows integrationtest |
From: Bot.io (Windows)ReceivedCommand cmd_integrationtest from @calixteman received. Current queue size: 0 Live output at: http://54.193.163.58:8877/74814f393270ee0/output.txt |
From: Bot.io (Windows)FailedFull output at http://54.193.163.58:8877/74814f393270ee0/output.txt Total script time: 11.88 mins
|
This integration test sadly fails pretty much all the time now, but only on Windows somehow. Should we perhaps disable it on Windows only for the time being given that this PR doesn't seem to solve the issue? Note that there are more integration tests that can fail, but those are more intermittent. |
This PR likely fix an issue: I saw an exception when I ran the test in the vm. |
Oh, definitely. If the issue in this test is indeed composed of multiple issues, fixing one of them is already great! In that case we can indeed try this to at least hopefully decrease the frequency of the issue. |
/botio-windows integrationtest |
From: Bot.io (Windows)ReceivedCommand cmd_integrationtest from @timvandermeij received. Current queue size: 0 Live output at: http://54.193.163.58:8877/6337ca2fce090d3/output.txt |
From: Bot.io (Windows)FailedFull output at http://54.193.163.58:8877/6337ca2fce090d3/output.txt Total script time: 9.71 mins
|
/botio-windows integrationtest |
From: Bot.io (Windows)ReceivedCommand cmd_integrationtest from @timvandermeij received. Current queue size: 1 Live output at: http://54.193.163.58:8877/e86f36571b96553/output.txt |
From: Bot.io (Windows)FailedFull output at http://54.193.163.58:8877/e86f36571b96553/output.txt Total script time: 9.96 mins
|
@@ -534,10 +534,15 @@ describe("Interaction", () => { | |||
page, | |||
getSelector(refOpen), | |||
async () => { | |||
const sel = getSelector(refOpen); | |||
await page.waitForSelector(sel, { | |||
timeout: 0, |
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.
Is this safe in the sense that if this selector would not trigger we would basically be waiting here forever? Or is there a global timeout on the Jasmine tests or so?
If the comment above is answered, I'm fine with merging this if it fixes an exception you encountered, but in that case I'd propose we reword the commit message to |
@calixteman Ping for the above so we can hopefully get this landed to at least improve the situation for this failing test. |
I'll take the time "asap" to fix this stuff. |
@calixteman Gentle ping for this; if it's still useful it'd be nice to get this merged given the many integration test failures we have. |
No description provided.