From fa2c82b00ba08e81ca8d9d9a692fb1e323ac605b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?R=C5=ABdolfs=20O=C5=A1i=C5=86=C5=A1?= <rudolfs@osins.org>
Date: Wed, 30 Sep 2026 11:31:07 +0200
Subject: [PATCH] Document the testing strategy and e2e conventions

An ADR records how we decide what to test and where, and a checklist lets
people and agents check a change against it.
---
 .gitignore                        |  2 +
 CLAUDE.md                         | 36 +++++++++++++++++-
 docs/adr/0001-testing-strategy.md | 63 +++++++++++++++++++++++++++++++
 docs/testing-checklist.md         | 49 ++++++++++++++++++++++++
 4 files changed, 148 insertions(+), 2 deletions(-)
 create mode 100644 docs/adr/0001-testing-strategy.md
 create mode 100644 docs/testing-checklist.md

diff --git a/.gitignore b/.gitignore
index c3377178e..482eaa03b 100644
--- a/.gitignore
+++ b/.gitignore
@@ -31,3 +31,5 @@ tests/artifacts/**/*
 
 # direnv cache
 /.direnv/
+
+.claude
diff --git a/CLAUDE.md b/CLAUDE.md
index 9843091be..5ca418174 100644
--- a/CLAUDE.md
+++ b/CLAUDE.md
@@ -44,6 +44,11 @@ npm run format          # auto-fix prettier issues
 npm run test:unit
 ```
 
+DOMPurify does not sanitize under happy-dom (the unit-test environment):
+`<script>` and `onerror=` pass through, and `setConfig` allowlists are
+ignored. Don't assert sanitization in `tests/unit/`; verify it in the real
+app or in an E2E test.
+
 ### E2E tests
 
 ```sh
@@ -51,13 +56,31 @@ npm run test:e2e
 npm run test:e2e -- tests/e2e/<file>.spec.ts
 ```
 
-`SKIP_SETUP=true` skips fixture creation for faster iteration.
+Peer keys have no passphrase, so the app authenticates without an
+ssh-agent. Tests that need a running node use the `peer` fixture; tests that
+need an ssh-agent use the `sshAuthSock` fixture and create a peer with a
+`passphrase`. When a fixture depends on one peer seeing another's changes,
+wait for the node event with `waitForEvent` instead of sleeping.
+Reload pages with `reload(page)` from `@tests/support/fixtures.js`, not
+`page.reload()`: the app polls the backend, and the harness would otherwise
+fail the test on the request the reload cancels.
+
+Tests run against a production build in `tests/tmp/build`, served by
+`vite preview`; the build takes a few seconds at startup. Set
+`E2E_DEV_SERVER=1` to run against the Vite dev server instead, e.g. for hot
+reload while writing tests.
+
+Each run writes `tests/artifacts/results.json` (Playwright's JSON report)
+and per-test logs in `tests/artifacts/<test>/`. Read those to see what
+failed in a run someone else started.
+
+`SKIP_FIXTURE_CREATION=true` skips fixture creation for faster iteration.
 Only use it when you are solely editing `.spec.ts` files and fixtures
 already exist from a previous full run. Any change to app code, the Rust
 backend, or test fixtures requires a full run.
 
 ```sh
-SKIP_SETUP=true npm run test:e2e -- --project webkit
+SKIP_FIXTURE_CREATION=true npm run test:e2e
 ```
 
 ### Rust backend (`crates/`)
@@ -88,6 +111,12 @@ Adding or changing a Tauri command touches these layers in order:
    `cargo test -p test-http-api --test parity` fails until you do
 5. `src/` — call via `invoke<T>("command_name", args)`, import types from `@bindings`
 
+### Tests for a feature
+
+When adding tests for a feature, go through `docs/testing-checklist.md` and
+report which items are covered and which don't apply. The testing strategy is
+in `docs/adr/0001-testing-strategy.md`.
+
 ### Pre-push checklist
 
 These mirror what CI runs on every PR. Run them all before shipping a feature.
@@ -270,6 +299,9 @@ Read these when you need domain context for UI work:
   `#[ts(export)]`, and `#[ts(export_to = "<dir>/")]`
 - Optional fields: `#[serde(skip_serializing_if = "Option::is_none")]`
 - Error type: `radicle_types::error::Error`
+- `rust-toolchain.toml` tracks heartwood's. When bumping the `radicle` crate,
+  check heartwood's toolchain too: an older rustc fails inside the dependency
+  with a confusing error (e.g. E0382 in `cob/identity.rs` for radicle 0.25)
 
 ## Commit messages
 
