Skip to content

docs: state the testing bar in CONVENTIONS, COMPONENT_SPEC, and AGENTS - #1381

Merged
cody-dot-js merged 1 commit into
mainfrom
docs/testing-conventions
Jul 29, 2026
Merged

docs: state the testing bar in CONVENTIONS, COMPONENT_SPEC, and AGENTS#1381
cody-dot-js merged 1 commit into
mainfrom
docs/testing-conventions

Conversation

@cody-dot-js

Copy link
Copy Markdown
Collaborator

Lands the conventions from #1367 ahead of the test changes themselves, so the rules are in force for new work while that PR is still in review. Docs-only in intent — no shipped source changes, so no changeset.

What's here

CONVENTIONS.md § Testing gains the rules the audit behind #1367 showed were missing:

  • A test must be able to fail — name the one-line implementation change it catches. Bans toBeDefined() / not.toThrow() as a test's only assertion, tautologies, unmatchable queries, and call-count-blind spies.
  • Drive the interaction — a component with an event handler, controlled prop, or keyboard contract needs a real event, not an assertion about its initial markup.
  • Assert behavior, not styling internals — why Tailwind utility-string assertions are not coverage, with the three legitimate carve-outs spelled out.
  • Pin cross-file contracts — when a selector in one file depends on an attribute emitted in another, assert both sides in one test.
  • Cover the server renderrenderToString for SSR branches; evaluate stringified inline scripts.
  • Determinism — no arbitrary sleeps, spies installed after userEvent.setup(), no order dependence.

COMPONENT_SPEC.md §8 and the AGENTS.md diff-audit checklist point at the same bar.

Why the Vitest configs came along

CONVENTIONS.md § Determinism now states that every test-bearing package sets restoreMocks, unstubEnvs, and unstubGlobals, and that a trailing spy.mockRestore() is therefore dead code. Without the configs that sentence is false, and it would tell a reader to delete teardown that is still load-bearing. All five packages the bullet names now set the flags.

packages/mantle also pins TZ/LC_ALL in the config rather than only in its test script, so the pin survives a single-file or editor-driven run, and pins Chromium's own locale/timezoneId through the Playwright contextOptions, which process.env cannot reach.

The one test change

restoreMocks restores spies before each test, which breaks a spy installed once in beforeAll. integer-ticks.browser.test.tsx recorded canvas fillText calls that way, so all 4 of its tests read an empty label array under the new config. Its spy moves to beforeEach — the same beforeAllbeforeEach hazard the new § Determinism bullet documents. This is required to keep the suite green, not new coverage; it was the only file in the repo with that pattern.

Numbers restated against this branch

COMPONENT_SPEC.md's scope-and-status counts in #1367 describe the tree after its test work. Re-derived against main: 20 of 67 component directories ship no test file (not 13). The other three counts in that sentence — 67 directories, 20 with no asChild, 4 with no data-slot, exactly one docs page with a data-attribute table — hold as written.

Known pre-existing flake, left to #1367

code-block.test.tsx → "fires onCopyError when clipboard write fails" fails under shuffled order (seed 7 of the 4 tried). It patches navigator.clipboard.writeText before userEvent.setup(), which replaces navigator.clipboard wholesale. Verified identical on clean main at the same seed — this branch neither causes nor worsens it, and #1367 fixes it. It is the exact violation the new "install spies after userEvent.setup()" bullet describes.

Verification

```
lint 0 errors
fmt:check clean (722 files)
typecheck 7/7 tasks, 0 errors
test 8/8 tasks green
@ngrok/mantle 127 files, 1821 tests
@app/www 21 files, 169 tests
@ngrok/mantle-vite-plugins 5 files, 123 tests
@ngrok/mantle-server-syntax-highlighter 7 files, 109 tests
shuffle seeds 1, 42, 1234 green; seed 7 hits the pre-existing code-block flake above
```

Lands the conventions from #1367 ahead of the test changes themselves, so
the rules are in force for new work while that PR is still in review.

