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

[Uptime] Fix overview flaky tests #99781

Merged
merged 6 commits into from
May 14, 2021
Merged

[Uptime] Fix overview flaky tests #99781

merged 6 commits into from
May 14, 2021

Conversation

dominiqueclarke
Copy link
Contributor

@dominiqueclarke dominiqueclarke commented May 11, 2021 •
edited
Loading

Summary

Fixes #57737
Relates to #97558
Relates to #98412

Fixes a series of flaky functional tests that were failing on finding the datepicker popover.

@dominiqueclarke dominiqueclarke added failed-test A test failure on a tracked branch, potentially flaky-test v8.0.0 Team:Uptime - DEPRECATED Synthetics & RUM sub-team of Application Observability skipped-test release_note:skip Skip the PR/issue when compiling release notes v7.14.0 v7.13.0 auto-backport Deprecated - use backport:version if exact versions are needed labels May 11, 2021
@dominiqueclarke dominiqueclarke requested a review from a team as a code owner May 11, 2021 12:51
@elasticmachine
Copy link
Contributor

Pinging @elastic/uptime (Team:uptime)

dominiqueclarke
dominiqueclarke commented May 11, 2021
View reviewed changes
test/functional/page_objects/time_picker.ts
@@ -68,7 +69,9 @@ export function TimePickerProvider({ getService, getPageObjects }: FtrProviderCo
}

private async getTimePickerPanel() {
return await find.byCssSelector('div.euiPopover__panel-isOpen');
return await retry.try(async () => {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@justinkambic findByCssSelector also takes a timeout, is that correct? Perhaps I should be using that instead of retry.try

Copy link
Contributor

Choose a reason for hiding this comment

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

It does take a timeout parameter. If you can achieve the same level of stability while removing the retry wrap with just a timeout then it's probably worth simplifying.

justinkambic
justinkambic reviewed May 11, 2021
View reviewed changes
Copy link
Contributor

@justinkambic justinkambic left a comment

Choose a reason for hiding this comment

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

This all looks fine - is the test in question not skipped right now in master?

Also, will this actually fix #98412, or will the backport to 7.13 cover that? We should probably wait to close that issue until the change is actually merged.

And we can close out #97558 and merge to 7.12, I don't know that there will be another patch release for that minor or not.

test/functional/page_objects/time_picker.ts
@@ -68,7 +69,9 @@ export function TimePickerProvider({ getService, getPageObjects }: FtrProviderCo
}

private async getTimePickerPanel() {
return await find.byCssSelector('div.euiPopover__panel-isOpen');
return await retry.try(async () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

It does take a timeout parameter. If you can achieve the same level of stability while removing the retry wrap with just a timeout then it's probably worth simplifying.

@dominiqueclarke
Copy link
Contributor Author

@elasticmachine merge upstream

justinkambic
justinkambic approved these changes May 14, 2021
View reviewed changes
Copy link
Contributor

@justinkambic justinkambic left a comment

Choose a reason for hiding this comment

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

LGTM

@dominiqueclarke
Copy link
Contributor Author

@elasticmachine merge upstream

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

Unknown metric groups

References to deprecated APIs

id before after diff
canvas 29 25 -4
crossClusterReplication 8 6 -2
fleet 4 2 -2
globalSearch 4 2 -2
indexManagement 12 7 -5
infra 5 3 -2
licensing 18 15 -3
monitoring 109 56 -53
total -73

History

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

@dominiqueclarke dominiqueclarke merged commit 25cad22 into elastic:master May 14, 2021
@dominiqueclarke dominiqueclarke deleted the fix/97558-overview-flaky-tests branch May 14, 2021 18:33
kibanamachine added a commit to kibanamachine/kibana that referenced this pull request May 14, 2021
@dominiqueclarke @kibanamachine
[Uptime] Fix overview flaky tests (elastic#99781)
02b3d31 
* add retry logic and add describe.only to prepare for flaky test runner

Co-authored-by: Kibana Machine <[email protected]>
@kibanamachine kibanamachine mentioned this pull request May 14, 2021
Merged
kibanamachine added a commit to kibanamachine/kibana that referenced this pull request May 14, 2021
@dominiqueclarke @kibanamachine
[Uptime] Fix overview flaky tests (elastic#99781)
e07b5a2 
* add retry logic and add describe.only to prepare for flaky test runner

Co-authored-by: Kibana Machine <[email protected]>
@kibanamachine kibanamachine mentioned this pull request May 14, 2021
Merged
kibanamachine added a commit to kibanamachine/kibana that referenced this pull request May 14, 2021
@dominiqueclarke @kibanamachine
[Uptime] Fix overview flaky tests (elastic#99781)
6bdbe98 
* add retry logic and add describe.only to prepare for flaky test runner

Co-authored-by: Kibana Machine <[email protected]>
@kibanamachine kibanamachine mentioned this pull request May 14, 2021
Merged
@kibanamachine
Copy link
Contributor

💚 Backport successful

Status Branch Result
✅ 7.12
✅ 7.13
✅ 7.x

The backport PRs will be merged automatically after passing CI.

kibanamachine added a commit that referenced this pull request May 14, 2021
@kibanamachine @dominiqueclarke
[Uptime] Fix overview flaky tests (#99781) (#100157)
1d6e26b 
* add retry logic and add describe.only to prepare for flaky test runner

Co-authored-by: Kibana Machine <[email protected]>

Co-authored-by: Dominique Clarke <[email protected]>
kibanamachine added a commit that referenced this pull request May 14, 2021
@kibanamachine @dominiqueclarke
[Uptime] Fix overview flaky tests (#99781) (#100156)
df8b580 
* add retry logic and add describe.only to prepare for flaky test runner

Co-authored-by: Kibana Machine <[email protected]>

Co-authored-by: Dominique Clarke <[email protected]>
kibanamachine added a commit that referenced this pull request May 14, 2021
@kibanamachine @dominiqueclarke
[Uptime] Fix overview flaky tests (#99781) (#100155)
22a3460 
* add retry logic and add describe.only to prepare for flaky test runner

Co-authored-by: Kibana Machine <[email protected]>

Co-authored-by: Dominique Clarke <[email protected]>
yctercero pushed a commit to yctercero/kibana that referenced this pull request May 25, 2021
@dominiqueclarke @kibanamachine @yctercero
[Uptime] Fix overview flaky tests (elastic#99781)
bb645ef 
* add retry logic and add describe.only to prepare for flaky test runner

Co-authored-by: Kibana Machine <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@justinkambic justinkambic justinkambic approved these changes

Assignees
No one assigned
Labels
auto-backport Deprecated - use backport:version if exact versions are needed failed-test A test failure on a tracked branch, potentially flaky-test release_note:skip Skip the PR/issue when compiling release notes skipped-test Team:Uptime - DEPRECATED Synthetics & RUM sub-team of Application Observability v7.12.2 v7.13.0 v7.14.0 v8.0.0
Projects
None yet
Milestone
No milestone
4 participants
@dominiqueclarke @elasticmachine @kibanamachine @justinkambic