Skip to content

[Uptime] Fix flaky unit test snapshot#46492

Merged
justinkambic merged 14 commits intoelastic:masterfrom
justinkambic:uptime_fuzzy-test-snapshot
Oct 1, 2019
Merged

[Uptime] Fix flaky unit test snapshot#46492
justinkambic merged 14 commits intoelastic:masterfrom
justinkambic:uptime_fuzzy-test-snapshot

Conversation

@justinkambic
Copy link
Contributor

Summary

Resolves #46217.

In #40650 we added a helper function to avoid this same problem in other tests. There is a computed value that can sometimes be off by one. This does not impact performance or pose any problem in practice, but when performing snapshot comparisons it can be problematic.

This patch extracts that helper function, adds a test for it, and updates the existing unit test to avoid this flakiness.

Checklist

Use strikethroughs to remove checklist items you don't feel are applicable to this PR.

For maintainers

@justinkambic justinkambic added v8.0.0 Team:Uptime - DEPRECATED Synthetics & RUM sub-team of Application Observability release_note:skip Skip the PR/issue when compiling release notes v7.5.0 test-failure-flaky labels Sep 24, 2019
@justinkambic justinkambic self-assigned this Sep 24, 2019
@elasticmachine
Copy link
Contributor

Pinging @elastic/uptime

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@elasticmachine
Copy link
Contributor

💔 Build Failed

@justinkambic
Copy link
Contributor Author

@elasticmachine merge upstream

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@justinkambic
Copy link
Contributor Author

@elasticmachine merge upstream

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@spalger spalger added the test-matrix Use this label to ensure PRs are tested with matrix jobs label Sep 25, 2019
@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@spalger
Copy link
Contributor

spalger commented Sep 26, 2019

@elasticmachine merge upstream

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@spalger
Copy link
Contributor

spalger commented Sep 26, 2019

@elasticmachine merge upstream

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@justinkambic
Copy link
Contributor Author

@spalger do you think we should run this through again? And will you remove your additions from the diff when you think we're good? Thanks 👍

@justinkambic
Copy link
Contributor Author

@elasticmachine merge upstream

Copy link
Contributor

@shahzad31 shahzad31 left a comment

Choose a reason for hiding this comment

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

Looks good , i don't understand jobs.yml file diff, rest looks good !!

*/

import { assertCloseTo } from '../assert_close_to';

Copy link
Contributor

Choose a reason for hiding this comment

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

i like tests for a test helped function, it's like nice little inception going on here ;)

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@spalger spalger removed the test-matrix Use this label to ensure PRs are tested with matrix jobs label Oct 1, 2019
Copy link
Contributor

@spalger spalger left a comment

Choose a reason for hiding this comment

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

LGTM

@elasticmachine
Copy link
Contributor

💔 Build Failed

@justinkambic
Copy link
Contributor Author

@elasticmachine merge upstream

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@justinkambic justinkambic merged commit 29c5d67 into elastic:master Oct 1, 2019
justinkambic added a commit that referenced this pull request Oct 4, 2019
* Extract test helper function for reuse.

* Modify unit test to avoid flaky behavior.

* Fix outputted error string in test helper.

* Add test file for test helper function.

* run x-pack-intake job 40 times

* Revert "run x-pack-intake job 40 times"

This reverts commit 53b520c.
@justinkambic
Copy link
Contributor Author

justinkambic commented Oct 4, 2019

Backported to:
7.x/7.5.0 0ae39fc
#47309

@justinkambic justinkambic deleted the uptime_fuzzy-test-snapshot branch October 14, 2019 16:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

release_note:skip Skip the PR/issue when compiling release notes Team:Uptime - DEPRECATED Synthetics & RUM sub-team of Application Observability test-failure-flaky v7.5.0 v8.0.0

Projects

None yet

5 participants