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

Improve the e2e tests involving animations and waitForAnimation #13739

Closed
youknowriad opened this issue Feb 7, 2019 · 0 comments
Closed

Improve the e2e tests involving animations and waitForAnimation #13739

youknowriad opened this issue Feb 7, 2019 · 0 comments
Labels
[Type] Automated Testing Testing infrastructure changes impacting the execution of end-to-end (E2E) and/or unit tests. [Type] Enhancement A suggestion for improvement.

Comments

@youknowriad
Copy link
Contributor

As part of #13617, a waitForAnimation helper was added to ensure that we wait for a reasonable delay when an animation happens before moving forward. This approach is not ideal in a lot of ways.

See these comments:

#13617 (comment)

I think as proposed it's at least doing well enough to not assume that it operates on some timer, since I think an implementation leveraging transitionend is probably a worthwhile exploration for further improvement. But the bigger issue to me is forcing it to be a conscious part of someone's mind in authoring tests, and whether that's strictly necessary. It seems far too prone to being forgotten and causing unexpected test failures, though if they're obvious enough and consistent failures, perhaps they're not truly "unexpected" and it's okay.

#13617 (comment)

Other possibilities to avoid it being a conscious part of the developer test authoring workflow is to ensure that most of the interactions which involve an animation are sufficiently abstracted. So rather than a test including:

await page.click( 'button[aria-label="More options"]' );
await waitForAnimation();

We'd see something like await openMoreOptions(); which handles the animation implementation detail internally.

#13617 (comment)

it resurfaces the problem of imposing the need to be aware of a delay to the test author. My original intent with raising the point was in trying to see through the implications, and if not to solve it in place, have some idea for a future goal. To me, that future goal ideally wouldn't involve waitForAnimation existing, hence why I'd considered whether to mark it as unstable.

The alternatives I'd have in mind which bypass the need for the function are rough in my mind, and in many ways similarly flawed / technically challenging:

  • Not have the transitions exist for the tests. It's still an issue that these are going to add measurable delays to the runtime of every build.
  • In some way, every transitionstart which ever occurs on a page is monitored such that when a test includes an interaction (e.g. click) which either triggers / follows (unsure which yet) a transition, the following action would be delayed automatically.

I'm also open to saying that accepting that this needs to be consciously considered as part of writing an end-to-end tests. I'm sure there's an argument that these "steps" imitate a user's physical actions, which includes that when a user clicks a button, they may have to sit and wait for the animation.

tl;dr: I have concerns, and I wanted them to be noted, but I also think this is good enough in its current state to be able to move forward on.

#13617 (comment)

I just had an idea probably similar to your first point here: maybe in the tests we load a specific stylesheet forcing all animations to be very quick (unnoticeable by tests) like 1ms.

  • It ensures we're testing the existence of the animation (like prod)
  • It ensures the tests are stable.
@youknowriad youknowriad added [Type] Enhancement A suggestion for improvement. [Type] Automated Testing Testing infrastructure changes impacting the execution of end-to-end (E2E) and/or unit tests. labels Feb 7, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Type] Automated Testing Testing infrastructure changes impacting the execution of end-to-end (E2E) and/or unit tests. [Type] Enhancement A suggestion for improvement.
Projects
None yet
Development

No branches or pull requests

1 participant