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 #20226 : Navigation timeout of 30000 ms in click-all-buttons-on-about-foundation-page #20279
Conversation
Hi @Akhilesh-max, can you complete the following:
|
Hi @Akhilesh-max please assign the required reviewer(s) for this PR. Thanks! |
@@ -401,6 +402,9 @@ export class LoggedInUser extends BaseUser { | |||
await this.page.$eval(weCannotContentId, element => | |||
element.getElementsByTagName('a')[0].click() | |||
); |
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.
This test is intermittently failing due to a race condition between two navigations. After clicking the '420 million' link, the test is not waiting for the resulting navigation to complete before the beforeEach block started a new navigation. This lefts the first navigation unresolved, which sometimes leads to failures.
Despite the test logging the final showMessage, it is not properly handling the navigation it initiated.
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.
Great find, @Akhilesh-max!
@@ -401,6 +402,9 @@ export class LoggedInUser extends BaseUser { | |||
await this.page.$eval(weCannotContentId, element => | |||
element.getElementsByTagName('a')[0].click() | |||
); | |||
await this.page.waitForNavigation({waitUntil: ['load', 'networkidle2']}); | |||
await this.page.waitForSelector(_420millionPageHeaderSelector); | |||
|
|||
if (this.page.url() !== _420MillionUrl) { | |||
throw new Error('The 420 Million link does not open the right page!'); |
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 suggest printing the incorrect URL here in this error message (and in other similar error messages) -- it would help with debugging.
@@ -401,6 +402,9 @@ export class LoggedInUser extends BaseUser { | |||
await this.page.$eval(weCannotContentId, element => | |||
element.getElementsByTagName('a')[0].click() | |||
); | |||
await this.page.waitForNavigation({waitUntil: ['load', 'networkidle2']}); | |||
await this.page.waitForSelector(_420millionPageHeaderSelector); |
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.
Do you actually need this line, or is the previous one alone sufficient?
@@ -401,6 +402,9 @@ export class LoggedInUser extends BaseUser { | |||
await this.page.$eval(weCannotContentId, element => | |||
element.getElementsByTagName('a')[0].click() | |||
); |
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.
Great find, @Akhilesh-max!
Hi @Akhilesh-max, I'm going to mark this PR as stale because it hasn't had any updates for 7 days. If no further activity occurs within 4 days, it will be automatically closed so that others can take up the issue. |
Overview
Essential Checklist
Please follow the instructions for making a code change.
Proof that changes are correct
PR Pointers