write-unit-test

# Writing Unit Tests ## Core Principles Unit tests in this project are written against the **Test Desiderata** — the 12 properties that make tests valuable, defined by Kent Beck. Not every property applies equally to every test, but treat them as the standard to reason about and optimize for. | Property | What it means | | ------------------------- | ----------------------------------------------------------------------------------- | | **Isolated** | Tests return the same result regardless of run order. No shared mutable state. | | **Composable** | Different dimensions of variability can be tested separately and results combined. | | **Deterministic** | Same inputs always produce the same result. No randomness, no wall-clock time. | | **Fast** | Tests run in milliseconds. Unit tests must never block on I/O or sleep. | | **Writable** | Writing the test should cost much less than writing the code it covers. | | **Readable** | A reader can understand what behaviour is being tested and why, without context. | | **Behavioral** | Tests are sensitive to changes in observable behaviour, not internal structure. | | **Structure-insensitive** | Refactoring the implementation should not break tests that test the same behaviour. | | **Automated** | Tests run without human intervention (`cargo test`). | | **Specific** | When a test fails, the cause is immediately obvious from the failure message. | | **Predictive** | Passing tests give genuine confidence the code is ready for production. | | **Inspiring** | Passing the full suite inspires confidence to ship. | Some properties support each other (automation makes tests faster). Some trade off against each other (more predictive tests tend to be slower). Use composability to resolve apparent conflicts. Reference: <https://testdesiderata.com/> and Kent Beck's original papers on [Test Desiderata](https://medium.com/@kentbeck_7670/test-desiderata-94150638a4b3) and [Programmer Test Principles](https://medium.com/@kentbeck_7670/programmer-test-principles-d01c064d7934). ## Coverage and Test-Gap Policy The repository prefers high maintainable automated coverage. Tracked test code is Rust. A temporary issue-local verification script is not a test: it requires an issue-specification rationale explaining why it is better than a maintained Rust automatic test and, when written in Python, why Rust is unsuitable. Promote durable behavioral checks into Rust tests when practical. Manual verification is separate real use of the finished artifact; record its actual evidence in `manual-verification-evidence.md` as defined by [`docs/testing.md`](../../../../../docs/testing.md#verification-types). When recording validation evidence in an issue specification or test-refactor plan, name the toolchain or runtime that produced every command result when it can affect behavior. For example, record `cargo +nightly fmt --all -- --check` as `nightly Rust toolchain`; do not present `cargo fmt --all -- --check` as an unqualified formatter result. Practical priority order: 1. Unit tests first (fast, deterministic, low maintenance) 2. Integration tests where unit tests are insufficient 3. End-to-end tests for cross-process/system validation When behaviour is left untested, document why explicitly in one or more of: - code comments near the boundary/constraint, - issue spec notes, - PR description. Acceptable reasons to defer or avoid direct unit tests include: - behaviour depends on out-of-process services not controlled by the test, - deterministic unit tests would be disproportionately brittle, - validation is better covered by integration/E2E tests with clear evidence. If a feature is hard to test, treat that as design feedback first and improve testability when practical. ### Coverage Attribution Is Unit-First For package-owned behavior, treat unit-only coverage as the primary measurement and aggregate/global coverage as a separate broad-progress measurement. An aggregate report can include unit, integration, example, or end-to-end binaries; it cannot prove that a source seam has adequate unit protection. Record unit-only and integration-only measurements separately when coverage informs a test-boundary decision. Do not reject a feasible focused unit test because an integration, example, or end-to-end test already executes the behavior. Decline a unit test only when it cannot protect the behavior at an appropriate boundary, or when a higher-level contract is demonstrably clearer and more maintainable; record that rationale in the issue-local evidence. ### Inventory Behaviors and Choose the Test Level Before writing or refactoring tests for a module, list what the module itself decides and what it only forwards to collaborators. Ask: **what does this module decide?** A test belongs at the lowest level that can observe everything it asserts without reading other modules. 1. Write the inventory as a temporary comment at the top of the module's `tests` block: each behaviour, its edge cases, and whether it is covered, uncovered, or deliberately untested. 2. Classify each behaviour: - **Unit** — the module decides it and the outcome is observable from the module's own API. - **Collaboration** — the outcome depends on a collaborator's semantics (for example, a registry's comparison rule or a repository's set-versus-increment behaviour). Place it one level up, typically a `tests` module in the parent `mod.rs`, and say so in its module comment. - **Collaborator-owned** — the rule lives in another module; test it there. 3. If a module's decision has no observable seam (it is only visible through a collaborator's answer), treat that as design feedback: extract the decision into a pure function and unit-test it directly. See [Pure decision function for collaborator-observed outcomes](../../../../../docs/testing/refactoring-patterns/pure-decision-function-for-collaborator-observed-outcomes.md). 4. Implement only the tests in the current issue's scope. Replace the inventory with a short note in each owning module listing the missing tests and the tracking issue (for example, the package coverage EPIC), so the gap stays visible in code rather than in chat or agent memory. 5. Split the work into a reorganisation commit (move tests, add notes) and a refactor commit (new seams and tests), so reviewers can verify that behaviour did not change in the first step. A test that passes only because of a collaborator's rule is not a unit test of the module it sits in, even when it is deterministic and fast; it fails for reasons the module cannot explain. ### Prove a Regression Test Guards the Bug A regression test is only evidence if it has been seen to fail against the bug it claims to guard. Reasoning that it "would" fail is not enough; the #2226 before/after pair was reasoned to be sufficient and a mutation showed it was not. 1. **Mutate, then restore.** With the fix in place, reintroduce the bug in the working tree (do not stage it), run the test, and confirm it fails. Restore the production file with `git checkout -- <file>`. Try the nearest plausible variants too, for example a value cached after first use instead of captured at startup. Record which tests failed for which mutation in the issue-local evidence. 2. **Stale-state bugs need two iterations of the same instance.** A test that runs one tick, one request, or one call cannot distinguish "computed once at startup" from "computed at first use" from "computed every time". When the defect is a value captured or cached across iterations, the regression test must observe the same job or object before and after the value should have changed. 3. **Read the failure as a stranger.** Make the test fail on purpose and read the output as if you had never seen the test. `left: 0 / right: 1` forces the reader to open the file. Put the causal facts in the assertion message: what was measured, and the inputs that determine the expectation (for example the gauge name, the elapsed time, and the timeout). When two assertions in one test read the same value, their messages must say which step each belongs to. ### Lifecycle Fixture Design Review When a test fixture manages a child process, asynchronous I/O, network readiness, or failure cleanup, define its lifecycle design before implementing the main scenario: 1. Identify the narrow test-facing interface and the owner of each resource. 2. Keep passive infrastructure, such as output draining, separate from domain-specific interpretation such as readiness rules. 3. State resource lifetime through normal shutdown and panic/drop cleanup; retain diagnostic output after reader tasks complete where possible. 4. Use one absolute deadline that bounds all awaited readiness work, including connection attempts, response decoding, child-exit checks, and retry delays. 5. After the first passing vertical slice, review whether the responsibilities remain coherent. Record material lessons in the issue completion review; do not create generic abstractions without a demonstrated need. ### Project-specific conventions - **Behavior-driven naming** — test names document what the code does - **AAA Pattern** — Arrange → Act → Assert (clear structure) - **Deterministic** — use `MockClock` instead of real time (see Phase 2) - **Isolated** — no shared mutable state between tests - **Fast** — unit tests run in milliseconds ### Make Arrange State-Centered Before writing or refactoring an Arrange section, ask: **what is the one difference in initial state that makes this Act behave differently?** Make that causal condition visible to the reader. Pick the Arrange tool that states that condition most directly: - **Inline values** when a literal or a single constructor call already says it. - **Test builders** when a readable call chain in the test body names the choice, e.g. `configuration().private().with_expired_key(...)`. Builders fit when each test varies one or two named options on the same object. - **Scenario fixtures** when the condition emerges from several coordinated steps across objects, resources, or registries, and no chain reads as clearly. Name the fixture for the resulting state, such as `ServerStartWithDuplicateRegistration`. The fixture owns incidental mechanics (resource allocation, configuration mutation, dependency construction, state seeding); the test keeps the production Act and observable assertions visible. Focused fixtures form a catalog of important system scenarios. These tools compose: a fixture may use builders internally, and a scenario may expose a small builder for the few variations it legitimately supports. Apply the pattern that fits the moment; none of them is a rule against the others. Whatever the tool, do not hide the Act or assertions inside it, let it accumulate unrelated optional components, or derive an expected outcome using production code under test. For the full constraints and example, see [Scenario fixtures for causal initial state](../../../../../docs/testing/refactoring-patterns/scenario-fixtures-for-causal-initial-state.md). ### Reveal Behavioral Data; Hide Collaborator Mechanics Trace every value that crosses from Arrange into the Act or Assert. Keep a value visible in the test body when it selects the behavior under test, establishes a causal initial state, or independently specifies an expected result. Its use in the Act or Assert must make that relationship readable. Hide only ordinary valid collaborator-construction mechanics that do not vary the selected behavior, such as locks, reference-counted handles, default dependency construction, or required repository setup. For example, a banning-handler gauge test keeps an `unrelated_client_ip` visible when it establishes the pre-existing tracked-IP state, keeps the event's `cookie_error_client_ip` visible where it enters the event context, and keeps `expected_distinct_client_ip_total` visible before the Act and in the Assert. A state-named test context may hide its `Arc<RwLock<BanService>>` and `Repository` setup. Do not hide the relevant IPs or expected total inside that context. During review, ask: **“Can the reader follow every value that makes the Act behave differently or sets the expected result from its Arrange origin to its Act/Assert use?”** If not, expose that value or rename/refocus the scenario. Also ask: **“Does this value merely make an ordinary collaborator valid?”** If yes, it belongs in focused setup rather than the test narrative. ### Name Coherent Actions at One Abstraction Level Use a helper when it gives a coherent sequence of setup or transport actions a meaningful name and keeps the caller at one readable abstraction level. A helper does **not** require multiple callers: `start_ephemeral_udp_tracker()` can be justified by naming one complete ordinary setup action even when one contract test currently uses it. Judge a helper by semantic value, not reuse count. Keep it when its name expresses a capability or state relevant to the test and it hides only incidental mechanics. Reject it when it merely moves code away behind a vague name such as `setup()`, becomes a parameter bag, hides the causal state, production Act, or expected result, or mixes unrelated responsibilities. See [Named helpers for abstraction-level alignment](../../../../../docs/testing/refactoring-patterns/named-helpers-for-abstraction-level-alignment.md) for selection criteria and examples. ### Anti-Pattern: Duplicated Fixture-Derived Expectations Do not extract a second helper that manually reconstructs a representation already derived from a fixture when that representation is not independently under test. For example, a test that passes a `ConnectionContext` to production code should not separately hard-code every metric label expected from that context merely to add one causal label such as `request_kind=connect`. The fixture and expectation become coupled by hidden duplication: an unrelated fixture change makes the test fail with stale expected details. Instead, derive fixture-owned details from the exact fixture value used by the Act, and specify only the test's causal input or independently asserted result in the test body. In the metric example, create `LabelSet::from(connection_context.clone())` and visibly add `request_kind=connect`. Add a separate focused test when conversion of the fixture into its derived representation is itself the behavior under test. During prose-first review, ask: **“If this fixture changes, should this test fail?”** If no, derive the incidental expectation from the fixture. If yes, keep the relevant fixture value and its assertion visibly connected in the test prose; use a scenario or builder if several coordinated values establish that causal state. ### Review Test-Code Smells Before Finishing Before requesting maintainer review for a test-producing increment, inspect the final test against these smells. A smell is a prompt to improve the design, not an automatic rule: keep the clearest test when an alternative would weaken its behavioral contract or diagnostic value. | Smell | Review question | Preferred response | | ------------------------------- | --------------------------------------------------------------------------------------------------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------- | | Complex Arrange | Can a reader name the causal initial state without reconstructing setup plumbing? | Use inline values, a readable builder, or a narrowly named scenario fixture. Keep causal input visible and move only coordinated incidental mechanics. | | Hidden behavioral data coupling | Can the reader trace every causal input and expected value from Arrange into its Act or Assert use? | Keep behavior-selecting inputs, causal pre-existing state, and independent expected values visible; hide only ordinary collaborator construction. | | Multiple assertions | Do the assertions specify one complete observable result or unrelated behaviors? | Use one higher-level semantic assertion when it preserves the full result and diagnostic clarity; otherwise split the test so each has one reason to fail. | | Hidden fixture coupling | Would an unrelated fixture change fail this test? | Derive incidental expectations from the exact fixture used by the Act; keep independently specified causal values visible. | | Hidden production Act | Can the reader identify the production behavior under test directly? | Keep the production invocation visible; do not move it into setup or assertion helpers. | | Production-derived expectation | Is the expected result calculated by code that the test is meant to verify? | Construct the expected result independently; move only mechanical comparisons into a semantic assertion helper. | Record material refactoring decisions from this review in the file-local plan or task evidence. ### Verify Intent with Prose-First AAA Before considering any new or materially refactored test ready for maintainer review, make its intent explicit and verify that the final code communicates it. This is mandatory for every test-producing increment: 1. Write temporary normal-prose **Arrange**, **Act**, and **Assert** paragraphs above the test. State the causal initial state, the production action, and independently specified observable result; do not describe implementation mechanics without explaining their behavioral purpose. 2. Repeat each paragraph above the corresponding `// Arrange`, `// Act`, or `// Assert` code section. 3. Compare the code with each paragraph. Refactor names, setup, builders, scenario fixtures, the visible Act, or assertions until the code itself expresses the paragraph. 4. Remove prose that is redundant once the code communicates the intent. Retain only essential context that cannot be expressed clearly in code without disproportionate complexity or a misleading abstraction. 5. Record the completed prose-first comparison in the task evidence or file-local test plan before maintainer review and commit. The temporary prose is the test's specification, not permanent commentary. A parameter bag, an opaque fixture, a hidden Act, or an assertion derived through production code is evidence that the code has not yet expressed its specification. See [Prose-first Arrange-Act-Assert verification](../../../../../docs/testing/refactoring-patterns/prose-first-arrange-act-assert-verification.md) for a repository example. ## Phase 1: Basic Unit Test ### Naming Convention **Format**: `it_should_{expected_behavior}_when_{condition}` - Always use the `it_should_` prefix - Never use the `test_` prefix - Use `when_` or `given_` for conditions - Be specific and descriptive ```rust #[cfg(test)] mod tests { use super::*; #[test] fn it_should_return_error_when_info_hash_is_invalid() { // Arrange let invalid_hash = "not-a-valid-hash"; // Act let result = InfoHash::from_str(invalid_hash); // Assert assert!(result.is_err()); } #[test] fn it_should_parse_valid_info_hash() { // Arrange let valid_hex = "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"; // Act let result = InfoHash::from_str(valid_hex); // Assert assert!(result.is_ok()); } } ``` ### Running Tests ```bash # Run all tests in a package cargo test -p bittorrent-tracker-core # Run specific test by name cargo test it_should_return_error_when_info_hash_is_invalid # Run tests in a module cargo test info_hash::tests # Run with output cargo test -- --nocapture ``` ## Phase 2: Deterministic Time with `clock::Stopped` The `clock` workspace package provides `clock::Stopped` for deterministic time testing. Never call `std::time::SystemTime::now()` or `chrono::Utc::now()` directly in production code that needs testing. Instead, use the type-level clock abstraction. ### Use the Type-Level Clock Alias Copy the following boilerplate into each crate that needs a clock. The `CurrentClock` alias automatically selects `Working` in production and `Stopped` in tests: ```rust