ci(e2e): un vert qui veut dire quelque chose — les parcours connectés tournent enfin - #271
Merged
Conversation
Three guards stood between CI and the authenticated journeys, and none of
them announced itself.
`adminClientOrNull()` decided whether Supabase was real by checking whether
the URL contained `localhost:54321` — the exact address `supabase start`
serves. Standing up a local stack in CI would therefore have changed nothing:
all 173 skipped cases would have kept skipping. Readiness is now declared with
E2E_SUPABASE_READY, and a declared-but-broken environment throws instead of
skipping, because a green mostly-skipped run is the failure being fixed.
`rateLimit('auth', …)` allows 5 attempts per 15 minutes keyed by IP, and the
suite performs well over a hundred real logins. Each test now carries its own
`x-forwarded-for`, which the app already trusts as the caller's address
(Vercel overwrites it upstream), so the buckets isolate without a single line
of production code changing.
GoTrue enforces its own quota on top of ours — `sign_in_sign_ups` defaults to
30 per 5 minutes per IP. Left alone it would have failed the suite while
looking like a broken login. `supabase/config.toml` raises it for the
ephemeral stack only; production is configured in the dashboard.
The config is hand-trimmed from the 396 lines `supabase init` emits: realtime,
storage, studio, analytics, edge_runtime and the pooler are off (measured
unused), inbucket stays on because the forgot-password spec needs a mail sink.
Ports are shifted off the Supabase defaults — the professional project runs its
own stack on this machine, and `supabase start` suggested stopping *that* one.
Both unconditional skips are gone. The anti-enumeration spec only ever needed a
reachable Supabase. The locale soft-navigation spec was skipped on a false
premise: it hunted a FAQ link the desktop landing does not render, but the
landing does mount MktFooter with real next-intl Links — it needed any
prefetched Link, not that one. Selector now matches the href, since the label
is precisely what the locale switch changes.
Spec selection is shared between the local runner and CI, and the expected list
is committed. Discovery previously matched `adminClientOrNull` alone and missed
every spec seeding through the `seededUser` fixture, so `e2e:auth --all` had
been running 9 specs while reporting it ran them all. Divergence in either
direction now fails loudly.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
A second Playwright job boots an ephemeral Supabase stack, applies the 15 migrations to an empty database, and runs the specs that seed a user and log in. Measured locally before being pushed: 24 cases, 0 failures, 1 minute, 79 real logins served with zero rate-limit refusals. It runs OUTSIDE a job container, unlike `e2e`. `supabase start` needs the Docker daemon, and from inside a container the Supabase containers would be siblings on the host with localhost routing nowhere. Two address spaces coexist in the job and confusing them is the likeliest way to break it: SRH publishes 8079 for the host-side Next server, and reaches Redis by service name on the job network. The per-test IP is derived from the test's identity, not a counter. A counter looked equivalent and was not: Playwright runs retries in a FRESH worker, which re-imports the module and resets it to zero, so every retry reused the first test's address. A handful of failures piled their retries onto the same few IPs, exhausted the 5-per-15-minutes budget and turned isolated failures into a cascade of "Trop de tentatives" — 29 failures in 24.8 minutes, against 12 in 5.4 once derived from testId + retry. Six specs are quarantined, listed with a reason each and printed on every run. They describe a dashboard that no longer exists: three assert data-testids present in ZERO files under src/ (effort-financier-card, substat-*, capacite-epargne-*) — the hero radar the Situation Hero replaced — and three look for a heading role on CardTitle, which renders a div. They were merged, the cockpit was rebuilt, and nothing objected because they had never executed anywhere. Running them for the first time is what surfaced it. That last cause is also a real accessibility defect: the cockpit's section titles are not headings for a screen reader. Left for its own PR, since this one touches no src/. Also surfaced and left alone: audit logging fails outright on a database rebuilt from the migrations (permission denied for table audit_log). It works in production, so a grant was applied there outside the migration set — the migrations are not a complete description of production, which matters for a GDPR-mandated audit trail. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Six are quarantined; the table said otherwise two sections above the section that explains the quarantine. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Adds the permanent DoD criterion: the number of e2e cases executed never decreases. Two jobs, so two floors — a single aggregate number would be unreadable at the first conflict, and an unmeasurable criterion is an ignored one. Floors as measured on 2026-07-26: 214 for the public job, 24 for the authenticated one. Also states that the quarantine list may only shrink. Adding to it is an admission that gets justified in writing, not a shortcut to a green pipeline. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Sorry @thierryvm, you have reached your weekly rate limit of 500000 diff characters.
Please try again later or upgrade to continue using Sourcery
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
github-actions
Bot
added
area:ci
…iling
The unconditional skip was lifted and the spec added to the authenticated
job's selection — but nothing stopped it also running in the PUBLIC job,
which has no Supabase. It failed there three times, once per browser
project, exactly the contradiction the plan review flagged and that I only
half-closed.
The skip is now CONDITIONAL on E2E_SUPABASE_READY, the same declared-readiness
signal the seed helper uses: it runs in the authenticated job and reports
itself skipped elsewhere, with the reason attached rather than vanishing.
`grep "test.skip('"` still returns zero — the ban was on unconditional skips,
not on stating a real precondition.
Public job floor raised 214 → 215: the locale soft-navigation spec, skipped
since May on a mistaken premise, passes there.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Étape 2 / 17. Rendre un
gh pr checks ✅significatif. Aujourd'hui il ne l'est pas.Le problème, chiffré
Run
30210147687surmain: 214 cas passent, 173 sautent — 44,7 % de la suite. Les 173 contiennent tous les parcours connectés. Le vert ne couvre que la surface publique.Six blocages, tous vérifiés en code
adminClientOrNull()sautait surlocalhost:54321supabase start. Démarrer une stack locale n'aurait rien changé.rateLimit()échoue fermé,next startforceNODE_ENV=productionrateLimit('mutation')aussi.auth= 5 / 15 min par IPgoto('/login')× 3 projets → 100+ connexions par passe.supabase/config.tomlsupabase starten exige un.e2e:auth --allannonçait tout couvrir en exécutant 9 specs sur 13.Ce que la revue a empêché
Ma v1 proposait un rate-limiter « en mémoire » pour les tests, désactivé si
NEXT_PUBLIC_APP_ENV === 'production'. Fail-open : cette variable a un.default('development'). Oubliée ou non propagée au scope Preview → le contournement s'active. Or les previews pointent sur l'unique projet Supabase, la production.Le plan final ne touche aucune ligne de
src/. Une IP unique par test, et un Redis réel dans le job — la suite gagne la couverture du rate-limiting au lieu de la perdre.Mesuré avant d'être poussé
Première passe : 29 échecs en 24,8 min. Cause : Playwright exécute chaque réessai dans un worker neuf, qui remettait mon compteur d'IP à zéro — chaque réessai réutilisait l'adresse du premier test, épuisait le budget, et transformait quelques échecs en cascade. Dérivée de
testId+retry: 29 → 12 → 0.Acquis permanent : les migrations rejouent depuis zéro
15 migrations → 14 tables sur une base vide. Jamais vérifié auparavant : la production ne les a reçues qu'incrémentalement. Fidélité confirmée, Postgres 17.6 des deux côtés.
Ce que ça révèle, et que je ne corrige pas ici
Six specs décrivent un tableau de bord qui n'existe plus. Mergées, puis le cockpit a été reconstruit, et rien n'a protesté puisqu'elles ne tournaient nulle part. Trois cherchent des
data-testidprésents dans zéro fichier desrc/. Trois cherchent un rôleheadingsurCardTitle, qui rend une<div>.→ Quarantaine explicite : découvertes, listées, comptées, imprimées à chaque run avec leur raison. Aucun
test.skipajouté.Trois dettes tracées : un défaut d'a11y réel (les titres du cockpit ne sont pas des titres), l'audit RGPD qui ne s'écrit pas sur une base reconstruite depuis les migrations (donc un droit posé hors migration en prod), et un angle mort du préflight sur le compte Supabase actif.
Incident de comptes évité
supabase starta échoué : le projet professionnelOVBoccupe les ports par défaut sur la machine, et le CLI suggérait d'arrêter sa stack. Ports d'Ankora décalés — les deux coexistent, aucune commande d'ici ne peut plus toucher au projet pro.La protection de branche est hors périmètre (PR dédiée, revue humaine). Le job ne sera pas dans les checks requis : il pourra être rouge sans bloquer un merge. Tant qu'une PR de suivi ne l'ajoute pas, le filet reste décoratif.
Rapport complet :
docs/prs/PR-refonte-02-filet-e2e-report.md