diff --git a/docs/adr/0001-testing-strategy.md b/docs/adr/0001-testing-strategy.md
new file mode 100644
index 000000000..7fdca17c0
--- /dev/null
+++ b/docs/adr/0001-testing-strategy.md
@@ -0,0 +1,63 @@
+# 1. Testing strategy
+
+Date: 2026-09-30
+
+## Status
+
+Accepted
+
+## Context
+
+The app has many features and few tests. We need a way to decide what to
+test, at which layer, and in what order, without trying to reach full
+coverage.
+
+## Decision
+
+Prioritise by risk: how bad a failure is (corrupt COB writes are worse than
+wrong display), how likely it is (complex logic, churn, crossing the Rust/TS
+boundary) and how hard it is to notice manually (multi-peer, caching,
+timing).
+
+Test each behaviour at the cheapest layer that can catch its failure:
+
+| Layer    | Tool                          | Scope                                                                    |
+| -------- | ----------------------------- | ------------------------------------------------------------------------ |
+| TS unit  | Vitest, `tests/unit/`         | Pure logic: parsing, routing, formatting, filters, diff and review logic |
+| Rust     | `cargo test`                  | Trait default implementations, type conversions, error codes            |
+| E2E      | Playwright + `test-http-api`  | User journeys and every write path against a real Radicle profile       |
+| Manual   | Tauri build                   | Native-only features: dialogs, clipboard images, drag and drop, badges  |
+
+Rules:
+
+- Unit tests come first because they are the cheapest.
+- Every bug fix adds a regression test, at the cheapest layer that
+  reproduces the bug, and the test fails without the fix.
+- Non-trivial logic inside a component (branching, ordering, grouping,
+  deduplication, parsing) is extracted into a module in `src/lib/` and
+  tested there. A one-line helper with a single caller stays in the
+  component. Comments that explain why the code is the way it is move with
+  it.
+- Tests assert behaviour: an outcome, a property or a relationship between
+  inputs. They don't restate UI copy, CSS classes, selector strings or
+  constant tables, since such tests only ever change in lockstep with the
+  code.
+- A test earns its place by failing when the logic it covers breaks. New
+  tests are checked by mutating that logic (flip a comparison, drop a
+  filter, shift a boundary), and a test that never fails is removed.
+- Extracted logic is tested in isolation, so the component flow that wires
+  it into a write path still needs its E2E test.
+- Every mutating command gets at least one E2E happy path and one
+  permission or error case. Reads are covered by unit tests plus one E2E
+  test showing the data renders.
+- Combinations such as filters and sort orders are covered in unit tests,
+  not in E2E.
+- State persisted in `localStorage` gets one reload test in E2E.
+- Sanitisation is asserted in E2E only, since DOMPurify is a no-op under
+  happy-dom.
+- `test-http-api` mirrors every Tauri command. Logic lives in
+  `radicle-types` so that both drivers stay thin wrappers, and a parity test
+  fails when a command, route or frontend `invoke` has no counterpart.
+
+New features follow [the testing checklist](../testing-checklist.md).
+
diff --git a/docs/testing-checklist.md b/docs/testing-checklist.md
new file mode 100644
index 000000000..2183b59b8
--- /dev/null
+++ b/docs/testing-checklist.md
@@ -0,0 +1,49 @@
+# Testing checklist
+
+Go through this list for every feature or bug fix. Tick an item when it's
+covered, or say in the commit or patch description why it doesn't apply.
+The strategy behind it is in [ADR 1](adr/0001-testing-strategy.md).
+
+## Bug fixes
+
+- [ ] The fix comes with a regression test that fails without it, written
+      at the cheapest layer that reproduces the bug.
+
+## Unit (`tests/unit/`)
+
+- [ ] New or changed pure logic in `src/lib/` has unit tests.
+- [ ] Non-trivial logic that was added inside a component (filtering,
+      grouping, sorting, parsing) has been moved into `src/lib/` and tested.
+      One-line helpers with a single caller stay where they are.
+- [ ] Tests assert behaviour, not UI copy, CSS, selector strings or copies
+      of constant tables.
+- [ ] Each new test was seen to fail when the logic it covers was broken.
+- [ ] Edge cases are covered: empty input, a single item, invalid input,
+      boundaries.
+- [ ] New routes or URL parameters are covered by the router round-trip
+      tests.
+- [ ] New `localStorage` schemas have tests for their defaults and for
+      invalid stored values.
+
+## Rust (`crates/`)
+
+- [ ] New logic in trait default implementations or type conversions has a
+      `#[test]`.
+- [ ] New error variants are tested, as the frontend matches on error codes.
+
+## E2E (`tests/e2e/`)
+
+- [ ] A new Tauri command has a matching route in `crates/test-http-api`
+      (enforced by `cargo test -p test-http-api --test parity`).
+- [ ] A new write path, meaning any command that changes a COB, a repo or
+      the node's state, has a happy-path E2E test, even when its logic is
+      unit-tested in `src/lib/`.
+- [ ] Actions limited to authors or delegates have a test for the user who
+      isn't allowed.
+- [ ] New persisted UI state survives a reload.
+- [ ] New keyboard shortcuts are exercised at least once.
+
+## Manual
+
+- [ ] Native-only behaviour (dialogs, clipboard, drag and drop, dock badge,
+      ssh-agent) has been checked in a real `npm run tauri dev` build.