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

Add retry to find.existsByDisplayedByCssSelector #48734

Merged
merged 13 commits into from
Nov 18, 2019

Conversation

pheyos
Copy link
Member

@pheyos pheyos commented Oct 21, 2019

Summary

There are scenarios where testSubjects.existOrFail fails earlier than the provided timeout because the timeout is only applied to the find operation and not to the filter for displayed elements, e.g.

await testSubjects.existOrFail('myTestSubject', { timeout: 5000 });

may fail after a few milliseconds, if myTestSubject is found that fast but is not visible yet.
In order to make the whole existOrFail operation follow the provided timeout, this PR adds a retry to the underlying find.existsByDisplayedByCssSelector.

@pheyos pheyos added test_ui_functional v8.0.0 release_note:skip Skip the PR/issue when compiling release notes v7.5.0 labels Oct 21, 2019
@pheyos pheyos requested a review from spalger October 21, 2019 09:46
@pheyos pheyos self-assigned this Oct 21, 2019
@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@pheyos pheyos requested a review from dmlemeshko October 22, 2019 06:07
@pheyos pheyos requested a review from dmlemeshko October 23, 2019 13:33
@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💔 Build Failed

@pheyos
Copy link
Member Author

pheyos commented Oct 24, 2019

retest

@elasticmachine
Copy link
Contributor

💔 Build Failed

@pheyos pheyos changed the title Add retry to testSubjects.existOrFail Add retry to find.existsByDisplayedByCssSelector Oct 24, 2019
@spalger
Copy link
Contributor

spalger commented Oct 30, 2019

@elasticmachine merge upstream

@elasticmachine
Copy link
Contributor

💔 Build Failed

@pheyos pheyos requested a review from LeeDr November 13, 2019 09:40
@elasticmachine
Copy link
Contributor

💔 Build Failed

@pheyos
Copy link
Member Author

pheyos commented Nov 15, 2019

retest

Copy link
Member

@dmlemeshko dmlemeshko left a comment

Choose a reason for hiding this comment

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

Code LGTM, as we discussed - let's do more run on CI to feel safe :)

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@pheyos
Copy link
Member Author

pheyos commented Nov 15, 2019

Dashboard snapshot test (oss ciGroup3) passed 20x with this PR, see https://kibana-ci.elastic.co/job/kibana+flaky-test-suite-runner/5/

@pheyos
Copy link
Member Author

pheyos commented Nov 15, 2019

retest

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@pheyos
Copy link
Member Author

pheyos commented Nov 15, 2019

Dashboard snapshot test (oss ciGroup3) passed another 20x with this PR, see https://kibana-ci.elastic.co/job/kibana+flaky-test-suite-runner/6/

@pheyos
Copy link
Member Author

pheyos commented Nov 15, 2019

retest

Copy link

@LeeDr LeeDr left a comment

Choose a reason for hiding this comment

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

LGTM and Thanks for making this improvement!

At some point we should refactor the save visualization functionality to be consistent with this;
https://github.com/elastic/kibana/blob/master/test/functional/page_objects/visualize_page.js#L766

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@pheyos
Copy link
Member Author

pheyos commented Nov 18, 2019

@elasticmachine merge upstream

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@pheyos pheyos added v7.5.1 and removed v7.5.0 labels Nov 18, 2019
@pheyos pheyos merged commit 39aeaaa into elastic:master Nov 18, 2019
@pheyos pheyos deleted the fix_existOrFail branch November 18, 2019 08:37
pheyos added a commit to pheyos/kibana that referenced this pull request Nov 18, 2019
This PR fixes timeout handling in `find.existsByDisplayedByCssSelector` for elements that are found but (not yet) displayed.
pheyos added a commit that referenced this pull request Nov 18, 2019
This PR fixes timeout handling in `find.existsByDisplayedByCssSelector` for elements that are found but (not yet) displayed.
jloleysens added a commit to jloleysens/kibana that referenced this pull request Nov 18, 2019
…her [skip ci]

* upstream/master: (54 commits)
  allows plugins to define validation schema for "enabled" flag (elastic#50286)
  Add retry to find.existsByDisplayedByCssSelector (elastic#48734)
  [i18n] integrate latest translations (elastic#50864)
  ui/resize_checker 👉 src/plugins/kibana_utils (elastic#44750)
  Fix @reach/router types (elastic#50863)
  [ML] Adding ML node warning to overview and analytics pages (elastic#50766)
  Bump storybook dependencies (elastic#50752)
  [APM Replace usage of idx with optional chaining (elastic#50849)
  [SIEM] Fix eslint errors (elastic#49713)
  Improve "Browser client is out of date" error message (elastic#50296)
  [SIEM][Detection Engine] REST API improvements and changes from UI/UX feedback (elastic#50797)
  Move @kbn/es-query into data plugin - es-query folder (elastic#50182)
  Index Management new platform migration (elastic#49359)
  Increase retry for cloud snapshot to finish (elastic#50781)
  Removing EuiCode from inside EuiPanel (elastic#50683)
  [SIEM] Tests for search_after and bulk index (elastic#50129)
  Make babel understand TypeScript 3.7 syntax (elastic#50772)
  Fixing mocha tests and broken password change status codes (elastic#50704)
  [Canvas] Use compressed forms in sidebar (elastic#49419)
  Add labels to shell scripts in Jenkins (elastic#49657)
  ...
pheyos added a commit to pheyos/kibana that referenced this pull request Dec 3, 2019
This PR fixes timeout handling in `find.existsByDisplayedByCssSelector` for elements that are found but (not yet) displayed.
pheyos added a commit that referenced this pull request Dec 3, 2019
This PR fixes timeout handling in `find.existsByDisplayedByCssSelector` for elements that are found but (not yet) displayed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release_note:skip Skip the PR/issue when compiling release notes test_ui_functional v7.5.1 v7.6.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants