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

[Uptime] Fix flaky uptime overview page test #54767

Merged
merged 15 commits into from
Jan 21, 2020

Conversation

justinkambic
Copy link
Contributor

Summary

Fixes #54541.

The reason this test was failing was due to our test checking the URL before it had been updated. This usually happens really fast, so it passes most of the time. This patch adds a timeout to the function responsible for checking the URL state between page and filter updates so we will only fail after 2000ms, which is about 10x the duration of the check.

Checklist

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

For maintainers

@justinkambic justinkambic added blocker 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 v7.6.0 labels Jan 14, 2020
@justinkambic justinkambic self-assigned this Jan 14, 2020
@elasticmachine
Copy link
Contributor

Pinging @elastic/uptime (Team:uptime)

@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-test-triage (failed-test)

@justinkambic justinkambic added release_note:fix release_note:skip Skip the PR/issue when compiling release notes and removed release_note:fix labels Jan 14, 2020
@@ -51,8 +51,14 @@ export function UptimePageProvider({ getPageObjects, getService }: FtrProviderCo
await Promise.all(monitorIdsToCheck.map(id => uptimeService.monitorPageLinkExists(id)));
}

public async pageUrlContains(value: string) {
return await uptimeService.urlContains(value);
public async pageUrlContains(value: string, expected: boolean = true, timeout: number = 2000) {
Copy link
Contributor

@nreese nreese Jan 14, 2020

Choose a reason for hiding this comment

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

The common pattern for retires like this are to use retry.try. This will keep trying a couple of times until the expression does not throw.

await retry.try(async () => {
        const isChromeHidden = await PageObjects.common.isChromeHidden();
        expect(isChromeHidden).to.be(true);
      });

Copy link
Contributor

@nreese nreese left a comment

Choose a reason for hiding this comment

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

lgtm with green CI
code review

@justinkambic
Copy link
Contributor Author

@elasticmachine merge upstream

@justinkambic
Copy link
Contributor Author

@elasticmachine merge upstream

@justinkambic
Copy link
Contributor Author

@elasticmachine merge upstream

@justinkambic
Copy link
Contributor Author

@elasticmachine merge upstream

@justinkambic
Copy link
Contributor Author

@elasticmachine merge upstream

@justinkambic
Copy link
Contributor Author

@elasticmachine merge upstream

@justinkambic
Copy link
Contributor Author

@elasticmachine merge upstream

@justinkambic
Copy link
Contributor Author

@elasticmachine merge upstream

@justinkambic
Copy link
Contributor Author

@elasticmachine merge upstream

@justinkambic
Copy link
Contributor Author

@elasticmachine merge upstream

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

History

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

@justinkambic justinkambic merged commit 2bf111c into elastic:master Jan 21, 2020
justinkambic added a commit to justinkambic/kibana that referenced this pull request Jan 21, 2020
* Fix flaky uptime overview page test.

* Increase timeout for url checks.

* Prefer standard `retry.try` to custom retry implementation.

* Remove unneeded symbol.

* Remove unnecessary type annotation.

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
justinkambic added a commit to justinkambic/kibana that referenced this pull request Jan 21, 2020
* Fix flaky uptime overview page test.

* Increase timeout for url checks.

* Prefer standard `retry.try` to custom retry implementation.

* Remove unneeded symbol.

* Remove unnecessary type annotation.

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
justinkambic added a commit that referenced this pull request Jan 21, 2020
* Fix flaky uptime overview page test.

* Increase timeout for url checks.

* Prefer standard `retry.try` to custom retry implementation.

* Remove unneeded symbol.

* Remove unnecessary type annotation.

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
justinkambic added a commit that referenced this pull request Jan 21, 2020
* Fix flaky uptime overview page test.

* Increase timeout for url checks.

* Prefer standard `retry.try` to custom retry implementation.

* Remove unneeded symbol.

* Remove unnecessary type annotation.

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
@justinkambic
Copy link
Contributor Author

@justinkambic justinkambic deleted the uptime_fix-flaky-test-54527 branch January 21, 2020 18:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
blocker 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.6.0 v7.7.0 v8.0.0
Projects
None yet
4 participants