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

✅ Fix flakey tests #184

Merged
merged 4 commits into from
Feb 10, 2021
Merged

✅ Fix flakey tests #184

merged 4 commits into from
Feb 10, 2021

Conversation

wwilsman
Copy link
Contributor

@wwilsman wwilsman commented Feb 9, 2021

What is this?

Some tests are flakey. This PR aims to unflake them all

  • @percy/dom frame serialization tests

    Sometimes a frame may take longer than expected to load. Even the JS only frame may load after modifying the frame document causing that test to flake. On windows, frames can take even longer to load than 3s, so the wait timeout was upped to 5s and the hook timeout was removed.

  • @percy/core capture handles closing during network idle

    Expected: Network error: Page closed
    Received: Protocol error (Runtime.callFunctionOn): Page closed

    Received message suggests that the page is sometimes closing either during the provided script execution or after the second network idle during DOM serialization script execution.

    It's possible for the test to continue after an asset has been accessed but before the request has been intercepted causing it to flake. Delaying when asset access is detected should help the request be intercepted in time for the assertion.

  • @percy/cli-exec start abort test

    The test sends the process quit signal after running the exec:start command, then issues a request to see if the server is down. The server is sometimes still up when that request is sent, but it should be on its way down.

    Sometimes it can take more than half of a second for the server to shutdown when forcefully terminating. Removing the arbitrary wait and making the test more deterministic should show if this was a test timing issue, or if termination itself is flakey.

  • @percy/core flakey coverage

    Coverage is sometimes missing from here. Not sure which test normally covers this, but a test was added that should always cover this as long as it passes.

Sometimes a frame may take longer than expected to load. Even the JS only frame may load after
modifying the frame document causing that test to flake. On windows, frames can take even longer to
load than 3s, so the wait timeout was upped to 5s and the hook timeout was removed.
It's possible for the test to continue after an asset has been accessed but before the request has
been intercepted causing it to flake. Delaying when asset access is detected should help the
request be intercepted in time for the assertion.
Sometimes it can take more than half of a second for the server to shutdown when forcefully
terminating. Removing the arbitrary wait and making the test more deterministic should show if this
was a test timing issue, or if termination itself is flakey.
@wwilsman wwilsman force-pushed the ww/fix-flakes-attempt-1 branch from c1dc6f2 to a1512bd Compare February 9, 2021 23:31
await expect(
fetch('http://localhost:5338/percy/healthcheck', { timeout: 10 })
).rejects.toThrow();
// check a few times rather than wait on a timeout to be deterministic
Copy link
Contributor

Choose a reason for hiding this comment

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

connnveerrrggeeeeee

@wwilsman wwilsman marked this pull request as ready for review February 10, 2021 16:39
@wwilsman
Copy link
Contributor Author

Re-ran latest CI 4-5 times and it didn't fail. This looks good to go.

inb4 it fails after merging

@wwilsman wwilsman requested a review from Robdel12 February 10, 2021 16:40
Copy link
Contributor

@Robdel12 Robdel12 left a comment

Choose a reason for hiding this comment

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

🏁 🐛 🔨

@wwilsman wwilsman merged commit de0ea99 into master Feb 10, 2021
@wwilsman wwilsman deleted the ww/fix-flakes-attempt-1 branch February 10, 2021 17:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants