Skip to content

test: drop cases that re-assert their own input or a stronger sibling - #2933

Merged
thymikee merged 1 commit into
mainfrom
test/remove-tautological-and-subsumed-cases
Sep 25, 2026
Merged

thymikee merged 1 commit into
mainfrom
test/remove-tautological-and-subsumed-cases

Conversation

@thymikee

Copy link
Copy Markdown
Member

Summary

Eight tests could not fail for a reason worth hearing. Net −203 test lines, 0 production
lines changed
. Each was checked against its production owner and the stronger test that
already covers it:

Site Why it cannot fail
ax-digest.test.ts computeAxDigest(nodes) vs a second call to the same pure function; the empty-array case keeps its nodeCount pin, loses the self-comparison
ios-snapshot-engine/conformance.test.ts replays ios-snapshot-capture-hint.json through createIosSnapshotRequest + deriveIosCaptureHint with the same four assertions ios-snapshot-planning.test.ts makes; the golden table keeps its owning leg
replay-coordinator-ownership.test.ts divergence-chain import ban is a strict subset of the src/daemon/replay/-wide scan beside it (same module, same predicate)
device-claim-rule.test.ts production compared to a hand-copied test table of the three rules the same file asserts directly; exhaustiveness is production's default-less switch
snapshot-viewport-root.test.ts over the complete 31-name vocabulary, "equals Application or Window" is the same proposition as the preceding "filters to exactly those two"
install-source-public.test.ts same URL its sibling asserts at the owner + a typeof probe; subpath proven by package-exports and the installed-tarball probe
metro-protocol-public.test.ts runtime half repeated metro-public.test.ts verbatim; its compile-time wire fixtures move there, so src/sdk/metro.ts has one public-surface test
session-replay-command-trust.test.ts mocked the façade and read the command field back; the retained suite drives the same two real handlers to the terminal trustedScripts

Two candidates I proposed to myself and rejected while editing — click-runtime-execution.test.ts's
toBe (deep-equal on lines 20-21 does not subsume strict identity, and the file's comment says
the #1900 claim rests on that identity) and snapshot-quality-backend-capabilities.test.ts:80
(its forceable subset is derived at the type level from kernel SnapshotPreferredBackend, so the
pin has a real failure mode). Both kept.

Validation

Tested at a836be12e.

  • Mutation proof for the deleted security file. Deleting the internal.publicNetworkOnly
    mapping in replay-dispatch-envelope.ts turns
    session-replay-maestro-remote-evalscript.test.ts's two remote cases red
    (2 failed | 2 passed). Reverted; git diff on that file is empty.
  • 11 focused owner/sibling files: 67 tests green.
  • pnpm check:affected --run — all runnable checks passed.
  • oxlint . --deny-warnings, repo-wide oxfmt, package tsc -b, root tsc -p tsconfig.json.

Not device-facing: no production path moves, so no device lane is selected.

Eight tests could not fail for a reason worth hearing. Each was checked against
its production owner, its callers, and the stronger test that already covers it.

- `ax-digest.test.ts`: `computeAxDigest(nodes)` compared against a second call to
  the same pure function — true by construction. The empty-array case kept its
  `nodeCount` assertion and lost its self-comparison.
- `ios-snapshot-engine/conformance.test.ts`: replayed
  `contracts/fixtures/ios-snapshot-capture-hint.json` through
  `createIosSnapshotRequest` + `deriveIosCaptureHint` with the same four
  assertions `ios-snapshot-planning.test.ts` makes one file away. The golden table
  keeps its owning leg; only the second copy goes.
- `replay-coordinator-ownership.test.ts`: the divergence-chain import ban is a
  strict subset of the `src/daemon/replay/`-wide scan beside it, same module and
  same predicate, so it cannot fire without that one firing first.
- `device-claim-rule.test.ts`: compared production against a hand-copied test
  table of the three rules the same file already asserts directly. Exhaustiveness
  is production's own default-less switch.
- `snapshot-viewport-root.test.ts`: over the complete 31-name iOS vocabulary the
  preceding case fixes, "equals `Application` or `Window`" is the same
  proposition as "filters to exactly those two".
- `install-source-public.test.ts`: asserted the URL its sibling asserts at the
  owner, plus a `typeof` probe. The subpath is proven by `package-exports` and by
  the installed-tarball probe that calls through it.
- `metro-protocol-public.test.ts`: its runtime half repeated `metro-public.test.ts`
  verbatim; the compile-time wire fixtures it also carried move there, so the
  public metro surface has one test file mirroring `src/sdk/metro.ts`.
- `session-replay-command-trust.test.ts`: mocked the replay façade and read the
  command field back. `session-replay-maestro-remote-evalscript.test.ts` drives
  the same two real handlers and asserts the terminal `trustedScripts` the engine
  receives. Proven by mutation: dropping the mapping in
  `replay-dispatch-envelope.ts` turns that suite's two remote cases red.
@github-actions

Copy link
Copy Markdown
Contributor

Size Report

Metric Base Current Diff
Installed (including dependencies) 4.81 MB 4.81 MB 0 B
Package (unpacked) 4.81 MB 4.81 MB 0 B
Package (download) 1.44 MB 1.44 MB +12 B

Startup median (7 runs, lower is better):

Scenario Base Current Diff
CLI --version 28.1 ms 27.7 ms -0.3 ms
CLI --help 81.1 ms 80.7 ms -0.4 ms

@thymikee

Copy link
Copy Markdown
Member Author

Reviewed at a836be1. This is test-only and ready for human review.

The removed cases each re-asserted an already-covered input or duplicated a stronger sibling, so nothing was lost. CI is green: 17/17 checks pass, and the diff is test-only (+0/-0 production), consistent with no production behavior change.

Not blocking: src/tests/install-source-public.test.ts:11 dropped the only assertion importing from the public entry src/sdk/install-source.ts, and no remaining test smokes validateDownloadSourceUrl through that entry (installed-package-metro.test.ts:318 only covers isTrustedInstallSourceUrl), so you could add that one function check back into the ./install-source smoke, or leave it since this is optional.

@thymikee thymikee added the ready-for-human Valid work that needs human implementation, judgment, or maintainer merge label Sep 24, 2026
@thymikee
thymikee merged commit 614bfc1 into main Sep 25, 2026
17 checks passed
@thymikee
thymikee deleted the test/remove-tautological-and-subsumed-cases branch September 25, 2026 05:44
@github-actions

Copy link
Copy Markdown
Contributor
PR Preview Action v1.8.1
Preview removed because the pull request was closed.
2026-09-25 05:44 UTC

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

Labels

ready-for-human Valid work that needs human implementation, judgment, or maintainer merge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant