QA-testing a pull request

How to take on the role of QA for one PR: click through it locally, find what is broken or unfinished, leave deterministic e2e specs behind, and say whether it is ready to merge. This page comes from QA runs on real PRs. Update it when a run teaches something new.

A QA run always produces three things:

1. Specs added to the PR’s branch (deterministic, fixtures, no key).
2. Fixes for small bugs found, in the same branch, and issues for the rest.
3. A summary posted as a comment on the PR, with a coverage audit and a merge verdict (template). It lives on the PR, not in the repo.

Bugs live in issues, nowhere else. Run gh issue list first (a bug may be filed already; link it instead of filing again). Don’t commit PR-specific reports, and don’t commit test.fixme or other tests for bugs that aren’t fixed. Put the repro steps, and the spec to add with the fix, in the issue.

0. Before you start (five minutes, saves an hour)

1. Read, then write the claims down

Read the PR body and the diff (not the whole repo). From the body, list every claim: each thing the PR says a learner can now do, each rule it says holds, each “blocked while…” state. That list is the audit table in the summary. A claim with no test is a gap.

Then read the code for what the body does not say. These found the real problems so far:

• Every consumer of a new state. Adding an enum value or a course state (revising)? grep every place that branches on the enum and on its string form (templates, list pages, JS). Each must handle the new value.
• What the functions the PR calls do to state. The best find on #52 was not in its diff. The new Save button calls revalidate, which flips a stopped run to ready and so kills Resume. For each existing function a new button, route or script calls, read its body for side effects (UPDATE … SET state, cache invalidation, redirects) and ask which other screen depends on the old state.
• Every state the new UI can appear in, not just the happy one: running, done, stopped (failed, cancelled), no JavaScript, phone width.
• Every other way into the same data. Locked while X runs? Try each write path: editor save, upload API, delete, Undo, a second start, Cancel.
• Races. A check followed by an unconditional write (if ready … UPDATE) is a race. Fire 3 requests at once.
• Text the JS rewrites. Server-rendered text that a poller overwrites (the “rewritten or added” label became “written”).
• Owner-only. New routes must 404/400 for another user.
• Failure ends. Bad input, a model that returns junk, cancel, a restart. Does the page stay usable and say what happened in plain words?
• Project rules in CLAUDE.md and .claude/rules/: docs updated, data-testid on new markup, a journey for a new essential flow, fixtures for a new model call.

2. Baseline

zsh -ic 'cargo test 2>&1 | grep "^test result"'
zsh -ic 'scripts/smoke.sh | tail -3'
zsh -ic 'scripts/e2e.sh'            # all exact + smoke specs, Jev too

Record the numbers. A failure here is either the PR’s or already on main; settle which before going on (see pre-existing).

3. Explore with Jev (cheap, leads only)

Write a throwaway persona (e2e/personas/<name>.ts, goals start at /dev/login?open=spanish&keys=fake&next=<page> so AI forms are unlocked) and run zsh -ic 'scripts/e2e-explore.sh <name> --max-calls 150'. A run costs about half a cent and takes three minutes.

What to expect, from experience:

• Jev is good at finding unlabelled buttons, error text not in role=alert, overflow on a phone, state only in a class, and pages that never say what happened.
• Jev is bad at long flows with a background job. It clicks away while a job runs, hits Cancel, wanders to unrelated pages, and the page polls (4 s) so the end state arrives after it has left. 0 of 3 goals reached is normal.
• “No effect” findings are usually the harness: a confirm() dialog Playwright dismisses, or a required box left empty. Reproduce by hand before believing one.
• Findings on pages the PR didn’t touch (the tutor 404) go in “not this PR”.
• A Jev journey needs a short goal with an exact check and a start state that already exists. A “generate, then revise” journey can’t have either, so it was left out. Say that in the summary, and don’t ship a flaky one.
• A run covers one viewport. A persona that lists ["desktop", "phone"] ran only the first. Run the phone size as a second pass (viewports: ["phone"]). The overflow check is always on at phone width.
• When the flow has no background wait (a panel on an existing page), Jev reaches the goal in a few steps for a tenth of a cent. Its findings were low-severity wording and testability. Run it, but don’t trust it.

Delete the persona unless it is reliable. Never commit a profile that reaches 0 goals.

4. Write the deterministic specs (the real deliverable)

One file per feature: e2e/specs/<feature>.spec.ts, exact, no model in the loop. This pattern worked:

• Helpers at the top (makeCourse, revise, lessonTitles), so each spec reads as the user’s story.
• Drive scenarios with fixtures. [fixture:NAME] in the typed text picks a reply from data/test-fixtures/llm/<purpose>.json. Need a state that is too brief to catch (a job that finishes in 1 s)? Add a slow scenario with delay_ms instead of racing it.
• Wait for state, not time. Assert on data-* attributes, or expect.poll the JSON status API. Never waitForTimeout. Poll the API (not the DOM) once you’ve navigated away from the page that updates itself.
• Refusals. Use request.post(..., { maxRedirects: 0 }) and assert the 303 and the Location (the notice text), so you test the server and not just the UI.
• Dialogs. Call page.once("dialog", d => d.accept()) before the click. Test accept and dismiss, and read d.message() for the warning wording.
• Progress through the API. POST /api/progress/seen then read /api/progress/lessons/<id>. This is faster than clicking a lesson.
• A helper that clicks a <details> toggle closes it if it is already open. After goBack() or a reload the panel may be restored open. Load the URL fresh (page.goto(statusUrl)) before calling the helper.
• No JavaScript. Use test.describe(…, () => { test.use({ javaScriptEnabled: false }); … }). Use page.request (the page’s session), not the request fixture, which has none. Without scripts nothing polls, so reload until data-done is true.
• Each spec makes its own data (unique wish text), and afterAll resets.
• Cover, per PR: the happy path, each failure end, each lock, the second time (do it twice in a row), undo/back, and owner-only.

Tell a spec bug from a product bug

When a new spec fails, read the code before changing anything. In the first run, the spec caused three of three failures: a fixture that legitimately returned “nothing changed” the second time, a page that renders in place (no redirect), a check that never reloaded. Fix the spec if the product does what its code and body say. File or fix the product if a user would be surprised.

Prove the spec can fail

Copy the spec file to the scratchpad, break an assertion (or remove the fix), rerun, and copy it back. A spec that passes in half a second looks suspicious but may just be fast, and the break test settles it. Temporarily remove the fix (or break the behaviour) and restore it. A spec that stays green is not guarding what you think. Say so in the summary if you can’t make it fail (a double-submit spec once stayed green because another check already blocked the race, so the fix there was defence in depth).

5. Fix, file, or note

Pre-existing or caused by the PR?

Check what the PR touched (git diff --stat <base>...<head>) and whether the failing text or code is in it. A Jev-judged spec scoring the same low value on every run, on wording the diff doesn’t touch, is pre-existing. When it is cheap, check it against the base branch in a second worktree.

6. Re-run and push

zsh -ic 'scripts/e2e.sh'            # everything, not only your spec
zsh -ic 'cargo test 2>&1 | grep "^test result"'
zsh -ic 'scripts/smoke.sh | tail -1'

Commit specs and fixes together with a message that lists what is covered, and push to the PR’s branch. Specs for new fixtures and scenarios update docs/testing.md in the same commit.

Shell gotchas (macOS)

• sed -i needs an empty suffix argument (sed -i ''); edit with Python or the Edit tool instead.
• A pipe through grep after zsh -ic hides the build output; add grep -E "✓|✘|passed|failed|skipped|Expected|Received|^ +>" to read results and open e2e/results/artifacts/<test>/error-context.md for the page at the failure (it shows the accessibility tree, which usually explains it).
• Stacked PRs: specs pushed to the base PR’s branch reach the upper PR when it is rebased or merged. Say so in the summary so a conflict in e2e/specs/ doesn’t surprise anyone.

Summary template

## QA: PR #N <title>
**Verdict:** merge / merge after <X> / not ready — one sentence why.
**Checked at** `<8-character commit>`
**Stack:** base branch, merge order.
**Checks:** cargo test n · smoke ✓ · e2e a passed, b skipped, c failed (why).
**Bugs:** fixed (with spec) · filed (#issue) · none.
**Not this PR:** pre-existing findings.
**Coverage audit:**
| Claim in the PR | Covered by | Gap |
**Left uncovered, and why:** (flaky by nature, needs a real provider, …)

Post it with gh pr comment N --body-file …. A verdict names the blockers and says “none” when there are none. The author reads the verdict first. “Checked at” is the PR head after your own last push (git rev-parse --short=8 HEAD), in backticks, with nothing else on the line before it. It must match the demo manifest’s checked_at. pr-qa-demo’s find-prs.sh compares it with the PR head to tell when QA is out of date.

Efficiency notes

• Read the body and files first; they tell you which e2e/specs and templates matter. Skip the rest of the repo.
• Run one spec at a time while writing it (scripts/e2e.sh specs/x.spec.ts -g "name"), the whole suite once at the end. Each run builds and starts a server.
• Write all specs for a PR in one pass from the claims table, run, then triage failures together.
• One exploration run is enough. More runs of the same persona repeat themselves.
• Don’t spawn agents for this. The work is sequential and each step needs the last result.