CONVENTIONS.md § Testing gains the rules the audit behind #1367 showed were
missing: a test must be able to fail (name the one-line change it catches),
drive the interaction with a real event, assert behavior rather than Tailwind
utility strings, pin contracts that cross files, cover the server render, and
determinism. COMPONENT_SPEC.md §8 and the AGENTS.md diff-audit checklist point
at the same bar.

The Vitest configs come along because CONVENTIONS.md § Determinism now states
that every test-bearing package sets `restoreMocks`, `unstubEnvs`, and
`unstubGlobals`, and that an inline `mockRestore()` is therefore dead code.
Without the configs that instruction is false, and it would tell a reader to
delete teardown that is still load-bearing. All five packages the bullet names
now set the flags. `packages/mantle` also pins TZ/locale in the config rather
than only in its `test` script, so the pin survives a single-file run.

`restoreMocks` restores spies *before each test*, which breaks a spy installed
once in `beforeAll` — `integer-ticks.browser.test.tsx` recorded canvas
`fillText` calls that way, so its per-test spy moves to `beforeEach`. That is
the only test change here; it is required to keep the suite green, not new
coverage.

COMPONENT_SPEC.md's scope-and-status counts are restated against this branch
(20 of 67 component directories ship no test file), not against #1367's
post-audit numbers.
Copilot AI review requested due to automatic review settings July 29, 2026 21:32
@changeset-bot

changeset-bot Bot commented Jul 29, 2026

Copy link
Copy Markdown

⚠️ No Changeset found

Latest commit: 10a1aa1

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@ngrok-ship

ngrok-ship Bot commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

🚀 Deploy Previews

Updated 2026-07-29 21:34 UTC

App URL Commit Status
ngrok-mantle https://mantle-1381.ngrok-previews.ngrok.app 10a1aa1 🟢 Running

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Aligns repo documentation and test-runner configuration around a stricter, explicit testing bar (ability-to-fail, event-driven interaction, determinism), and makes Vitest/Playwright defaults match what the docs now promise.

Changes:

  • Expands CONVENTIONS.md testing guidance with concrete rules on interaction, assertions, determinism, and cross-file contracts.
  • Updates COMPONENT_SPEC.md and AGENTS.md to point at (and restate) the same testing bar.
  • Hardens Vitest configs across packages/apps with centralized mock/env/global cleanup, pins locale/timezone for Mantle (including Chromium locale/timezone via Playwright context options), and adjusts one browser test to avoid beforeAll spies under restoreMocks.

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated no comments.

Show a summary per file
File Description
packages/mantle/vitest.config.ts Adds centralized mock/global/env hygiene and pins deterministic locale/timezone (including Playwright context locale/timezoneId).
packages/mantle/src/components/chart/integer-ticks.browser.test.tsx Moves a long-lived canvas spy from beforeAll to beforeEach to remain compatible with restoreMocks.
packages/mantle-vite-plugins/vitest.config.ts Enables centralized mock/global/env cleanup for determinism across tests.
packages/mantle-server-syntax-highlighter/vitest.config.ts Introduces Vitest config with centralized cleanup and TZ pin moved from scripts to config.
packages/mantle-server-syntax-highlighter/package.json Removes script-level TZ=UTC since TZ is now configured in Vitest config.
CONVENTIONS.md Documents the testing “bar” (tests must be able to fail), interaction-driven tests, and determinism rules.
COMPONENT_SPEC.md Updates scope/status counts and strengthens the testing minimums to match CONVENTIONS.md.
apps/www/vite.config.ts Enables centralized mock/global/env cleanup for determinism in app tests.
AGENTS.md Updates the diff-audit checklist to reference the new testing bar and common failure modes.

@cody-dot-js
cody-dot-js merged commit 22f6132 into main Jul 29, 2026
13 checks passed
@cody-dot-js cody-dot-js self-assigned this Jul 29, 2026
@cody-dot-js
cody-dot-js deleted the docs/testing-conventions branch July 29, 2026 21:41
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.

2 participants