test: name tests by the tier they actually occupy, and guard it - #340
Merged
Conversation
Nine files were named `*.e2e.test.ts` and ran on every commit inside run-tests.sh. None of them requires a live system: they spawn the real built CLI against fake services. Good tests, wrong label. The cost was not cosmetic. "the E2E tests pass" and "the E2E tests ran" meant different things depending on who said them — the genuinely live suite is in the monorepo (`bun run tests/e2e/e2e.ts`) and runs separately. A green run-tests.sh could be offered, honestly and wrongly, as evidence that an end-to-end path had been exercised against staging. *.test.ts unit — nothing spawned *.int.test.ts real code paths, real spawned CLI, faked boundaries *.browser.test.ts as above, plus a real headless browser over CDP *.e2e.* RESERVED for live systems; none belong in this package ONE FILE IS NOT WHAT THE INVENTORY SAID. browserFlow was reported as touching nothing live because a search for `playwright`/`chromium` came back empty. It drives a REAL headless Chrome over CDP — `findHeadlessShell`, `CdpSession`, chrome-headless-shell — so calling it `.int.` would have replaced one inaccurate label with another. It gets `.browser.` because it needs a binary that is not in this repo and not in `bun install`, and a run that silently skipped it would otherwise look identical to one that passed it. The guard is the part that matters more than the rename: a test that fails if any `.e2e.` file exists in this package, with a message naming the offender and where its tier is actually run. It lives in the test suite rather than in run-tests.sh because anyone can type `bun test` and CI can grow a second entrypoint. Proven able to fail: adding a stub `zzTempDrift.e2e.test.ts` turns it red; removing it returns green. It also carries its own control — an empty offenders list is the passing state AND the result a broken directory walk produces, so the scan asserts it can see files at all. Suite: 1851 pass, 0 fail.
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.
Nine files were named
*.e2e.test.tsand ran on every commit insiderun-tests.sh. None requires a live system — they spawn the real built CLI against fake services. Good tests, wrong label.The cost is not cosmetic: "the E2E tests pass" and "the E2E tests ran" meant different things depending on who said them. The genuinely live suite is in the monorepo (
bun run tests/e2e/e2e.ts) and runs separately, so a greenrun-tests.shcould be offered — honestly and wrongly — as evidence that an end-to-end path had been exercised against staging.The taxonomy
*.test.ts*.int.test.ts*.browser.test.ts*.e2e.*One file is not what the inventory said
browserFlowwas reported as touching nothing live, because a search forplaywright/chromiumcame back empty. It drives a real headless Chrome over CDP —findHeadlessShell,CdpSession,chrome-headless-shell. Calling it.int.would have swapped one inaccurate label for another.It gets its own
.browser.tier because it needs a binary that is neither in this repo nor inbun install, and a run that silently skipped it would otherwise look identical to one that passed it.The guard matters more than the rename
A test that fails if any
.e2e.file exists here, naming the offender and where its tier is actually run. It lives in the suite rather than inrun-tests.sh, because anyone can typebun testand CI can grow a second entrypoint.Proven able to fail: adding a stub
zzTempDrift.e2e.test.tsturns it red; removing it returns green. It also carries its own control — an empty offenders list is both the passing state and what a broken directory walk produces, so the scan asserts it can see files at all.Suite: 1851 pass, 0 fail.