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.

Sign up for GitHub

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

Jump to bottom

Fix flaky ui_settings integration tests #92028

Merged
merged 4 commits into from
Feb 22, 2021
Merged

Fix flaky ui_settings integration tests #92028

merged 4 commits into from
Feb 22, 2021

Conversation

afharo
Copy link
Member

@afharo afharo commented Feb 19, 2021 •
edited
Loading

Summary

  • Starting off by unskipping them

Resolves #89191

Checklist

Delete any items that are not applicable to this PR.

For maintainers

@afharo afharo added release_note:skip Skip the PR/issue when compiling release notes v7.13.0 labels Feb 19, 2021
@afharo afharo requested a review from a team as a code owner February 19, 2021 15:34
TinaHeiligers
TinaHeiligers reviewed Feb 19, 2021
View reviewed changes
Copy link
Contributor

@TinaHeiligers TinaHeiligers left a comment

Choose a reason for hiding this comment

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

Do we know why they were failing in the first place?
They seem to have passed now here. If you haven't already, I'd prefer we make sure that the test passes every time on a number of runs using the flaky tet runner.

@afharo
Copy link
Member Author

afharo commented Feb 19, 2021

@elasticmachine merge upstream

Second run to see if it fails. After 3 successful runs, I'll merge.

@TinaHeiligers, I might need some help with the flaky test runner. I think I've never used it before.

@afharo
Copy link
Member Author

afharo commented Feb 22, 2021

@elasticmachine merge upstream

Third run to see if it fails. I'll also try the flaky-test runner, and if all good, I'll merge.

@afharo
Copy link
Member Author

afharo commented Feb 22, 2021

UPDATE: It's not a FTR suite. I'll follow @pgayvallet's suggestion then: #92030 (comment)

@afharo
Copy link
Member Author

afharo commented Feb 22, 2021

Result after 200 runs of the same suite: https://kibana-ci.elastic.co/job/elastic+kibana+pipeline-pull-request/108298/execution/node/465/log/?consoleFull, search for src/core/server/ui_settings/integration_tests/routes.test.ts. They all pass! 🎉

@afharo afharo enabled auto-merge (squash) February 22, 2021 12:02
@afharo afharo added the auto-backport Deprecated - use backport:version if exact versions are needed label Feb 22, 2021
@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

✅ unchanged

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

TinaHeiligers
TinaHeiligers approved these changes Feb 22, 2021
View reviewed changes
Copy link
Contributor

@TinaHeiligers TinaHeiligers left a comment

Choose a reason for hiding this comment

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

LGTM

@afharo afharo merged commit 5c07fff into elastic:master Feb 22, 2021
@kibanamachine kibanamachine mentioned this pull request Feb 22, 2021
Merged
kibanamachine added a commit to kibanamachine/kibana that referenced this pull request Feb 22, 2021
@afharo @kibanamachine
Fix flaky ui_settings integration tests (elastic#92028)
b1dd3ee 
* Unskip ui_settings integration tests

* Run 200 times the test to ensure it does not fail

* Revert 200 execution loop

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
@kibanamachine
Copy link
Contributor

💚 Backport successful

✅ 7.x / #92232

Successful backport PRs will be merged automatically after passing CI.

@afharo afharo deleted the unskip_ui_settings_tests branch February 22, 2021 16:48
kibanamachine added a commit that referenced this pull request Feb 22, 2021
@kibanamachine @afharo
Fix flaky ui_settings integration tests (#92028) (#92232)
bbe3417 
* Unskip ui_settings integration tests

* Run 200 times the test to ensure it does not fail

* Revert 200 execution loop

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>

Co-authored-by: Alejandro Fernández Haro <alejandro.haro@elastic.co>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@TinaHeiligers TinaHeiligers TinaHeiligers approved these changes

Assignees
No one assigned
Labels
auto-backport Deprecated - use backport:version if exact versions are needed release_note:skip Skip the PR/issue when compiling release notes v7.13.0
Projects
None yet
Milestone
No milestone
3 participants
@afharo @kibanamachine @TinaHeiligers