tiny-pilot / tiny-pilot/tinypilot

Fix e2e interferences, and establish robust parallelism

Open
#1,694 2 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

bug enhancement
Dominant language
Python
Stars
3.5k
Forks
291
PR merge metrics
No merged PRs in 30d

Description

Merging over the grouped e2e tests uncovered an interference problem in our e2e test setup.

Update: we disabled concurrent tests locally to alleviate the problem for local development for the time being. The topic in itself is still valid, however.

Problem

Our Playwright setup is this:

  • On CI, we only use a single Playwright worker. Therefore, Playwright will run each spec file one after the other, in alphabetic order.
  • Locally, however, we run had been running the files in parallel, which is the default behaviour, in case workers wasn’t set (i.e., undefined).

Due to the new grouping and our usage of the beforeEach mechanism, the timing behaviour of the tests happened to change, compared to before:

When the security-dialog tests are running, they alter the server state and toggle on the auth requirement. The about-dialog tests are executed in parallel, and can now randomly fail, since Playwright happens to attempt to open the about dialog during a time when the auth requirement is active (caused by one of the concurrently ran security tests). Therefore, Playwright fails to open the about dialog in the first place, as it’s stuck on the login page.

This problem isn’t new, but so far the timing was in our favour, so we were lucky enough to not run into this. Now, with the different timing behaviour, the e2e tests can fail locally.

Again: due to us only using a single worker on CI, this problem only manifests was only manifesting itself locally, not on CI.

Solution

As we continue to add more and more e2e tests (which I think is terrific), I suggest we fix our e2e test setup, and make it robust against these kinds of issues. Specifically, I think we should establish a deliberate mechanism for parallelism:

  • If we perform sequential scenarios that alter server state (such as the security tests), we should make sure that no other test can run in parallel. That is because server state acts like a singleton, and is shared across all tests.
    • These kinds of test also must clean up the server state afterwards, otherwise we might see erratic failures in subsequent tests.
  • For all other, “read-only” tests (i.e., ones that don’t alter server state), we should embrace parallelism as much as possible, to take advantage of lower execution times. I think we should be able to do that on CI and locally likewise, so I’d suspect we don’t need to differentiate here like we do now.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with playwright.config.js and compare the state-changing scenarios in e2e/security.spec.js with the setup in e2e/about.spec.js. Verify how the referenced grouping and worker configuration affect execution, then ensure state-changing tests are isolated and cleaned up while read-only tests can run in parallel. Done means the e2e suite is reliable both locally and on CI.

Written by the indexing model from the issue text.

Assessment

Tech stack
javascript, playwright
Domain
testing-qa
Issue type
Refactor
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
42/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.