Test Writing Discipline

Test-writing discipline

Green is not the goal — correct behavior is. A test that passes but asserts nothing, or asserts the buggy output, is worse than no test: it certifies the bug and reads as coverage. Never make a red test pass by weakening it. These rules keep agent-written tests meaningful. They apply on top of the framework guides (handsontable-playwright-e2e, handsontable-e2e-testing, handsontable-unit-testing).

The test encodes intent, not the implementation

Write the test from the requirement — the behavior the user or the API is supposed to have — not from what the code currently does. Where feasible, write it first (or independently of the implementation) so it is an oracle you cannot accidentally fit to a bug. For E2E, state the user-observable expectation (what the grid should show or do) before you wire a single selector.

When a test is red, decide what is actually wrong — don't just chase green

At the feature stage the code is the prime suspect, not the test. Before you touch the test, ask: does its expectation match the intended behavior?

• Expectation correct, code wrong → fix the code. This is the common case. Leave the test alone.
• Expectation genuinely mis-encoded the intent → fix the test by tightening it toward the real behavior — never by loosening it to match the current (possibly wrong) output.

If you cannot tell which is wrong, that is a signal to re-read the requirement, not to relax the test.

Banned ways of faking green

Never reach green by any of these — they defeat the point of the test:

• Deleting or loosening an assertion, or widening a tolerance, to match what the code emits.
• .skip / xit / xdescribe, or focusing with .only / test.only / describe.only / fit / fdescribe (focusing silently drops the rest of the suite).
• Wrapping the body in try/catch to swallow a failure.
• Asserting whatever the code happened to produce (a "snapshot of the bug").
• it.flaky / retries to paper over a real intermittent failure.

The Stop hook and pre-push run the test you touched and block on red — that forces you to reconcile red, it does not authorize you to make it green by weakening it. Reconcile by fixing the right thing.

Weakening over an admitted race needs a ticket in the same PR

An assertion weakened or deleted with a race as the reason — "held 15/15 locally", "flakes on classic", "racy on develop" — descopes a product defect; it does not fix a test. It ships only with a filed ticket in the same PR, named beside the change. "Still open, no ticket" is a review blocker. The strand-window fix (#13332) descoped two racy shapes that way, and the tickets are the only thing that keeps them from vanishing into a green run.

"Passes on retry" and "passes in isolation" are not determinism evidence

• A test that passed on its second attempt, or alone after failing inside the suite, has shown you the flake — not its absence. Before you call it deterministic, hammer it focused across every leg: cd tests && npx playwright test e2e/<spec>.spec.ts --repeat-each 20 (no --project filter, so all six theme × bundle legs run). The hidden-init migration needed ~700 such runs to expose three rAF-starvation timeouts that any single green run hid.
• A test that fails about half the time under that load on unchanged develop is reporting a product race. File a product ticket and keep the assertion — softening it to green certifies the race.

Bug fixes: write the failing test first

1. Reproduce the bug as a test and watch it fail — for the right reason (the missing behavior, not a typo or a bad selector).
2. Then apply the fix and watch the same test pass.
3. A regression test that was never red proves nothing. On a bugfix PR, name the spec that fails without the fix.

Cover what the change actually adds

• A timing-semantics claim is verified against the primitive that implements it. A JSDoc that says "closes when the task ends" above a setTimeout(0) is wrong: setTimeout(0) is the next macrotask, not end-of-task, so a synchronous caller in the same task runs inside the window the comment calls closed. That mismatch shipped a data-corruption bug — an editor stranded by alter() committed through stale coordinates and appended records (#13332). Read the primitive (setTimeout, queueMicrotask, requestAnimationFrame, a hook's call site) and write the test from what it does, not from what the comment says.
• A dedicated X-during-Y path requires a test that drives X during Y. flush, drain, cancel, pending, suspend, a depth counter — those names are the tell that the code handles one operation landing inside another. A test that runs X and Y in sequence never enters that path. Fire X from inside Y: a hook callback, a nested alter(), an edit inside a batch.
• Every documented form of a new option executes at least once — the guide's lead form especially. The shorthand a guide opens with is the form users try first, and the one most often left untested.

Verify before you say "done"

• Run the exact test command fresh, read the full output and the exit code, then state the result with that evidence.
• Banned phrasings: "should work", "this fixes it" without a run, "tested manually, looks fine". If you did not run it, say so.
• After editing source, run the impacted test — not the whole suite, the impacted one — and confirm green before finishing.

No hollow assertions

• Assert the behavior, not that the code ran. expect(getDataAtCell(0, 0)).toBe('x'), not expect(true).toBe(true) and not an it() with no expect at all.
• One meaningful assertion beats five that restate the setup.
• Coverage-on-new-code can be satisfied by a test that executes a line without checking its result — do not rely on it to judge quality; the assertion is what matters.

Don't mock the unit under test

• Never mock the thing you are testing. Mock at the real boundary (network, timers, ResizeObserver — see the mocks in test/__mocks__/).
• When you must build a mock, mock the complete real data shape; an incomplete mock gives a false pass. In this repo the default is to favor an E2E test over a unit test that would need to mock a module.

Fix broken legacy tests by migrating them

• The Jasmine suite is frozen. If a legacy *.spec.js is broken or flaky, do not patch it in place and do not add a sleep() — rewrite it as a Playwright test in tests/e2e/ and delete the Jasmine one. The tests that hurt most migrate first; the suite shrinks by attrition.
• Routine, low-risk edits to an existing Jasmine spec are still fine.