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

fix: Adding timeout to flaky cypress test, to wait for animation to complete #11074

Merged
merged 2 commits into from
Sep 28, 2020

Conversation

rusackas
Copy link
Member

SUMMARY

The remove, and add chart flow test seems to flake out fairly often, and I've seen it do so with errors saying that pointer-events were disabled by a parent's CSS, and that the element was still animating. I'm assuming that all of this is due to the animation and such that takes place when switching tabs. Here's hoping 1s of waiting will properly cover that issue.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

TEST PLAN

Keep an eye on CI!

ADDITIONAL INFORMATION

  • Has associated issue:
  • Changes UI
  • Requires DB Migration.
  • Confirm DB Migration upgrade and downgrade tested.
  • Introduces new feature or API
  • Removes existing feature or API

Copy link
Member

@ktmud ktmud left a comment

Choose a reason for hiding this comment

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

The way Cypress get works is to constantly retry querying the DOM until the elements that match the selector are found or timeout. https://docs.cypress.io/guides/core-concepts/retry-ability.html

So waiting arbitrary duration wouldn't really help with test flakiness. In fact, there is an ESLint rule to discourage that: https://docs.cypress.io/guides/references/best-practices.html

@rusackas
Copy link
Member Author

The way Cypress get works is to constantly retry querying the DOM until the elements that match the selector are found or timeout. https://docs.cypress.io/guides/core-concepts/retry-ability.html

So waiting arbitrary duration wouldn't really help with test flakiness. In fact, there is an ESLint rule to discourage that: https://docs.cypress.io/guides/references/best-practices.html

The problem seems to be though that it IS finding an element that matches the selector, but that element is not yet in a state where it's completely visible (animating) or clickable (with pointer-events:none applied to it). It seems like it's just not waiting for that transition to finish before starting the drag step of the test.

@rusackas
Copy link
Member Author

image

The help link for this error is here; it sounds like we're one of the "edge cases" they discuss, where the test is trying to interact with the element before it's finished animating.

@ktmud
Copy link
Member

ktmud commented Sep 27, 2020

Wow, everyday I learn something new about Cypress. Thanks for the explanation! This sounds like a good theory. Let's hope it really does fix it!

@rusackas rusackas merged commit 497d3f3 into apache:master Sep 28, 2020
@rusackas rusackas deleted the fixing-flaky-cypress-test branch September 28, 2020 03:08
auxten pushed a commit to auxten/incubator-superset that referenced this pull request Nov 20, 2020
…omplete (apache#11074)

* adding timeout for animation

* 1s timeout
@mistercrunch mistercrunch added 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 0.38.0 labels Mar 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels size/XS 🚢 0.38.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants