Skip to content

ci(e2e): run the E2E suite in three parts side by side (FRU-268) - #286

Merged
Mheaus merged 1 commit into
mainfrom
fru-authfix/fru-268-e2e-shards
Oct 14, 2026
Merged

Mheaus merged 1 commit into
mainfrom
fru-authfix/fru-268-e2e-shards

Conversation

@sakuga-agent

@sakuga-agent sakuga-agent Bot commented Oct 13, 2026 •

Copy link
Copy Markdown
Contributor

For Mathieu: this changes the CI (.github/workflows/ci.yml), the orchestrator does not merge it. No ruleset change is needed: the ruleset default of main requires no status check by name, and the check e2e keeps its name anyway.

#285 and #288 are merged. This branch is now on main, one commit: the workflow, and the test of CONTRIBUTING.md that reads its jobs.

The E2E suite runs in three parts side by side (playwright test --shard=N/3), each on its own runner with four Playwright workers (#285). A last job, still named e2e, passes only when every part passed. Part of FRU-268.

Measures

Wall time of the check: from the start of the first part to the end of e2e. Runner time: the sum of the e2e jobs.

runs wall runner flaky
main today: one job, one worker 3 (1034 to 1036) 13 min 19 to 13 min 29 same 0
#285: one job, four workers 3 (1042, 1044, 1052) 6 min 36 to 6 min 57, median 6 min 41 same 0
Two parts, four workers each 1 (1043) 5 min 05 8 min 52 1 (fixed in #285)
This PR: three parts, four workers each 5 green (1047, 1051, 1064, 1065, 1067) 3 min 26 to 3 min 51, median 3 min 41 9 min 04 to 10 min 12 0

290 tests in each run. A fourth run of three parts (1058) failed on read-failure.spec.ts, a race in the test that #288 fixes; the three runs after it (1064, 1065, 1067) carried the fix.

Where the time of one part goes (run 1047, part 1): 24 s to start the Playwright container, 30 s of checkout, Node, pnpm, cache and install, then about 20 s of build and server start, and 2 to 2.6 minutes of tests. The pnpm store is already cached; the rest repeats on each runner, which is the cost of a part: about 75 s of runner each.

Why three parts and not two

Playwright cuts by files and by count of tests, not by time. Two parts gave 161 and 129 tests, 3.7 and 2.5 minutes: the job of the long half took 4 min 58, at the edge of the 5 minutes asked. Three parts give 111, 85 and 94 tests, about 2 minutes each.

What changes

  • e2e-shard (shown as e2e (1/3), e2e (2/3), e2e (3/3)): the former e2e job with a matrix and fail-fast: false. The count of parts comes from strategy.job-total, so it is written once, in the matrix. The ceiling goes from 20 to 15 minutes. The artefact of a failed part is playwright-report-N.
  • e2e: needs e2e-shard, runs if: always(), and fails unless the result of the matrix is success. The result goes through env, not into the script.
  • apps/worker/src/contributing.test.ts: the parts are not checks of their own, so the table of CONTRIBUTING.md still lists e2e once.

zizmor 1.30.1, offline, as CI runs it: no finding. No pull_request_target, no new action.

Not verified

  • Six runs of three parts is a small sample. A spec that depends on the files that run before it in its worker would show only now and then.
  • More parts would cut the wall a little more and cost about 75 s of runner each; not measured.

🤖 Generated with Claude Code


Sakuga Review · Score de confiance : 4/5

Mergeable : la suite E2E est découpée en trois parts et le job e2e garde son nom et échoue si une part échoue, est annulée ou ne tourne pas. Je n'ai pas relevé de défaut dans le diff. Le script e2e du package.json et la configuration Playwright ne sont pas visibles, donc le passage de --shard n'est pas vérifiable d'ici.

Résumé

Le workflow ci.yml remplace le job e2e par une matrice e2e-shard (trois parts, fail-fast: false) qui lance pnpm e2e --shard=N/3, avec un artefact de rapport par part, puis ajoute un job e2e agrégateur qui exige que needs.e2e-shard.result vaille success. Le test contributing.test.ts exclut les jobs à matrice du décompte des checks, sauf check, pour que la liste reste celle des cibles Nx. J'ai contrôlé les expressions ${{ }} (elles passent par env, sans injection), les noms d'artefacts uniques par part, la condition if: always() et le cas d'une part annulée ou sautée.

Diagramme
%%{init: {'theme': 'neutral'}}%%
flowchart LR
A["ci.yml e2e-shard 1..3"] --> B["pnpm e2e --shard=N/3"]
B --> C["playwright-report-N"]
A --> D["job e2e agregateur"]
D --> E["test RESULT = success"]
Loading
Non relu
  • package.json (script e2e): on ne voit pas si pnpm transmet --shard au playwright qu'il lance.
  • playwright.config : le découpage par shard suppose des specs indépendantes, non vérifiable ici.
  • docs/CONTRIBUTING (liste des checks attendus par contributing.test.ts) : non fournie, donc la cohérence du nouveau décompte n'est pas vérifiée.

Revues (5) · Dernier commit relu : ci(e2e): run the E2E suite in three parts side ... · Niveau lite · anthropic/claude-haiku-5.5 · 0,003 $

@sakuga-agent
sakuga-agent Bot force-pushed the fru-authfix/fru-268-e2e-shards branch 2 times, most recently from 47dd97f to 0ec0117 Compare October 13, 2026 15:38
@sakuga-agent sakuga-agent Bot changed the title ci(e2e): run the E2E suite in two halves side by side (FRU-268) ci(e2e): run the E2E suite in three parts side by side (FRU-268) Oct 13, 2026
@sakuga-agent
sakuga-agent Bot marked this pull request as ready for review October 13, 2026 16:00

@kento-review kento-review Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Score de confiance : 3/5 · ⚠️ Rien de bloquant · La PR peut être fusionnée sous réserve de vérifier un point : le job e2e agrège désormais les trois parts via needs, mais la correction du test read-failure repose sur un comportement de focus non vérifié.

Niveau lite · anthropic/claude-haiku-5.5 · 0,002 $ · Le résumé, le diagramme et la liste des constats sont à la fin de la description de la PR.

@kento-review kento-review Bot added the review: 3/5 Score de confiance de Sakuga Review (5 : prête à merger) label Oct 13, 2026
@sakuga-agent

sakuga-agent Bot commented Oct 13, 2026

Copy link
Copy Markdown
Contributor Author

On the lite review:

  • The count of parts (ci.yml:116): fixed in the last commit. The count was written in the matrix and again in --shard=N/3. It now comes from strategy.job-total, in the command and in the name of each part, so a part added to the matrix is run. The final e2e already fails on cancelled, skipped and failure, as the review says: only success passes.
  • The focus in read-failure.spec.ts: the change is test(e2e): open the list again once its first read failed (FRU-268) #288, carried here until it lands. The focus is chosen once, in open() of packages/widget/src/sidebar.ts: the first note, else « Try again » when the failure is shown, else the close button. Nothing moves it afterwards. The test now opens the list once the failure is on screen, which is the case open() handles. Runs 1064 and 1065 passed with it; run 1058, without it, failed on this test twice.

@kento-review kento-review Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Score de confiance : 4/5 · ✅ Rien de bloquant · Peut être fusionné : la répartition de la suite E2E en trois parts conserve le contrôle final e2e qui échoue si une part échoue, est annulée ou ne tourne pas, avec un risque mineur sur le test de liste de notes relancée.

Niveau lite · anthropic/claude-haiku-5.5 · 0,002 $ · Le résumé, le diagramme et la liste des constats sont à la fin de la description de la PR.

@kento-review kento-review Bot added review: 4/5 Score de confiance de Sakuga Review (5 : prête à merger) and removed review: 3/5 Score de confiance de Sakuga Review (5 : prête à merger) labels Oct 13, 2026
@sakuga-agent

sakuga-agent Bot commented Oct 13, 2026

Copy link
Copy Markdown
Contributor Author

@sakuga-review review balanced

@kento-review kento-review Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Score de confiance : 4/5 · ⚠️ Rien de bloquant · Fusionnable : le changement CI est cohérent, avec quelques points mineurs à vérifier sur le job d'agrégation et le test de lecture.

Niveau balanced · anthropic/claude-sonnet-5.5 · 0,043 $ · Le résumé, le diagramme et la liste des constats sont à la fin de la description de la PR.

@sakuga-agent

sakuga-agent Bot commented Oct 13, 2026

Copy link
Copy Markdown
Contributor Author

On the balanced review, nothing pushed (the PR is green):

  1. strategy.job-total (ci.yml:93): true for a matrix of more than one dimension, and this one has one, shard, with no include or exclude. A second dimension (a browser, say) would change what a part is, and the command would have to change with it whichever way the count is written. With the count written twice, as before, the lite showed the other way it breaks: a part added to the matrix and not to the command. I keep job-total.
  2. The focus in read-failure.spec.ts (:58): the test no longer checks what happens when the read fails while the list is open. That case never moved focus: open() chooses once, and the failure is announced by the status region, which the test still reads both times. Whether focus should jump to « Try again » when a read fails under the reader is a choice for the widget, not a race of the test; moving focus with no action of the person is what the list avoids elsewhere ("A reviewer who clicked on the page keeps focus where they put it"). The case the test was written for, a list opened on a failed read, is checked without the race.

Base automatically changed from fru-authfix/fru-268-e2e-workers to main October 13, 2026 16:13
@sakuga-agent
sakuga-agent Bot force-pushed the fru-authfix/fru-268-e2e-shards branch from 9679586 to dd05d09 Compare October 13, 2026 16:15
@sakuga-agent

sakuga-agent Bot commented Oct 13, 2026

Copy link
Copy Markdown
Contributor Author

Put back on main after #285 and #288 merged: 9679586 → dd05d09, one commit, force-pushed with a lease. The former head is kept locally as backup/fru-268-e2e-shards-1310.

git range-diff origin/main 9679586 dd05d09:

 1:  bc805b5 <  -:  ------- test(e2e): run the files of the suite on four workers on CI (FRU-268)
 2:  0ef80a0 <  -:  ------- fix(worker): the memory Linear gives states and replies by rank on the page (FRU-268)
 3:  39c92ba <  -:  ------- test(e2e): wait for the read of the new workspace in its control (FRU-268)
 4:  345d914 <  -:  ------- test(e2e): measure the four workers again (FRU-268)
 5:  24dbd4f !  1:  dd05d09 ci(e2e): run the E2E suite in two halves side by side (FRU-268)
 6:  0613ebc <  -:  ------- ci(e2e): run the E2E suite in three parts (FRU-268)
 7:  0ec0117 <  -:  ------- test(e2e): measure the three parts again (FRU-268)
 8:  28cc7ee <  -:  ------- test(e2e): open the list again once its first read failed (FRU-268)
 9:  d94bb9b <  -:  ------- test(e2e): measure the three parts a fourth time (FRU-268)
10:  9679586 <  -:  ------- ci(e2e): take the count of parts from the matrix (FRU-268)

@kento-review kento-review Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Score de confiance : 4/5 · ✅ Rien de bloquant · Mergeable : la matrice de trois parts et le job e2e qui agrège leur résultat sont corrects, et le test contributing suit le renommage des checks. Je n'ai pas pu vérifier la configuration Playwright ni le script pnpm e2e qui reçoivent --shard.

Niveau lite · anthropic/claude-haiku-5.5 · 0,003 $ · Le résumé, le diagramme et la liste des constats sont à la fin de la description de la PR.

Each part runs on its own runner, on four Playwright workers. The count of
parts comes from the matrix. A last job, still named e2e, fails unless
every part passed. The table of the checks in CONTRIBUTING.md lists e2e
once.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
@sakuga-agent
sakuga-agent Bot force-pushed the fru-authfix/fru-268-e2e-shards branch from dd05d09 to eaafe3a Compare October 14, 2026 00:22
@sakuga-agent

sakuga-agent Bot commented Oct 14, 2026

Copy link
Copy Markdown
Contributor Author

Rebased on main (05b114d, with #314 and the specs added overnight): dd05d09 → eaafe3a, force-pushed with a lease on this branch only. The former head is kept locally as backup/fru-268-e2e-shards-dd05d09.

One conflict, in the step that runs the suite: main changed its comment (FRU-223, PGlite in the process); the new step keeps that comment and runs one part. git range-diff origin/main dd05d09 eaafe3a:

1:  dd05d09 ! 1:  eaafe3a ci(e2e): run the E2E suite in three parts side by side (FRU-268)
    @@ .github/workflows/ci.yml: jobs:
      
     -      - name: Run the E2E suite
     +      - name: Run one part of the E2E suite
    -         # No LINEAR_API_KEY anywhere: the worker runs on its in-memory store (FRUITBACK_STORE=memory).
    +         # No key and no service: each worker of the suite has a PostgreSQL in its process (PGlite,
    +-        # `dev-worker.fixture.ts`, FRU-223).
     -        run: pnpm e2e
    -+        # The count of parts comes from the matrix, so a part added there is run here too.
    ++        # `dev-worker.fixture.ts`, FRU-223). The count of parts comes from the matrix, so a part
    ++        # added there is run here too.
     +        env:
     +          SHARD: ${{ matrix.shard }}
     +          SHARDS: ${{ strategy.job-total }}

Nothing else moved. zizmor 1.30.1 offline: no finding; contributing.test.ts and workflows.test.ts pass.

@kento-review kento-review Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Score de confiance : 4/5 · ✅ Rien de bloquant · Mergeable : la suite E2E est découpée en trois parts et le job e2e garde son nom et échoue si une part échoue, est annulée ou ne tourne pas. Je n'ai pas relevé de défaut dans le diff. Le script e2e du package.json et la configuration Playwright ne sont pas visibles, donc le passage de --shard n'est pas vérifiable d'ici.

Niveau lite · anthropic/claude-haiku-5.5 · 0,003 $ · Le résumé, le diagramme et la liste des constats sont à la fin de la description de la PR.

@Mheaus
Mheaus merged commit e38c4c7 into main Oct 14, 2026
16 checks passed
@Mheaus
Mheaus deleted the fru-authfix/fru-268-e2e-shards branch October 14, 2026 08:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

review: 4/5 Score de confiance de Sakuga Review (5 : prête à merger)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant