LearnNewsExamplesServices
Frontmatter
id18327
titleA unit test asserts playwright-core''s version string: it fires on every bump, detects no grammar change, and now blocks 19 updates
stateClosed
labels
bugaibuild
assigneesneo-opus-grace
createdAtSep 4, 2026, 9:26 PM
updatedAtSep 4, 2026, 11:56 PM
githubUrlhttps://github.com/neomjs/neo/issues/18327
authorneo-opus-grace
commentsCount0
parentIssuenull
subIssues[]
subIssuesCompleted0
subIssuesTotal0
contentTrust
projected
quarantined0
signals[]
blockedBy[]
blocking[]
closedAtSep 4, 2026, 11:56 PM

A unit test asserts playwright-core's version string: it fires on every bump, detects no grammar change, and now blocks 19 updates

neo-opus-grace
neo-opus-grace commented on Sep 4, 2026, 9:26 PM

Context

The first Dependabot version-update PR — #18325, bump the all-deps group with 19 updates, opened three minutes after #18322 merged — is red on unit because test/playwright/unit/e2e/browserLifecycleReporter.spec.mjs:39 asserts playwright-core is exactly 1.61.1 and the bump moves it to 1.62.1.

This ticket was originally filed to re-verify the grammar and move the pin. Operator challenge, 2026-09-04: "i would severely challenge unit tests that bind to specific versions. these have a negative ROI. utterly pointless." Verified, and the challenge is correct on a sharper basis than stated — the ticket is re-scoped from move the pin to delete it and replace it with a guard that can actually detect drift.

The Problem

The pin's stated purpose is to stop a Playwright upgrade silently inheriting a changed internal browser-log grammar — <launching> …, <launched> pid=…, [pid=…] <process did exit: exitCode=…, signal=…> — which classifyBrowserLaunchExit parses and which carries no compatibility promise. Real concern.

It cannot serve that purpose, and measuring the file shows why. All 12 tests build their input from the file's own launchExit() helper — a hand-written string constructor. Not one reads real Playwright output. The single reference to playwright-core anywhere in the suite is the version string at :39.

Two consequences, and the second is the load-bearing one:

  1. The assertion has ~100% false-alarm rate and 0% detection capability. It fires on every Playwright bump regardless of whether the grammar moved, and it can only ever detect a version change — never a grammar change. It tests npm's resolution, not Neo's behaviour.
  2. The 11 classifier tests are blind by construction. They validate the parser against strings the test itself wrote. If Playwright changed the grammar tomorrow, all 11 would still pass — confirming a reconstruction of a format that no longer exists. A probe you hand-feed can only confirm your reconstruction.

So the pin is not a backstop for the fixtures. It is the only signal in the file, it is a proxy for the thing it claims to guard, and its actual mechanism is "annoy a human into looking manually." That is a human process wearing a test's clothes — and the cost is now measurable: grouping is patterns: ["*"], so this one assertion blocks all 19 updates, and Dependabot re-opens the group daily against the same red.

The Fix

Delete the version assertion. It tests a package manifest, not this repository's behaviour, and its failure mode is indistinguishable from success-plus-noise.

Then close the real gap it was standing in for, in order of preference:

  1. Preferred — one probe against real output. Force a genuine launch failure (an invalid executablePath is the cheapest route), capture Playwright's actual error text, and assert classifyBrowserLaunchExit classifies it. That fires only when the grammar actually drifts: true signal, no bump-noise, and it makes the other 11 fixture tests meaningful by proving the shape they reconstruct is still real.
  2. Fallback — if a real launch cannot run in unit scope, delete the assertion anyway and record honestly in the spec docblock that the fixtures are a reconstruction with no live witness, naming the residual risk. An explicit unguarded risk is strictly better than a guard that cannot detect the thing it names.

Either way the dependency queue unblocks permanently instead of once per Playwright release.

Contract Ledger Matrix

Target Surface Source of Authority Proposed Behavior Fallback Docs Evidence
the version assertion (:39) operator challenge 2026-09-04 deleted keeping it ⇒ every Playwright bump reds unit and blocks the group spec docblock literal toBe('1.61.1'); fires on version, not grammar
grammar drift detection #16161's actual concern one probe against real launch-exit output infeasible ⇒ documented reconstruction + named residual risk spec docblock 12/12 tests use the hand-written launchExit() fixture
the 11 classifier tests shipped source unchanged — they stay valid as unit coverage of the parser — — they exercise branch logic, which is a different claim from grammar fidelity
the grouped update queue .github/dependabot.yml unblocked permanently, not per release — — #18325 red on this arm alone

Acceptance Criteria

  • The toBe('1.61.1') assertion is deleted, and the test name no longer carries a version.
  • Either a probe asserts classifyBrowserLaunchExit against real Playwright launch-exit output, or the spec docblock states that the fixtures are a reconstruction with no live witness and names the residual risk. Not both, not neither.
  • Non-vacuity, if the probe lands: mutating the parser's expected grammar reds the probe. A probe that passes against a deliberately wrong parser is another reconstruction.
  • The 11 existing classifier arms still pass unchanged — this removes a proxy, it does not reduce parser coverage.
  • unit is green and #18325 (or its daily successor) reaches green on unit.
  • The PR body states which of the two paths was taken and why, so the next reader knows whether a live witness exists.

Out of Scope

  • Changing the grouping strategy in .github/dependabot.yml. Fix the bad guard first; if batching still hurts afterwards, splitting the group is a separate, evidenced decision.
  • The other 18 updates in #18325 — not implicated; unit fails on this arm alone.
  • A general sweep for other version-bound assertions. If this pattern exists elsewhere it deserves its own ticket with its own census, not a drive-by.

Avoided Traps

  • Reading the red as "Dependabot is broken." The opposite: the config worked on its first run, majors flowed (chalk 5→6, cssnano 7→9, inquirer 13→14), and the failure surfaced pre-merge on the bot PR rather than on dev.
  • Moving the pin to 1.62.1. That was this ticket's original prescription and it is wrong: it buys one release of quiet and re-arms the same false alarm, while still detecting nothing.
  • Deleting the assertion and calling the gap closed. The concern behind it is real. Removing a bad guard without either replacing it or naming the residual is how a real risk becomes an invisible one.
  • Treating the 11 fixture tests as grammar coverage. They are parser-branch coverage. Both are worth having; only one of them is what #16161 was worried about.

Related

#18325 (the Dependabot PR that surfaced it) · #18302 → PR #18322 (version updates, merged dev@55920c5a79) · #16161 (closed — the origin of the pin and the real concern behind it) · test/playwright/unit/e2e/browserLifecycleReporter.spec.mjs · test/playwright/e2e/custom-reporter.js


Live latest-open sweep: latest 12 open issues at 2026-09-04T19:22Z; no equivalent. Duplicate sweep: open and closed searches for a Playwright pin/version ticket returned nothing; #16161 is closed and is the pin's origin, not a duplicate. A2A claim sweep: no claim on this scope. Memory Core: semantic recall serving stale 2025 hits today — recorded as a null, not a clean sweep.

Decision Record impact: none.

Origin Session ID: b983b2a8-18bc-4dd0-a63f-23380dc3a49d