# Unit test coding style

How a unit test file in this repository is shaped: where it lives, what it is
called, how its blocks nest, and which assertions to reach for.

Running the suites is covered in [`TESTING.md`](TESTING.md). Integration and
browser tests answer to different conventions —
[`UI_TESTING.md`](UI_TESTING.md) for reaching into shadow DOM, and
[`waiting-in-tests.md`](waiting-in-tests.md) for the rule against polling,
which applies to unit tests too.

Pattern tests are a different form again, and none of this guide reaches them.
A pattern test is itself a pattern: it instantiates the pattern under test,
drives it with `action()`, states each expectation with `assert()` from
`commonfabric`, and returns an ordered `tests` array for the runner to walk.
`assert()` is preferred over a `computed()` boolean there, because a failed
`assert()` names its operands and the values they held while a failed
`computed()` can only say it expected `true` — the comparison ran inside the
closure and only the verdict survived.
[Pattern testing](../common/workflows/pattern-testing.md) is the guide, and
its "Prefer `assert()` over `computed()`" section is the one to read first. A
`.test.tsx` file may be either form, so establish which one you are in before
editing it.

Not every test file in the tree follows this guide, so a neighboring file is
not evidence of what to write. A new file follows this guide closely. An edit
to an existing file conforms the area around the edit; converting a whole file
is its own change, made deliberately rather than as a side effect.

## What a test checks

A test checks the behavior that users care about, not implementation details,
so a correct refactor leaves it unchanged. It fails when that behavior breaks;
check this by breaking the behavior and running the test.

A test checks the negative as well as the positive: the code acts where it
should, and does not act where it should not. A test of a filter checks that it
keeps a match and that it drops a non-match.

## Where a test file lives, and what it is called

Unit tests go in a `test` directory directly under the package they test, not
beside the source file. Where the source tree has subdirectories, the `test`
tree mirrors them: a test for `src/codec-json/encode.ts` lives at
`test/codec-json/encode.test.ts`. That keeps a test findable from its source,
and its source findable from the test, as a package grows.

That rule is about a package of interdependent source files. A directory whose
entries are instead independent components — each self-contained, with no
shared structure for a parallel tree to mirror — keeps each component's test
beside it, and that arrangement is correct where it is already in use.
`packages/ui` is one, a component and its test sitting together in the
component's own directory. `packages/patterns` is another, where a pattern and
its test are a pair; its tests are pattern tests, so nothing else in this guide
reaches them, but the placement question is the same one. A `test/` tree buys
nothing in either case: what it would mirror is a flat list of unrelated
components, and splitting a pair across two trees makes both halves harder to
find. Follow whichever arrangement the directory already uses.

Name the file `<topic>.test.ts`, where `<topic>` is one of:

- The base name of the source file under test — `value-clone.test.ts` for
  `value-clone.ts`. This is the default.
- The name of a single class, function, or other export, when the file tests
  only that one thing — `SchemaAndHash.test.ts`. Prefer this when it applies.
- A `lower-kebab-case` phrase describing the subject, when the tests span
  several source files — `donut-cross-check.test.ts`.

A test file most typically concerns one main source file, but that is a default
rather than a requirement. Splitting one source file's tests across several
files is fine, especially when one of them goes deep on a single function.
Covering several source files from one test file is fine when that makes the
tests clearer or less repetitive. Being sensible beats holding the line.

## Structure

Use the BDD functions from `@std/testing/bdd` — `describe()`, `it()`,
`beforeEach()`, `afterEach()` — and `@std/expect` for assertions.

### One top-level `describe()`

Every test file has a single top-level `describe()` call containing everything
else. Its title is the name of the file under test without the source suffix:
`describe("donut-weight", ...)` for `donut-weight.ts`. As a special case, when
the file tests only one class, function, or other export, the title names that
export: `describe("DonutWeight", ...)` for a class, and
`describe("eatDonut()", ...)` for a function. That is the same condition under
which the file itself is named for the export rather than for a source file.

There is one carve-out. When a file parameterizes the same test body across a
fixed set of implementations — typically one `describe()` per implementation,
generated by a loop — those generated calls may each be top-level. The expected
shape is one top-level `describe()` for the shared API surface, plus one per
implementation.

Group related tests with nested `describe()` blocks rather than comment
banners. A `describe()` title appears in the test run transcript, where it
tells a reader which group failed; a comment does not. A comment that turns out
to describe several adjacent tests is the usual sign that a group is missing;
see [Commenting a block](#commenting-a-block).

### Each case stands on its own

A case may not depend on another case in the file having run. What a case
needs goes in the case or in a hook, and that reaches process-wide setup as
much as it reaches a local fixture: a one-time initialization another case
happens to trigger is setup like any other.

A whole-file run passes either way, so nothing local shows the dependence.
What shows it is a lane of a continuous-integration run, which takes part of
a file and skips the rest, so the case whose setup came from a sibling fails
there and nowhere else.
[Test selection](test-selection.md#a-case-that-fails-only-when-its-siblings-do-not-run)
holds the recipe for running one case of a file with its siblings skipped.

The shuffle every run applies does not find such a dependence either, and the
shape this guide asks for is why. `deno test --shuffle` reorders a run's files
and each file's top-level registrations, and a file holding one top-level
`describe()` registers one test whose cases are its steps, which keep their
order. So a case leaning on a sibling is caught by a lane that skips that
sibling, and by nothing else.
[Every test run shuffles its order](TESTING.md#every-test-run-shuffles-its-order)
covers what the shuffle does reach.

### Describing a class

Under the `describe()` naming a class, use literally
`describe("constructor()", ...)`, `describe("instance members", ...)`, and
`describe("static members", ...)` to hold the tests for its members. A test
that fits none of them — one cross-checking a static member against an instance
member, say — can sit directly under the class's block.

The `constructor()` block usually holds a plain sequence of `it()`s. Each
`... members` block usually holds one `describe()` per member, each of which in
turn holds `it()`s. Either may grow another layer of `describe()` when a group
of related cases earns one.

```ts
import { describe, it } from "@std/testing/bdd";
import { expect } from "@std/expect";

class DonutWeight {
  static readonly zero = new DonutWeight(0);

  constructor(readonly grams: number) {
    if (grams < 0) throw new Error("Weight cannot be negative.");
  }

  plus(other: DonutWeight): DonutWeight {
    return new DonutWeight(this.grams + other.grams);
  }
}

describe("DonutWeight", () => {
  describe("constructor()", () => {
    it("throws given a negative weight", () => {
      expect(() => new DonutWeight(-1)).toThrow();
    });
  });

  describe("instance members", () => {
    describe("plus()", () => {
      it("returns the sum of the two weights", () => {
        expect(new DonutWeight(3).plus(new DonutWeight(4)).grams).toBe(7);
      });
    });
  });

  describe("static members", () => {
    describe("zero", () => {
      it("is a weight of zero grams", () => {
        expect(DonutWeight.zero.grams).toBe(0);
      });
    });
  });
});
```

### Describing a function

A top-level function gets a `describe()` of its own, holding a sequence of
`it()`s, in the same shape as a method's block above. Where the file tests only
that function, that block is the file's single top-level `describe()`.

### Writing the description strings

An `it()` description reads as a third-person singular verb phrase completing
the word "it": "returns `undefined` for an empty list", not "should return
`undefined`" and not "empty list".

Prefer describing the **observable output** over ascribing an intent. "returns
`false` for a bare function" beats "rejects a bare function": both are verb
phrases, but the first states a checkable fact about what the code does, and
the second describes a motive that no assertion can confirm.

Where the fact is a return value, the verb is **"returns"**. Specifically not
"answers": that casts the call as a question put to the code, which is a
metaphor standing in for a word that is already exact and already shorter. It
also splits the vocabulary of the suite, so that two files pinning the same
kind of fact no longer read as though they do. This is one instance of the
[word choice](DEVELOPMENT.md#one-word-per-concept) the repository standardizes
on, which holds outside tests as well.

Mark up descriptions in Markdown, the same as a comment — backticks around code
and code-like things. The one exception is a `describe()` whose title would
otherwise be entirely one backticked string: write `describe("eatDonut()")`,
not ``describe("`eatDonut()`")``.

Description strings may run past the 80-column line width the rest of the
repository holds to. A description that reads well is worth more than a
description that wraps well.

Changing a description renames the test. The description string, together with
the `describe()` chain above it, is the test's identity in the run-record
store, so a reworded description starts a fresh identity and leaves the old
one's history where it was. Append a line under
`tasks/test-identity-aliases/` to join the two halves, as described in
[test records](test-records.md). No gate reports a rename that was never
bridged.

### Commenting a block

A comment about a test, or about a group of tests, goes inside the block it
describes, as the first thing in the callback, followed by a blank line:

```ts
import { describe, it } from "@std/testing/bdd";
import { expect } from "@std/expect";

declare class DonutWeight {
  constructor(grams: number);
  readonly grams: number;
  plus(other: DonutWeight): DonutWeight;
}

describe("instance members", () => {
  // Comparing two `DonutWeight`s would rest on whatever equality the class
  // gives. Each case reads `grams` off the result instead, so what the
  // assertion turns on is the arithmetic alone.

  describe("plus()", () => {
    it("returns the sum of the two weights", () => {
      expect(new DonutWeight(3).plus(new DonutWeight(4)).grams).toBe(7);
    });
  });
});
```

Above the call, such a comment is held in place by adjacency and nothing else.
A block inserted at that line lands between the two, and the comment then heads
a group it was not written for while the group it describes has none. Inside
the callback there is nothing to insert between them, and the indentation says
how far the comment reaches.

The blank line under it separates the two jobs a `//` comment does here. With
one, the comment is about the block. Without one, it is about the statement
directly beneath it, which is the ordinary use and is untouched by any of this.

This holds for a test and for a group of them: `describe()`, `it()`, and
`Deno.test()`, `Deno.bench()` and `t.step()` where a file uses them. It does
not reach the `beforeEach()` family. A hook is setup rather than a case, and
reads the way any other statement does with a line or two of `//` over it.

A callback that opens no body of its own has nowhere to hold a comment, and the
note stays beside it. Three shapes do that — a body that is a bare expression,
an empty body written on the opener's line, and an options object whose `fn` is
the name of a function declared elsewhere:

```ts
// Shown for illustration only.

it("returns `undefined` for a bad name", () => expect(f(x)).toBeUndefined());
Deno.test("would-pass", function () {});
Deno.test({ name: "derive array leak", fn: runTest, sanitizeOps: false });
```

This is the same allowance
[`code-comment-style.md`](code-comment-style.md#where-one-goes) makes for a
declaration with no body to put a note in.

An options object whose `fn` is written out — `fn() { … }` or
`fn: async () => { … }` — has a body like any other callback, and the
comment goes inside it.

Three nearby shapes are not this one:

- A comment whose subject is a **run of sibling blocks** — two or three adjacent
  cases that each pin one part of what it says — has no home inside any one of
  them. That comment is telling you the run wants a `describe()`: give the run
  its own block, and the comment goes inside that block by the rule above, which
  is where a long rationale belongs anyway. Wrapping a run renames every test in
  it, so where the history is worth keeping, bridge the rename as [test
  records](test-records.md) describes. A run that should not become a block
  takes a [section marker](code-comment-style.md#section-markers) instead, and
  then the region needs closing — a second marker after the run, or the end of
  the enclosing block or file — or a reader cannot see where it ends. And where
  what you have to say names the cases one at a time, say each part in the case
  it belongs to rather than all of it in one place.
- A comment describing the **file** rather than any block in it is a file
  header. It goes at the top of the file as a doc comment, per
  [File headers](code-comment-style.md#file-headers), and not above the
  top-level `describe()`. That section is also what says a test file is not
  owed one: a header that would only name the file under test, or restate its
  contract, is left out.
- A [section marker](code-comment-style.md#section-markers) titles a region of
  the file holding several blocks. Reach is what tells the two apart, not
  length or subject matter.

  A marker covering a single block is usually a block comment in the wrong
  frame: where it says something about that block, it becomes a block comment
  in the form above, and where it says no more than the block's own
  description, it is deleted. A comment that would say no more than the
  description above it is not worth writing.

  Two things stop that. Where a file marks its regions in a series and this
  marker is one of the series, it stays. The series is what says where each
  region begins and ends, so a marker missing from the middle of one leaves
  the region above it looking wider than it is. The text can say no more
  than the block does and the marker still earns its place, because what it
  carries is the boundary. And where the marker closes the region above it,
  removing it reopens that region, so it is replaced rather than removed —
  by a marker titling what follows, or by wrapping the region above in a
  `describe()` that ends it.

### The shape to aim for

A reader should be able to tell what a comment covers from where it sits, and
each place carries one reach. First thing inside a callback, it covers that
block — the single case, or the whole group, whichever the callback opens.
Framed as a section marker, it covers the region running to the next marker at
the same level, or to the end of the enclosing block or file. Beside a hook or a
body-less callback, it covers the statement it touches. At the top of the file,
as a doc comment, it covers the file. Write each comment into the place that
matches its reach and a reader never has to guess.

Give a file one way of showing its regions and keep to it. Where a file marks
regions, a marker ends the one before it, so a series of them reads as a series
and a lone label in another form leaves the region above it looking wider than
it is.

Moving a comment into its block takes a boundary away with it. Whatever sat
above was bounded in practice by that comment being there, and with the comment
inside a callback the thing above reaches past where the comment held it. Look
up before moving one: where a region ends at the comment, it needs a closer of
its own in the same change. Deleting a comment does the same thing.

None of this is checked mechanically, on purpose: the cases that do not fit
keep arriving, and a gate would only make them expensive. It is guidance. A
file that reads better than the guidance is the better file.

## Assertions

Default to `expect()`-style assertions — `expect(...).toBe(...)`,
`.toEqual(...)`, `.toStrictEqual(...)`, `.toThrow(...)` — over the `assert*()`
family. An `expect()` failure prints actual against expected, and structurally
diffs objects and arrays; a bare `assertEquals(a, b)` failure leaves the reader
to work out what differed. That difference is load-bearing exactly where tests
are hardest to debug.

- **Equality on structured values**: always `expect(actual).toEqual(expected)`
  or `.toStrictEqual(expected)`. Never `assertEquals` or `assertStrictEquals`
  on an object, array, or other composite — the failure output is strictly
  worse.
- **Strict equality on primitives**: `expect(thing).toBe(value)`.
- **Throwing**: `expect(() => fn()).toThrow(...)`.
- **Truthiness**: plain `assert(thing)` or `assert(!thing)` is the right tool
  when "is this truthy" _is_ the whole assertion, with no further structure to
  report. Never write `assertEquals(thing, true)`, which has the weaknesses of
  both forms; `expect(thing).toBe(true)` is also fine when the test reads more
  naturally as a strict-value check.

The boundary is semantic fit rather than taste. Reaching for `assert*()` is a
prompt to ask whether `expect()` would say the same thing with better failure
detail. When the `assert*()` form genuinely captures something cleaner, keep it
and say why in a comment.

## Which comparison decides the outcome?

Before writing a test whose whole point is a fine-grained distinction, ask
which comparison actually determines whether it passes: the matcher's, or the
code under test's. If it is the matcher's, the test is measuring the test
framework, and it will stay green no matter what the code does. A vacuous green
test is worse than no test at all, because it looks like coverage and so stops
anyone from writing the real one.

The check is mechanical: confirm the test **fails** against a deliberately
wrong expectation before finalizing it. A test that cannot fail is not
evidence.

The traps below all have the same shape — an operator that is correct for some
comparison, just not the one that was meant.

### Hazards of the JavaScript matchers

**`toEqual` cannot see negative zero.** `expect(-0).toEqual(0)` passes, and so
do the array and `Set` forms. Only `toBe()`, which uses `Object.is`,
discriminates `-0` from `+0`. Any `-0` assertion written with `toEqual` is
therefore vacuous. Assert the sign of zero with `toBe`, and where the value is
nested, reach in and assert the scalar rather than comparing the container.

**`toBe` answers the `NaN` question for you.** Both matchers treat `NaN` as
equal to itself, so `NaN` escapes the collapse above — but it has the opposite
problem. `expect(NaN).toBe(NaN)` passes because `Object.is` says so, whatever
the code under test decided. A test meaning to pin "this implementation treats
all `NaN`s as equal" has to route through the implementation's comparison and
assert on _that_ result:

```ts
import { expect } from "@std/expect";
import { valueEqual } from "@commonfabric/data-model";

// Vacuous: asserts `Object.is(NaN, NaN)`, which holds regardless of
// what `valueEqual()` does.
expect(NaN).toBe(NaN);

// Real: asserts what `valueEqual()` actually decided.
expect(valueEqual(NaN, NaN)).toBe(true);
```

Neither matcher is globally safe: `toEqual` is too coarse to see `-0`, and
`toBe` is precise enough to answer the `NaN` question in the code's place.

### Hazards specific to this repository

**`hashOf()` returns a boxed value.** Its result is a `FabricHash` object, not
a string, so `===` — and `toBe`, which is `Object.is` — compares identity
rather than hash value. Two structurally identical hashes of equal values
compare unequal. Compare through an accessor instead:

```ts
import { expect } from "@std/expect";
import { hashOf, hashStringOf } from "@commonfabric/data-model";

const a = { donut: "cruller" };
const b = { donut: "cruller" };

// Wrong: compares object identity, and always fails.
expect(hashOf(a)).toBe(hashOf(b));

// Right: compares hash value.
expect(hashOf(a).bytes).toEqual(hashOf(b).bytes);
expect(hashOf(a).hashString).toBe(hashOf(b).hashString);

// Also right: `hashStringOf()` returns a plain string, which `toBe` compares
// by value. Reach for it when the hash is only ever wanted as a string.
expect(hashStringOf(a)).toBe(hashStringOf(b));
```

The same caution applies to any other boxed primitive that wears the costume of
a scalar. When a comparison between two values that ought to be equal comes
back false, **print the operands before believing it**. A `FabricHash` renders
as `FabricHash {}` rather than as a hash string, which is the tell. A
confidently reported defect that turns out not to exist costs more than a
missed gap: it consumes the reporter, the reviewer, and everyone downstream of
both.

**A cast onto a class's internals reads whatever it is told to.** Writing
`(thing as unknown as { count: number }).count` declares the member's name and
type from the test's side, and nothing checks either. Rename the member and
the read yields `undefined`, which `assert(!thing.count)` accepts, so the test
stays green while asserting nothing. Reach internals through the class's
`accessForTestingOnly` getter instead, which types each member as the class
does and turns a wrong name into a type error;
[`DEVELOPMENT.md`](DEVELOPMENT.md#making-a-private-member-reachable-from-a-test)
says how a class shapes one, and what to do about the reaches it cannot
cover. A stand-in the test hands the class declares itself as one where it is
passed in, with an `as` on the argument or a small typed helper, never with a
cast on the receiver.

**`SpaceReplica.getDocument()` reads `undefined` for three different states.**
The replica has never examined the document, the replica examined it and found
it absent, and the replica holds a record whose value materializes as
`undefined` all read the same. So
`expect(replica.getDocument(id, scope)).toBeUndefined()` states only that the
replica has no value for the document, whichever of the three put it in that
position. Where the document is one no passing run holds a value for, the case
cannot fail: a change that makes the replica examine that document leaves it
green. Say which state is meant. Either name a document a run genuinely could
hold a value for, so a value arriving where none belongs turns the case red, or
read the record alongside the value:

```ts
// Shown at module scope.
import { expect } from "@std/expect";
import type { URI } from "@commonfabric/memory/interface";
import type { SpaceReplica } from "@commonfabric/runner/storage/v2";

declare const replica: SpaceReplica;
declare const id: URI;

expect(replica.accessForTestingOnly.hasDocumentRecord(id, "space")).toBe(true);
expect(replica.getDocument(id, "space")).toBeUndefined();
```

`hasDocumentRecord()` reports whether the replica holds a record for the
document, which is the predicate the absence-reconciliation path in
`packages/runner/src/storage/v2.ts` decides by. A record appears once the
replica has examined the document and also once a local write for it is
pending, so a case turning on the first of those says in a comment that
nothing has written the document yet.

**A label-map absence assertion passes in four different states.** A test
reaches the CFC label map through a chain like
`replica.getDocument(id)?.cfc?.labelMap?.entries ?? []`. The `getDocument()`
collapse above accounts for the first of the four. Two more come from the rest
of the chain: the document carries no `cfc` metadata, and that metadata
carries no `labelMap`. Each of the three yields the empty list the `??` hands
back. The fourth is the one an absence assertion means — the label map is
there and holds no matching entry — where `entries` may well be non-empty and
it is the `find()` or `filter()` over it that comes back empty. All four look
alike by the time the matcher runs, and where no passing run stamps that
document, the case cannot fail at all.

Say which state is meant. A positive companion the same helper reads back from
that same document closes the first three: the entry is there, so the label
map, the metadata and the document are there too, and the fourth is left as
the claim. Where the run labels nothing on that document, no such companion
exists, and naming the value the document stores is what there is. It rules
out a missing document and nothing else, which leaves the two middle states
standing — and those are what such a case asserts, since a document the run
stamps nothing on is one carrying no metadata. A companion read from another
document closes none of the three. It says the run stamped something
somewhere, which is worth asserting where the case turns on a label that must
not travel, and is not a substitute for pinning the document under assertion.

```ts
// Shown at module scope.
import { expect } from "@std/expect";

type StoredEntry = { label: { confidentiality?: string[] } };

declare const entriesOf: (id: string) => StoredEntry[];
declare const storedDocument: (id: string) => { value?: unknown } | undefined;
declare const sourceId: string;
declare const copyId: string;

// The stored value pins the copy document, and the source carries the atom
// the copy must not.
expect(storedDocument(copyId)?.value).toEqual({ reading: "37.77,-122.41" });
expect(
  entriesOf(sourceId).flatMap((e) => e.label.confidentiality ?? []),
).toContain("secret");
expect(
  entriesOf(copyId).flatMap((e) => e.label.confidentiality ?? []),
).not.toContain("secret");
```

## What a claim ranges over

A test exhibits instances. To exhibit an absence it has to exhaust the set the
claim ranges over, so a quantified claim — every input does this, no input
does that — is worth exactly as much as the enumeration behind it. Exhaust the
set and the claim is settled. Leave it open and the case passes by failing to
find a counterexample, which is also what it does when one exists in a
spelling nobody tried.

So before writing such a case, say what exhausts the set:

- **An exhaustive projection over a type closes it.** A union does not close
  it alone: `const cases: K[] = ["a", "b", "c"]` keeps compiling when `K`
  gains a `"d"`, leaving a test that iterates `cases` quietly short. What
  closes it is a shape the compiler checks against the type — a
  `satisfies Record<K, …>` table, or a `never` branch that reds when a member
  goes unhandled. Classifying a union's members is also not the same as
  covering their values: `string | number | boolean` has three members and
  unboundedly many values.
- **An enumeration closes it** when the table the test iterates *is* the
  domain, rather than a list the code also consults. Where production accepts
  something the table omits — `commands.includes(name) || name === "legacy"` —
  a test over the table is complete about the table and silent about the
  claim.
- **A search closes it** when the corpus is known complete and the pattern
  cannot miss a member — a fact about the pattern, to be established rather
  than assumed.
- **Examples chosen while writing the test close nothing.**

Where the set will not close, assert the bounded claim instead. Name the input
that reaches the behavior rather than declaring that none does:

```ts
import { expect } from "@std/expect";

/** The operands on a line: a bare `--` ends the options, and everything
 * after that first one is an operand whatever it opens with. */
function operandsOf(tokens: readonly string[]): string[] {
  const end = tokens.indexOf("--");
  const head = (end === -1 ? tokens : tokens.slice(0, end))
    .filter((token) => !token.startsWith("-"));
  return end === -1 ? head : [...head, ...tokens.slice(end + 1)];
}

// Vacuous for the claim "no line reaches an operand named `--`". It tries one
// line, passes, and would pass identically if some other line reached one.
expect(operandsOf(["--"])).toEqual([]);

// Checkable, because it names the line rather than quantifying over all of
// them. This one fails the moment the terminator stops being only the first.
expect(operandsOf(["--", "--"])).toEqual(["--"]);
```

The same shape reaches prose, and the remedy there is not resignation. A
comment or a design document can state a property that no case holds it to,
and a sentence nothing holds drifts quietly away from the code it describes.
What travels with the code is the case: a comment saying a function never
returns 2 for a boolean input is contradicted by a case over both inputs the
moment one of them returns 2, and after that the sentence and the case move
together. So a claim worth writing down is usually worth the case that fails
when it stops holding. Where the sentence goes in without one, write what a
reader can check — the spelling that reaches the behavior, or the rule that
decides — rather than the absence the sentence would have to survey to be
true.