Skip to content

fix(verify): read the checks playwright reports as failed - #1978

Open
rafaelnovah1 wants to merge 2 commits into
Agent-Field:devfrom
rafaelnovah1:fix/verify-playwright-failures
Open

rafaelnovah1 wants to merge 2 commits into
Agent-Field:devfrom
rafaelnovah1:fix/verify-playwright-failures

Conversation

@rafaelnovah1

Copy link
Copy Markdown

What changed

verify.FailingTests did not recognize any failure in Playwright output. The
project already detects Playwright as a test runner (playwright.config.* in
discovery.go), but when a Playwright suite was red, its reading named zero
failing checks. As a result, the gate could not tell which checks a change
turned red.

Playwright's list, line and dot reporters all end with a summary like
this:

  4 failed
    [chromium] › tests/e2e/login.spec.ts:3:7 › login › rejects a wrong password ────
    [chromium] › tests/e2e/login.spec.ts:6:5 › shows the footer ────
    ...
  1 flaky
    [chromium] › tests/e2e/flaky.spec.ts:2:5 › loads the dashboard ────
  2 passed (563ms)

This PR reads the N failed section and nothing else:

  • Only N failed, not the numbered banners. A test that fails and then
    passes on retry still gets a 1) … banner. Reading the banners would report a
    flaky-but-green check as a new failure. The ✘ progress lines are skipped too,
    because their leading counter changes with worker scheduling.
  • Project kept. [chromium] and [firefox] are two checks, so the bracketed
    project stays in the name.
  • Line and column stripped. login.spec.ts:3:7 becomes login.spec.ts
    inside a › chain, in normalizeTestName. Otherwise, adding a line above a
    test would make an unchanged red check look new. The strip applies only to
    file.(js|ts|…):line:col ›, so rspec's ./spec/a_spec.rb:12 keeps its line
    number.

Scope: this only changes the failing-check reading. ReportedTests (the roster)
still reads Playwright's ✓ lines with their counter. I left that alone so this
change stays small, and can follow up if useful.

How it was checked

  • Fixtures are real output of @playwright/test 1.64.1. They cover the list
    reporter with two projects, the same suite after a line was inserted above
    every test, the line reporter with no projects, and a retry run with one flaky
    test. The absolute workspace path was replaced with /work/app.
  • On dev without the change, the new test fails: every fixture reads [].
    With the change, it passes.
  • An extra test checks that rspec's file:line identity is unchanged and that
    mocha's numbered 1) … failure list is not read as Playwright.
  • go test ./internal/verify/... ./internal/revision/...: pass.
  • make test-quick: pass on macOS (arm64), Go 1.27.3.
  • Full go test -timeout 25m ./internal/session: one failure,
    TestTheDailyCeilingBindsTaskAndErrandTurnsToo (daily limit reached · $1.07 spent of $1). It also fails on a clean dev worktree run at the same
    time, so this change did not cause it. It passes when run on its own
    (-count=5). It looks like daily spend shared between tests that run in
    parallel. I can open a separate issue if useful.

Context: we run a fork of SWE-AF whose port of failing.go added this reading
for a Next.js + Playwright project. This PR brings the fix back here, improved
to handle flaky tests and moving line numbers. Written with help from an AI
coding agent and reviewed by me.

Checklist

  • A change entry: docs/changes/unreleased/<pr>-playwright-failing-checks.md (kind: fixed)
  • The manual knows about it: no slash command, key, tool, default, limit or refusal moved
  • Only my own paths are staged: no git add -A

FailingTests read nothing out of a red playwright suite, although discovery
already treats playwright.config.* as a test runner. Read the closing
`N failed` section of the list, line and dot reporters: it excludes checks
that passed on retry (listed under `N flaky`), keeps the project in the
name, and drops the spec file's line and column so a check does not look new
after an edit above it.
@CLAassistant

CLAassistant commented Oct 26, 2026 •

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

Copy link
Copy Markdown
Member

Hey, thanks for the PR. Could you sign the CLA when you get a chance so we can review it?

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants