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

[7.x] [Uptime] [Test] Repurpose unit test assertions to avoid flakiness (#40650) #42977

Merged
merged 5 commits into from
Sep 4, 2019

Conversation

andrewvc
Copy link
Contributor

@andrewvc andrewvc commented Aug 8, 2019

Backports the following commits to 7.x:

…astic#40650)

I had previously revised these tests to utilize Jest's toBeCloseTo assertion, because it provides a precision parameter. My understanding when I implemented this test was that it would check n significant digits, so if I ran code like expect(myValue).toBeCloseTo(36000, 3), a value like 36015 would be accepted by the assertion. After seeing this test fail in a flaky way while testing other changes (it failed when myValue === 36001), I researched the function more closely and found that it does not behave this way.

This led me to opt for the current solution, which should help avoid any flakiness issues while continuing to rely on the end-user simplicity of a snapshot. I do not expect a variance greater than 1 when running these tests, so the threshold of 100 should be sufficient to consider these tests stable.
@andrewvc andrewvc added backport Team:Uptime - DEPRECATED Synthetics & RUM sub-team of Application Observability labels Aug 8, 2019
@elasticmachine
Copy link
Contributor

Pinging @elastic/uptime

@elasticmachine
Copy link
Contributor

💔 Build Failed

@andrewvc
Copy link
Contributor Author

jenkins, retest this please

@elasticmachine
Copy link
Contributor

💔 Build Failed

@andrewvc
Copy link
Contributor Author

jenkins, retest this please

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@justinkambic
Copy link
Contributor

@andrewvc it looks like we're passing CI but you should perform a rebase against 7.x so the PR is no longer outdated.

@elasticmachine
Copy link
Contributor

💔 Build Failed

@andrewvc
Copy link
Contributor Author

jenkins, retest this please

@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💔 Build Failed

@nik9000
Copy link
Member

nik9000 commented Aug 29, 2019

@elasticmachine, run elasticsearch-ci/docs

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@andrewvc andrewvc merged commit 307e002 into elastic:7.x Sep 4, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport Team:Uptime - DEPRECATED Synthetics & RUM sub-team of Application Observability
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants