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

[test] Wait on e2e server to start before starting runner #25476

Merged
merged 1 commit into from Mar 24, 2021

Conversation

eps1lon
Copy link
Member

@eps1lon eps1lon commented Mar 24, 2021

Fixes https://app.circleci.com/pipelines/github/mui-org/material-ui/39638/workflows/d07cf743-eb36-4a71-b6a3-74400199ac2c/jobs/234518

We start server and runner concurrently but we didn't actually wait on the server. The regression suite did not have problems with this. Probably because starting the headful browser was enough time for the server to start.

@eps1lon eps1lon added the test label Mar 24, 2021
@mui-pr-bot
Copy link

mui-pr-bot commented Mar 24, 2021

No bundle size changes

Generated by 🚫 dangerJS against 02b98a8

@eps1lon eps1lon marked this pull request as ready for review March 24, 2021 09:55
* @param url
*/
async function attemptGoto(page: playwright.Page, url: string): Promise<boolean> {
const maxAttempts = 10;
Copy link
Member

Choose a reason for hiding this comment

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

Why have a max attempt if we already have a timeout on the before task?

Copy link
Member

Choose a reason for hiding this comment

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

To answer my own question, to get an accurate error message.

Copy link
Member Author

@eps1lon eps1lon Mar 24, 2021

Choose a reason for hiding this comment

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

Because the timeout does not necessarily translate to the number of attempts considering we await other async tasks before. The maxAttempts is just something to statically verify the loop doesn't run forever.

For example, launching the browser might take 15s. Then we don't ever reach maxAttempts nor shouldn't we.

@eps1lon eps1lon merged commit 5fc4575 into mui:next Mar 24, 2021
@eps1lon eps1lon deleted the test/wait-on branch March 24, 2021 15:52
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.

None yet

3 participants