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

Browser mode is flaky #5706

Open
6 tasks done
gossi opened this issue May 11, 2024 · 1 comment
Open
6 tasks done

Browser mode is flaky #5706

gossi opened this issue May 11, 2024 · 1 comment

Comments

@gossi
Copy link

gossi commented May 11, 2024

Describe the bug

I'm writing a testsuite for aria behavior (in vanilla js). These tests require checking against DOM, such as document.activeElement to see if the expected element received focus.

While writing tests, I realized one of my tests started to fail. I was even more suspicious when I commented out the tests that happen before and my failing test become successful, huh?

I figured I could shuffle the tests with describe.shuffle() and yes, running them through shuffle makes them fail and succeed.

I also have some ideas, why that might be:

  • Some DOM operations are put on micro/macro task queue and need to be awaited to finish (closing and opening popovers as I do in the tests, wait for document.activeElement)
  • between suites, the browser isn't "resetted" and state leaks between them

Here is a video of me running tests (shuffle mode):

Bildschirmaufnahme.2024-05-11.um.12.14.55.mov

Reproduction

My repro is the monorepo of my design system. Though the affected package in there is a leaf package. I committed the flaky state, here is the repro sha: https://github.com/hokulea/hokulea/tree/655f2ac11697dd5dc54bf2faafa56aceb142e40e

Repro steps:

  • Checkout the sha above
  • run pnpm install in the root
  • cd incubator/aria-navigator
  • run vitest in there (pnpm vitest:ui is what I use)

The affected tests are tests/menu/navigation.test.ts, the submenu navigation suite.

The test code is what matters, details the the things being tested shouldn't matter that much, but here is a summary for better understanding:

  • DOM event handlers
  • HTMLElement methods: focus(), showPopover(), hidePopover()
  • Popover API, more precise auto mode

System Info

System:
    OS: macOS 14.4.1
    CPU: (11) arm64 Apple M3 Pro
    Memory: 99.50 MB / 36.00 GB
    Shell: 5.9 - /bin/zsh
  Binaries:
    Node: 20.10.0 - ~/.volta/tools/image/node/20.10.0/bin/node
    Yarn: 4.1.1 - ~/.volta/tools/image/yarn/4.1.1/bin/yarn
    npm: 10.2.3 - ~/.volta/tools/image/node/20.10.0/bin/npm
    pnpm: 8.15.1 - ~/.volta/tools/image/pnpm/8.15.1/bin/pnpm
    Watchman: 2024.04.15.00 - /opt/homebrew/bin/watchman
  Browsers:
    Brave Browser: 103.1.41.100
    Chrome: 124.0.6367.158
    Edge: 124.0.2478.51
    Safari: 17.4.1
  npmPackages:
    @vitest/browser: ^1.6.0 => 1.6.0
    @vitest/coverage-istanbul: ^1.6.0 => 1.6.0
    @vitest/ui: ^1.6.0 => 1.6.0
    vite: ^5.2.6 => 5.2.8
    vitest: ^1.6.0 => 1.6.0

Used Package Manager

pnpm

Validations

@sheremet-va
Copy link
Member

I don't think that this flakiness is something that we can address on a runner level - if you need to wait for an event to resolve, then you should await until it happens - Vitest provides vi.waitFor utility for example. You also need to write tests isolated from one another - so one test doesn't affect the other.

What we will do in the future though is provide API to deal with events because dispatchEvent is not something that actually happens when user interacts with the web page - the most correct implementation would be to call Devtools protocol, because even a simple click can have a lot of sub-events and different behaviors depending on the browser and the element. For example, in the next version you should be able to use the new commands API: #5097. In the future, it will be expanded to support more APIs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants