-
Notifications
You must be signed in to change notification settings - Fork 8.2k
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
[Uptime] Fix flaky uptime overview page test #54767
Conversation
Pinging @elastic/uptime (Team:uptime) |
Pinging @elastic/kibana-test-triage (failed-test) |
@@ -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) { |
There was a problem hiding this comment.
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);
});
There was a problem hiding this 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
@elasticmachine merge upstream |
@elasticmachine merge upstream |
@elasticmachine merge upstream |
@elasticmachine merge upstream |
@elasticmachine merge upstream |
@elasticmachine merge upstream |
@elasticmachine merge upstream |
@elasticmachine merge upstream |
@elasticmachine merge upstream |
@elasticmachine merge upstream |
💚 Build SucceededHistory
To update your PR or re-run it, just comment with: |
* 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>
* 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>
* 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>
* 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>
Backported to: |
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
strikethroughsto remove checklist items you don't feel are applicable to this PR.For maintainers