Skip to content

Improve the e2e tests involving animations and waitForAnimation #13739

@youknowriad

Description

@youknowriad

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.

Metadata

Metadata

Assignees

No one assigned

    Labels

    [Type] Automated TestingTesting infrastructure changes impacting the execution of end-to-end (E2E) and/or unit tests.[Type] EnhancementA suggestion for improvement.

    Type

    No type
    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions