Skip to content

Fix flaky core_app integration tests#92030

Merged
afharo merged 6 commits intoelastic:masterfrom
afharo:unskip_core_app_tests
Feb 22, 2021
Merged

Fix flaky core_app integration tests#92030
afharo merged 6 commits intoelastic:masterfrom
afharo:unskip_core_app_tests

Conversation

@afharo
Copy link
Copy Markdown
Member

@afharo afharo commented Feb 19, 2021

Summary

  • Starting off by unskipping them

Resolves #81072

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:49
@afharo
Copy link
Copy Markdown
Member Author

afharo commented Feb 19, 2021

@elasticmachine merge upstream

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

@Bamieh
Copy link
Copy Markdown
Contributor

Bamieh commented Feb 22, 2021

@elasticmachine merge upstream

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

@Bamieh Bamieh left a comment

Choose a reason for hiding this comment

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

approving to unblock in case it gets 3 successive passes.

@pgayvallet
Copy link
Copy Markdown
Contributor

pgayvallet commented Feb 22, 2021

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

Imho 3 green runs is far from being good enough to unskip a flaky suite (I mean, it's flaky, not red, 3 runs will likely succeed if the flakiness rate is < 10%, which is most likely the case here)

I usually fork the branch, edit the CI group tests to have only this specific test running, then use the flaky runner on 200+ iterations.

EDIT: this is not a FTR test. Well in that case, I would edit the IT file to have the test run 200 times, and then revert the commit if/when green.

@afharo afharo mentioned this pull request Feb 22, 2021
Merged
9 tasks
@afharo
Copy link
Copy Markdown
Member Author

afharo commented Feb 22, 2021

Good call @pgayvallet! I've added a loop to run the suite 200 times. It forced me to move some of the vars and setup steps inside the describe context. Probably that was the cause of the flaky tests.

@afharo
Copy link
Copy Markdown
Member Author

afharo commented Feb 22, 2021

The tests passed 200 times in a row after the fixes mentioned in my previous comment: search src/core/server/core_app/integration_tests/default_route_provider_config.test.ts in the logs of the CI job to see them.

I'll go ahead and revert the 200 loop.

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

kibanamachine commented Feb 22, 2021

💚 Build Succeeded

Metrics [docs]

✅ unchanged

History

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

@afharo afharo merged commit 678f848 into elastic:master Feb 22, 2021
@afharo afharo deleted the unskip_core_app_tests branch February 22, 2021 14:52
kibanamachine added a commit to kibanamachine/kibana that referenced this pull request Feb 22, 2021
@afharo @kibanamachine
Fix flaky core_app integration tests (elastic#92030)
29932f5 
* Unskip core_app integration tests

* Run the integration test 200 times

* Revert 200 execution loop

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
@kibanamachine kibanamachine mentioned this pull request Feb 22, 2021
Merged
@kibanamachine
Copy link
Copy Markdown
Contributor

kibanamachine commented Feb 22, 2021

💚 Backport successful

✅ 7.x / #92190

Successful backport PRs will be merged automatically after passing CI.

kibanamachine added a commit that referenced this pull request Feb 22, 2021
@kibanamachine @afharo
Fix flaky core_app integration tests (#92030) (#92190)
93ca371 
* Unskip core_app integration tests

* Run the integration test 200 times

* 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

@Bamieh Bamieh Bamieh 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

Development

Successfully merging this pull request may close these issues.

Failing test: Jest Integration Tests.src/core/server/core_app/integration_tests - default route provider consumes valid values

4 participants

@afharo @Bamieh @pgayvallet @kibanamachine