From ca34734c305c04439ecd9e8a0a0a32fe96658489 Mon Sep 17 00:00:00 2001 From: "@permadeath.com" Date: Thu, 13 Aug 2026 15:58:05 -0400 Subject: [PATCH] docs(testing)!: cut TESTING.md to four paragraphs and move it into docs/ MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 535 lines, and it had already rotted: it named `repo.rs`, `pr.rs`, `browse.rs`, `key.rs`, `about.rs` and `review.rs`, none of which have existed since the module-layout refactor. Nothing went red, because it was the one long document outside `docs/` — so rustdoc never rendered it, `-D warnings` never checked a link in it, and `doc-lint.sh` never saw it. Most of the length was records rather than rules: 132 lines of what was verified by hand when each feature landed, 93 lines of per-fixture provenance. That kind of writing is stale the week after it lands and nothing checks it. What is left is four paragraphs of advice that does not go out of date — what earns a test, the two constraints that make parts of this crate untestable, fixtures for reads and mocks for writes, and an instruction to keep the page short. Moving it into `docs/` is the fix for the mechanism, not just the symptom: it renders with everything else now, and the twenty `see TESTING.md` references in `src/` became `[`crate::docs::testing`]` intra-doc links, which the doc build checks. Facts about one test moved into that test's own doc comment. Co-Authored-By: Claude Opus 5 (1M context) --- CONTRIBUTING.md | 11 +- Cargo.toml | 2 +- TESTING.md | 582 -------------------------------- TODO.md | 2 +- docs/index.md | 10 +- docs/testing.md | 56 +++ src/clients/atproto/pds.rs | 4 +- src/clients/endpoints.rs | 2 +- src/clients/git/ssh.rs | 6 +- src/clients/http.rs | 2 +- src/clients/tangled/resolve.rs | 2 +- src/cmd/about.rs | 2 +- src/cmd/completion.rs | 2 +- src/cmd/pr/read.rs | 4 +- src/config/account/selection.rs | 4 +- src/config/dir.rs | 2 +- src/config/lock.rs | 2 +- src/docs.rs | 4 + src/term/hyperlink.rs | 2 +- src/term/say.rs | 2 +- src/testutil.rs | 2 +- tests/exit_status.rs | 10 +- tests/pr_flows.rs | 4 +- tests/support/account.rs | 2 +- tests/support/mod.rs | 2 +- 25 files changed, 99 insertions(+), 624 deletions(-) delete mode 100644 TESTING.md create mode 100644 docs/testing.md diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 6b034b4..3cb18f5 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -57,12 +57,11 @@ either is fine for this, which is why neither is in the tool list above. ## Tests -`cargo test` — no network, no session, under a second. Most of what this tool -does is on the far side of a socket and is verified by running it instead. -[TESTING.md](TESTING.md) says what is worth testing here, what is deliberately -left to manual checks and how to run those, and the two constraints that trip -people up: `HOME` cannot be redirected in a test, and the working directory is -process-wide. +`cargo test` — no network, no session, about eight seconds. +[docs/testing.md](docs/testing.md) says what earns a test here, when to reach +for the mock services in `tests/support/` rather than a fixture, and the two +constraints that trip people up: `HOME` cannot be redirected inside the test +process, and the working directory is process-wide. ## Docs diff --git a/Cargo.toml b/Cargo.toml index 6ae3e00..05228d7 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -75,7 +75,7 @@ userinput-lexicon = { path = "vendor/userinput-lexicon" } # The mock services in tests/support/. All three are already in the tree via # reqwest, which is built on hyper 1 — what these lines add is hyper's # *server* half and the tokio IO adapter for it, not a new dependency tree. -# See TESTING.md on why a mock is used for the write path and not for the +# See docs/testing.md on why a mock is used for the write path and not for the # parsers. http-body-util = "0.1.4" hyper = { version = "1.11", features = ["http1", "server"] } diff --git a/TESTING.md b/TESTING.md deleted file mode 100644 index 106e66d..0000000 --- a/TESTING.md +++ /dev/null @@ -1,582 +0,0 @@ -# Testing - -`atgc` is a single-user CLI that is mostly a thin shell over network calls to -a PDS, a knot, and Bobbin. That shape decides what testing is worth doing -here, and this file is the argument for a small suite rather than a large one. - -Run them with `cargo test`. They need no network, no session, and no -particular repo. The unit tests take about a second; the integration -environments in `tests/support/` add about six, most of it a real command -waiting on a retry it would wait on in production. - -## What earns a test - -Three things, and not much else. - -**Parsers of other people's data.** Every `sh.tangled.*` record, DID -document, Bobbin response and knot redirect arrives as JSON or a URL written -by software this project does not control, in a lexicon that is still moving. -The code is deliberately permissive about it — `unwrap_or`, `as_str()?`, -fallbacks for pre-`rounds` records — and permissive parsing is exactly the -kind that fails silently. A test that feeds it a real record and checks what -comes out is worth having; so is one that feeds it a malformed record and -checks it declines rather than guesses. - -**Things that are fiddly out of proportion to their size.** The base64url -decoder in `logging/debug.rs`, the percent-decoder in `auth.rs`, the character-safe -truncation of PR titles, the frame arithmetic in `about.rs`. These are short -enough to look obviously right and are not. They also have no natural -manual check: you cannot see that a JWT dump decoded correctly by looking at -it, only that it decoded into *something*. - -**Anything a commit message had to justify.** If a change needed a paragraph -explaining why, the behaviour it describes is worth pinning, because the -paragraph will not survive the next refactor but the test will. The -empty-list-versus-stale-index distinction is the example: three lines of code -carrying a real observation about Bobbin's indexing lag. - -## What does not earn a test - -Glue. `main.rs` is a clap enum that forwards to one function per subcommand; -a test of it would assert that the code says what it says. The same goes for -the `println!` layout of every command, `browse.rs` in its entirety, and the -`async fn` bodies that build a URL, GET it, check the status and hand the -body to a parser — once the parser is tested, what is left is four lines of -plumbing whose failure modes are all network failures. - -Coverage of that kind is worse than nothing. It costs the same to maintain as -a real test, it goes red for unrelated reasons, and it creates the impression -that a green suite means the tool works. It does not: it means the pure parts -of the tool are consistent with what they were the day someone wrote them -down. Everything that makes `atgc` useful is on the other side of a socket. - -Mock HTTP servers are still not used **to test a parser**, and the argument -has not changed. A mock encodes what we believe Bobbin returns, so a test of -a reader against it passes precisely when our belief is self-consistent — -which is the thing already known. When Bobbin changes, the mock does not, and -the suite stays green through the outage. The fixtures below are the -compromise for that direction: real bytes off the real services, captured -once, with the capture command written down so they can be refreshed and the -diff read. - -A mock *is* used for the other direction, which that paragraph did not -distinguish and should have. See [Sequences against mock -services](#sequences-against-mock-services). - -## Tests must be hermetic - -Non-negotiable, and the constraint that shapes everything else. A test may -not touch the network, use OAuth, require a logged-in session, read or write -the real `~/.config/atgc/sessions.json`, alter the user's git config, or -assume any particular Tangled repo exists. A test that fails on a plane, or -in CI without credentials, is not a test — it is a monitoring check in the -wrong place, and it teaches people to ignore red. - -Two properties of this crate make that harder than usual, and both are worth -knowing before writing a test that cannot work: - -**`HOME` cannot be redirected.** The usual trick — point `HOME` at a temp -directory and let the code read its own store — needs `std::env::set_var`, -which is `unsafe` in edition 2024, and `Cargo.toml` sets `unsafe_code = -"forbid"` for the whole crate, tests included. There is no way around it -short of dropping the lint. So code that reads `HOME` directly cannot be -unit-tested at all: the session-store readers in `pr.rs` and `repo.rs`, the -`~/.ssh` scan in `clients/git/ssh.rs`, and `config_dir()` are all in this category. The -answer is to test the logic those functions apply — which is separable, and -in `clients/git/ssh.rs` has been separated: every function that decides anything about a -key takes the directory to look in, and only the one-line wrapper that says -"the directory is `$HOME/.ssh`" is untested. That is what lets the key tests -use real key material (see below) without going near the user's own keys. The same rule rules out testing `about.rs`'s `COLUMNS`/`LINES` -handling, which is why `frame()` has no test and `env_dimension` has only -its unset case. - -That is a limit on *unit* tests only, and the distinction is the doorway the -integration environments go through: `Command::env` is not `unsafe`, so a -test that spawns the built binary may give it any `HOME` it likes. See -[Sequences against mock services](#sequences-against-mock-services). - -**The working directory is process-wide.** `clients/git/run.rs` shells out to git in -whatever directory the process is in, so testing it means moving the process, -and cargo runs tests on threads that share that. `testutil::TempRepo` is the -only thing in the tree that moves the process, and it takes a module-level -mutex for exactly this reason; keep it the only one. It builds a throwaway -repo under the system temp directory, passes `-c user.name` and friends on -the command line so the machine's own git identity and signing settings -cannot decide whether a commit succeeds, and deletes the repo on drop. No -test writes outside its temp directory. `clients/git/run.rs`, `config/account/selection.rs` and `repo.rs` -all use it — `config/account/selection.rs` to check that a checkout's `[user] email = did:…` -is read as the account it belongs to and an ordinary email is not, `repo.rs` -to check the other end of the same convention, that writing that section -never overwrites one the checkout already had. - -**Some tests generate a real SSH keypair.** `testutil::TempKeys` shells out -to `ssh-keygen` for a throwaway ed25519 pair in a temp directory of its own, -and deletes it on drop. It is a hard dependency of the suite the way `git` -already is — `ssh-keygen` ships with the OpenSSH client, which any machine -that can push to Tangled has. - -Real key material rather than a constant pasted in, because what those tests -are about is agreement between three things this project does not control: -what `ssh-keygen` writes into a `.pub` file, what a `sh.tangled.publicKey` -record holds, and what `ssh-keygen -lf` prints as a fingerprint. A hardcoded -key would pin our own reading of all three and prove none of them — the -fingerprint test in particular asserts against `ssh-keygen`'s own output, so -it fails if our base64 or our digest is wrong in any way a fixture would have -frozen in place. It also lets `key.rs` prove the one thing there that -cannot be walked back: that pointing `key add` at a private key publishes -the public half beside it, and refuses outright when there is none. - -Network access is allowed in exactly one place: a test marked `#[ignore]`, -which never runs by default and must say in its message why it is ignored. -There are two. - -The one in `clients/tangled/resolve.rs` resolves the real `permadeath.com/atgc` through -tangled.org and asserts the repo's own DID comes back rather than its -owner's, which no fixture can prove: the whole claim is about how somebody -else's server routes a URL. Its offline half — which of the two DIDs a given -path shape holds — is a plain unit test that does run by default. - -The one in `clients/http.rs` opens a socket to an unroutable TEST-NET-3 address and -asserts the connect gives up on atgc's schedule rather than the kernel's SYN -retry schedule, which is the whole point of that module. It has no offline -half to split out: the claim is about what the network stack does when -nobody answers, and the only way to observe that is to wait for it. It is -ignored as much for its five seconds as for its socket — a suite that -finishes in one second stays worth running. - -That test used to be ignored for the opposite reason, as a failing assertion -pinning a known bug. If an `#[ignore]` is ever used that way again, its -message has to say so, because "ignored" reads as "not important" and a -suite that is green with a known-broken function in it is worse than one -that is red. - -## Sequences against mock services - -The suite above tests functions. `tests/stack_flows.rs` and -`tests/pr_flows.rs` test *sequences* — `stack create` then an amend then -`stack resubmit`, `pr create` then `pr merge` then a `pr resubmit` that has -to be refused — driven through the real binary against mock services on -loopback ports. `tests/support/` is the environment; read its module -comment before writing one. - -### Why this is not the thing the section above rules out - -The argument against mocks is about *responses*: a fixture asserting what -Bobbin returns is a fixture asserting what we already believe. That argument -is sound and these tests do not touch it — the mocks answer with the shapes -atgc already believes in, and nothing here is evidence about a real service. - -What they assert is the **request** side: what atgc sends, in what order, and -as whom. That is entirely atgc's own behaviour, no fixture can observe it, -and it is where a whole class of bug lives that neither a unit test nor a -fixture can reach: - -- **A round appended to the wrong record.** Three pulls with four rounds - between them is the same shape whether the round landed on the member whose - change-id owns it or on its neighbour. Only the record keys tell them apart, - and only after a `create` has written them. -- **A chain that half-links.** Every `dependentOn` in a stack must point at a - record the same `applyWrites` contains. Written one at a time it still ends - linear, and the appview still rejects it at ingest. -- **The wrong account's credentials.** Two logged-in accounts can both write a - well-formed pull, and the record does not say which one's token paid for it. - The `Authorization` header does, so each account in a scenario gets a - distinct access token and the journal records who sent what. -- **A read-modify-write that drops what it did not mean to touch.** `pr - resubmit` and `pr edit` read a record, change one field and put the whole - thing back. A pull that comes back with one round where it had two is not a - crash; it is a perfectly healthy-looking pull. - -### How it is hermetic - -Two properties make it work, and both are worth knowing: - -**A child process may have its own `HOME`.** The rule above — that `HOME` -cannot be redirected because `std::env::set_var` is `unsafe` and this crate -forbids unsafe code — is about the *running* process. `Command::env` is not -unsafe, so a test that spawns `CARGO_BIN_EXE_atgc` can point it at a scratch -config directory. That is the whole reason these drive the built binary -rather than calling functions, and it is what puts the session store, the -account registry and account selection inside the tested surface for the -first time. `tests/exit_status.rs` found this doorway first. - -**A session can be restored offline.** jacquard returns a stored session -untouched when its access token is not near expiry, contacting nothing. So -`tests/support/account.rs` writes a session with an expiry a year out — -through jacquard's own `FileAuthStore` and `ClientSessionData`, never as -hand-rolled JSON — and every request it then signs goes to the `host_url` the -fixture chose. The DPoP key is real and the proofs are really signed; nothing -here depends on a step being skipped that production would not skip. - -The five hostnames atgc has compiled in are redirected at loopback ports -through `clients/endpoints.rs` (`ATGC_PLC`, `ATGC_BOBBIN`, `ATGC_APPVIEW`, -`ATGC_KNOT`, `ATGC_BSKY_APPVIEW`). Nothing else is needed, because every -other address atgc uses it reads at runtime out of a record, a session or a -redirect — so a test that wants to move one moves the record. **An override -left unset is a request to the production host**, which is why `Scenario` -sets all five rather than only the ones a given command is known to use. - -The git remote points at the mock knot with the repo's DID as the whole path, -which `clients/tangled/resolve.rs` answers from without a request. Commands -that do reach for the remote — `pr resubmit` asks `git ls-remote` whether its -target branch still exists — get a loopback 404, which reads as "could not -ask": the same answer an unreachable host gives, arrived at without a packet -leaving the machine. - -### What the mocks do and do not check - -They enforce what a real PDS enforces and atgc depends on: the `swapRecord` -and `swapCommit` preconditions, that an update names a record that exists, -that a write carries credentials, and that `applyWrites` is all-or-nothing. -They do not verify DPoP proofs or validate records against their lexicon — -the first is jacquard's to get right, and the second would make the mock an -authority on a lexicon it does not have. - -They are also allowed to be *strict about shapes they do not know*: a method -nothing implements answers `the mock PDS has no `, which is a test -failure that tells you a command started asking for something new. - -### What this caught - -Worth recording, because it is the argument for the six seconds: - -- `stack create --add-change-ids` on a branch behind `origin/main` produced a - stack that would have **reverted** whatever `main` had gained. The rewrite - is `git commit-tree`, which reuses each commit's tree and only changes its - parent, so reparenting onto a base the tree never saw turns every patch - into a deletion of the difference. Reported by an agent that hit it for - real; the test asserts on the patches rather than on the refusal, so it - fails on the thing that matters if the guard is ever removed. -- A rebase gives every commit above the rewritten one a new sha, and - `git format-patch` puts that sha in a patch's first line, so a stack member - *above* an amend reads as changed and gains a round even though its diff is - identical. Defensible — its patch does now apply to a different base — but - not obvious. It is pinned in - `amending_one_commit_appends_a_round_to_the_record_carrying_its_change_id`. - -## Fixtures - -`tests/fixtures/` holds real records, fetched from public read-only -endpoints that need no auth. Real shapes are the point — the first thing the -old-record fixture showed was a `createdAt` of `""`, which no invented -fixture would have had. Refresh or extend them with, for example: - -``` -curl 'https://amanita.us-east.host.bsky.network/xrpc/com.atproto.repo.listRecords?repo=did:plc:nlzmjyfv6loqtxyzvdcznwgf&collection=sh.tangled.repo.pull&limit=5' -curl 'https://api.tangled.org/xrpc/sh.tangled.repo.listPullsBy?subject=did:plc:nlzmjyfv6loqtxyzvdcznwgf&limit=4' -curl 'https://plc.directory/did:plc:nlzmjyfv6loqtxyzvdcznwgf' -``` - -The knot fixtures — `knot_branches.json`, `knot_tags.json`, -`knot_tags_empty.json`, `knot_default_branch.json`, `knot_languages.json` — -are `repo view`'s half, captured anonymously off `knot1.tangled.sh`. Each -pins something the lexicon does not say and a reasonable reader would get -wrong: a repo with no tags answers `{}` rather than an empty array; -`is_default` is absent on non-default branches rather than false; -`getDefaultBranch` returns a whole commit object of which only `name` is -filled in, the rest being an empty hash and a Unix-epoch timestamp; and one -tags response holds annotated and lightweight tags side by side, only the -first of which has a tagger. - -`knot_tags.json` is the one fixture kept as the compact bytes came off the -wire rather than pretty-printed. Its commit hashes are JSON arrays of twenty -integers, and indenting those puts each integer on its own line, which more -than doubles the file to no one's benefit. - -`repo_record.json` and `repo_record_full.json` are the two ends of the -`sh.tangled.repo` range: this project's own record, which carries the bare -minimum, and tangled.org's, which carries nearly everything the lexicon -allows. The pair is what showed that `name` is absent from real records — a -repo is named by its record key — and that `spindle`, `website` and `labels` -were live in records the `Repo` struct was silently dropping. - -`pull_old_record.json` is a pre-`rounds` pull off tangled.org's own PDS — -the only source of that shape still around, and the only evidence that the -tolerance the code claims for old records is aimed at something real. - -`appview_pull_page_excerpt.html` is the one fixture that is not whole bytes -off the wire, and it says so in its own header. The page it comes from is -2.5 MB, nearly all of it a rendered diff, so what is kept is every region -within 160 characters of an `at://`, joined by a marker — which is every -place a record URI appears on it, and the only part `review.rs` reads. -Nothing inside a region is edited. - -It is deliberately the page for pull request 36, the one that added `pr -diff`, rather than a simpler page: because that pull's diff quotes three -*other* pulls' at-URIs out of fixtures and prose, it is the page that broke -the first version of the scrape and forced it to read the record-identity -widget rather than trusting the page's text. A fixture that only holds the -easy case would have let that regress silently. - -`pds_pulls_page.json` and `pds_pull_statuses_page.json` are the pair the -PDS-sourced listing is tested against, captured with: - -``` -curl 'https://amanita.us-east.host.bsky.network/xrpc/com.atproto.repo.listRecords?repo=did:plc:nlzmjyfv6loqtxyzvdcznwgf&collection=sh.tangled.repo.pull&limit=100' -curl 'https://amanita.us-east.host.bsky.network/xrpc/com.atproto.repo.listRecords?repo=did:plc:nlzmjyfv6loqtxyzvdcznwgf&collection=sh.tangled.repo.pull.status&limit=100' -``` - -Eleven of the 52 pull records were kept, verbatim, chosen so that every trap -in the live collection survives the trim. Six aim at one repo and five aim at -five *others* — a pull record lives in its author's PDS whatever it targets, -so this collection mixes every repo the account has ever sent a patch to, and -a listing that forgets to filter by `target.repoDid` is wrong in a way no -invented fixture would have shown. Some carry a `source` and some do not. -Two of the six have no status record at all, which is what makes an unknown -state testable; the statuses that do exist cover both `merged` and `closed`. - -The pair is also why the state logic is tested as a *join* rather than as a -lookup. State is not a field on a pull record — it is a separate -`sh.tangled.repo.pull.status` record naming the pull by at-URI — and nothing -about that is visible from a single fixture. - -`search_hits.json` is a real page off `sh.tangled.search.query`, captured -2026-08-13: - -``` -curl -sS --get --data-urlencode 'q=nice catch' --data-urlencode limit=12 \ - https://api.tangled.org/xrpc/sh.tangled.search.query | python3 -m json.tool -``` - -Chosen for its spread rather than its size. Search is the one read in atgc -whose results are not all one collection: twelve hits here span -`sh.tangled.repo`, `sh.tangled.string`, `sh.tangled.repo.issue`, -`sh.tangled.repo.pull` and `sh.tangled.feed.comment`, and no two of them are -named the same way — a `title`, a `name`, a `filename`, and in the comment's -case nothing at all. The lexicon is no help here, since both `value` and -`score` are typed `unknown`. - -The fixture has already earned its keep once. A comment's `body` is not a -string: it is a `sh.tangled.markup.markdown` object carrying the prose under -`text`, while a pull's and an issue's `body` are plain strings. The first -version of the excerpt reader took `as_str()` and left the one collection -with no title as the one collection with nothing to print, which no invented -fixture would have caught. The malformed half is constructed rather than -captured — a hit with no `uri`, a `value` that is a string, a `score` that is -not a number — since Bobbin does not serve those on request. - -`pds_repos_page.json` is the same idea for `repo list`, and captured off -somebody else's account on purpose: - -``` -curl 'https://grisette.us-west.host.bsky.network/xrpc/com.atproto.repo.listRecords?repo=did:plc:qfpnj4og54vl56wngdriaxug&collection=sh.tangled.repo&limit=100' -``` - -This account has been on Tangled since before the record key migration, so -its 100-record page holds both shapes at once: current records keyed by the -repo's name with no `name` field, and older ones keyed by a TID with the name -in the field. It also holds five pairs of repos whose `createdAt` order -reverses depending on whether the timestamps are parsed or compared as text, -because Tangled writes whatever offset the client was in. Six records were -kept, including the two that are the same repo written a second apart either -side of the migration, one in `Z` and one at `+02:00` — the pair that makes -"sort newest first" a real assertion rather than a tautology. The author's -own account has one repo and could prove neither. - -## Where the seams are - -Most of this codebase interleaves pure logic with I/O in the same function, -which is fine for a tool this size and is not something to go and fix. The -rule for extraction is: pull something out when the same logic already -appears more than once, or when it is the interesting part of a function -whose other half is a socket. Do not pull something out because it would be -tidier if it were smaller. - -A refactor that churns hundreds of lines to make a one-line function testable -is a bad trade and should be refused. Concretely, in this tree: the printing -loops in `pr::list` and `repo::list` are not worth restructuring into -formatter functions; `resolve::repo_ref`'s redirect loop is not worth -inverting behind a trait so a fake client can drive it; `auth::login` is not -worth decomposing to test the parts around the browser round trip. - -What has been extracted, and why each was worth it: `ellipsize` and -`round_count` in `pr.rs` (three or four copies each, and the character-safe -truncation is a real panic risk on a title with an emoji in it); -`classify_empty` (the logic a commit message had to explain); -`owner_and_name`, `keys_from_records`, `handle_from_doc`, `pds_from_doc` and -`callback_query` (the parsing half of a function whose other half is a -request). Each is under ten lines and each has a caller that reads better for -it. - -`repo.rs`'s copy of the truncation logic is deliberately left duplicated: -the rewrite it was waiting on has landed, and it is still not worth having -`repo list` reach into `pr.rs` for a display helper. - -`config/account/selection.rs` is the newest feature and is only partly reachable. `select()` -reads the session store and so cannot be tested; `resolve_spec` can be, but -only down its DID branch, which returns before any request is made. That -branch is the one worth having — it is what decides whether `--account` acts -as the account you named or somebody else — and its refusal path, which -lists the accounts you do hold rather than falling through to one of them, -is the behaviour the code's own comment calls the worst possible failure. - -## Testing the process itself - -Almost everything here is a unit test inside `src/`. The exception is a -property that is not a property of any function: how the *process* behaves — -its signal dispositions, its exit status, what it writes to which stream. -A `cargo test` binary has its own, so those cannot be observed from inside -one, and the test has to spawn the real thing. - -`tests/broken_pipe.rs` is the pattern. `env!("CARGO_BIN_EXE_atgc")` is the -binary cargo built for this test run, so what it drives is what a user runs; -the test pipes its stdout, reads a few bytes, drops the read end, and asserts -that atgc ended quietly rather than panicking. It is a real gate, not a -description: with `sigpipe::reset()` commented out of `main` it fails with -the panic it exists to prevent. - -Two rules for adding another. Pick a subcommand that cannot fail for an -unrelated reason — `completion` needs no session, no network and no checkout, -and is exempt from log initialization, so it touches nothing under `HOME`. -And assert on the *shape* of the outcome rather than one exact status: -`broken_pipe` accepts either a clean exit or death by SIGPIPE, because both -are correct and which one happens is a race with the pipe buffer. - -## What stays manual - -These have no hermetic test and are not going to get one. Verify them by -running the tool. - -- **The OAuth flow.** `atgc login ` should open a browser, land on a - consent page naming the granular scopes, redirect to a page that shows your - handle and avatar, and print `logged in as @you`. Then `atgc auth status` - should list the scopes and an unexpired token. Check - `~/.config/atgc/sessions.json` is mode 600 and holds exactly one - `oauth:/` key per account — pruning stale sessions is the part - that regresses quietly. -- **The OAuth log's refresh events.** `logging/oauth.rs` tests its own writer, - its fingerprints, its redaction and its concurrency claim, and the - authorization half was checked against a live PDS by running `auth login` - under a scratch `HOME` and reading the resulting `token_request` / - `token_refused` / `token_granted` lines. The refresh half — a - `grant_type=refresh_token` request, and the `session_delete` jacquard - performs when it fails permanently — has never been observed, because - producing one means burning a real session. Check it the next time a - session lapses on its own. -- **`logs oauth --follow`, and the rotation it has to survive.** The parsing, - filtering, time specs and `--incident` analysis are all pure and tested. - Following a file is not: it is a poll loop over `stat` and `read`, and the - case worth checking is the one the writer creates on its own at 8 MiB. - Reproduce it by hand — point a writer and a follower at the same scratch - log with `ATGC_OAUTH_LOG`, run a few commands against it, then - `mv oauth.jsonl oauth.jsonl.1` underneath the follower and run another. - The follower must print `log rotated` and pick up the new file rather than - going quiet, and `: > oauth.jsonl` must produce `log got shorter`. Both - were verified this way; neither has an automated test, because both need - two processes and a wall clock. -- **`logs oauth --incident` against a real failure.** Its verdicts have unit - tests over constructed entries, and it has been run against a log built by - hand to look like the incident. It has never seen a real `token_refused` - or a real `session_delete`, because none has been captured yet. Treat its - first real verdict as unproven and read the evidence lines above it, which - is why it prints them. -- **`repo create`.** Needs a knot and a real write. `--dry-run` first to - confirm the name and branch, then the real thing against a throwaway name. - Check the knot returned a `repoDid` (`--debug` shows the response), that - the record landed, and that `git remote -v` and `core.sshCommand` in - `.git/config` point where they should. -- **`repo clone`.** The identity it writes has tests; the clone around it - does not. Against a real repo, check that a matched key sends it over ssh - and pins `core.sshCommand`, that `--https` works with no account selected - at all, and that `--account` picks both the key and the identity written — - clone the same repo twice under two accounts and compare the two - `.git/config` files. Then run any atgc command inside the new checkout and - confirm `acting as … (via this repo's .git/config)`, which is the loop - between what clone writes and what selection reads. -- **Pushes and SSH key selection.** `clients/git/ssh.rs` can be tested on how it matches - keys but not on whether the match makes a push work. Confirm with a real - `git push` from a checkout where the pinned key is not the one ssh would - have offered first. -- **`pr create` and `pr resubmit` against a live PDS.** `--dry-run` covers - the patch-building half. The blob upload, the record write, and Tangled's - acceptance of the record's shape are only observable by doing it and - looking at the PR in the web view. Note the round trip in `pr resubmit` — - read the record, append, put it back — is the one place a bug could - overwrite a real PR, so check the earlier rounds are still listed. -- **`pr diff` and `pr checkout` against live records.** The parsers, the - bounded decompression, the branch naming and the appview scrape all have - tests; everything that makes them useful is a socket or a subprocess. - Verify in a **scratch clone**, never a checkout with work in it, since - `pr checkout` creates branches and can leave a `git am` stopped. What was - checked when they landed: a two-round pull printed both rounds and - `--interdiff` produced a real `git range-diff` between them; a pre-`rounds` - record from another account's PDS printed its inline patch; the same - commands ran with `HOME` pointed at an empty directory, so no session - existed to use; a pull whose patch no longer applies stopped mid-`am` and - the four recovery lines it printed were run verbatim and worked; a dirty - tree and an existing branch were both refused; and the scratch worktree the - interdiff builds was gone afterwards, on the success path and the failure - path both. -- **Knot XRPC.** `sh.tangled.repo.create` is called with a service-auth token - minted by the PDS. `--debug` prints the token's decoded claims; check `aud` - is `did:web:` and `lxm` is the method being called. The vendored - `jacquard-oauth` patch exists because those claims were wrong once. -- **The post-login page in a browser.** The tests cover what the page is - made of — the field is `about`'s art glyph for glyph, a profile's markup - arrives escaped — and nothing about how it looks. Check the field covers - the window at a few sizes and fades out at the edges rather than stopping - short, that the breath and the drifting highlight are slow enough to be - scenery, that both stop under `prefers-reduced-motion`, and that light and - dark mode each keep the bonds visible enough to read as a ladder. An - account with no avatar, and one whose avatar 404s, should both lose the - picture and nothing else. -- **`atgc about` at real terminal sizes.** Resize a terminal and watch the - frame follow it; check `COLUMNS=40 atgc about` narrows it, that a very - small terminal drops to bare text with the repo URL intact, that - `atgc about | cat` is byte-identical run to run, and that `NO_COLOR=1` - strips the escapes. -- **`repo view` against a live repo.** Its parsers are all tested against - captured knot responses; what is not testable is that the four knot reads, - the PDS read and the DID document read compose. What was checked when it - landed: this repo and `tangled.org/core` both printed a full report, with - core exercising the wide record (spindle, website, labels) and the - saturated page (`100+` branches); all five reference forms — `owner/name`, - `@owner/name`, a DID owner, an `at://` URI and a deep Tangled URL — - resolved to the same repo; the command ran with `HOME` pointed at an empty - directory, so no session existed to use; a missing repo, an unresolvable - handle, a DID with no document and an unparseable reference each failed - with a different message; output piped to a file had no escape sequences - and was byte-identical run to run, while a real terminal bolded the repo - name and `NO_COLOR=1` stopped it. An unreachable knot was checked by - pointing the knot host at unroutable TEST-NET-3: the record half of the - report still printed, one warning went to stderr rather than four, and the - command finished in 5.3s rather than the 20.4s it took before the four - knot reads were overlapped. -- **Bobbin's index lag.** `pr list` printing nothing is as likely to mean - Bobbin is hours behind as it is to mean a bug. Cross-check against the web - view before believing an empty list. -- **Account selection end to end.** The precedence order — `--account`, then - `ATGC_ACCOUNT`, then the checkout's own `user.email`, then `auth switch`, - then a sole account — is only half testable, since everything above the - repo config needs a session store. With two accounts logged in, check that - each selector wins over the one below it and that `acting as … (via …)` - names the right one, then confirm `atgc auth switch` survives a new shell. -- **What `auth login` and `auth switch` say about making an account active.** - The decision behind it is pure and is tested exhaustively (`account::advise` - over every combination of env var, checkout DID, pointer and account - count). Reading the state that feeds it is not, because it goes to `HOME`. - `auth switch` prints through the same routine and needs no session, so all - of it can be driven by hand: point `HOME` at a scratch directory holding an - `accounts.json` with two accounts and no `sessions.json`, then run - `atgc auth switch ` — a DID selector resolves with no network and no - token — in a plain directory, under `ATGC_ACCOUNT`, and inside a checkout - whose `user.email` is a third DID. The login path's own half is not - reachable that way: whether the pointer is written when there is none, and - the order of the lines around it, need a real browser consent, and have - only been read. - -## CI - -`.tangled/workflows/ci.yml` runs `cargo fmt --check`, `cargo clippy ---all-targets --locked -- -D warnings`, `cargo build --locked` and `cargo -test --locked`. All four are commands you can run locally in the same order. - -Everything in this file is compatible with that pipeline, because everything -here is hermetic: the test step needs no secrets, no session and no network, -so it can run on any spindle with a Rust toolchain and git on `PATH` — git -being a real dependency of the test step, not just of the tool. - -The workflow has never executed. No spindle is attached to this repo, so the -file's schema has not been validated once, and a green run is not evidence of -anything yet. Until one is attached, `prek run --all-files` plus `cargo test` -locally is the actual gate. diff --git a/TODO.md b/TODO.md index 5e2a150..3df91a0 100644 --- a/TODO.md +++ b/TODO.md @@ -1376,7 +1376,7 @@ warn where the stack command is better. properties make it hermetic: `Command::env` is not `unsafe`, so a child may have its own `HOME`, and jacquard returns an unexpired session without contacting anything, so a fixture session restores offline. The - argument against mocking a *parser* is unchanged; see TESTING.md + argument against mocking a *parser* is unchanged; see docs/testing.md - [x] `clients/endpoints.rs` — the five hostnames atgc has compiled in (`ATGC_PLC`, `ATGC_BOBBIN`, `ATGC_APPVIEW`, `ATGC_KNOT`, `ATGC_BSKY_APPVIEW`), each a whole base URL so a local instance on a diff --git a/docs/index.md b/docs/index.md index e992d19..0b564bc 100644 --- a/docs/index.md +++ b/docs/index.md @@ -35,7 +35,9 @@ Roughly the order in which the ideas depend on each other: tree, and what has to happen for it to leave. 9. [Module layout] — the seven folders under `src/`, what each holds, and where a new file goes. -10. [Writing documentation] — the docs build, and the rule about examples. +10. [Testing] — what earns a test, the two constraints that make parts of + this crate untestable, and when to reach for a mock. +11. [Writing documentation] — the docs build, and the rule about examples. ## Building the reference documentation @@ -78,11 +80,6 @@ time. clap definitions in `main.rs`. - **[CONTRIBUTING.md](../CONTRIBUTING.md)** — building, the commit hooks, and why pull requests go through `atgc pr create`. -- **[TESTING.md](../TESTING.md)** — what earns a test here and what does - not, the two constraints that make parts of this crate untestable, and - the list of things that stay manual. It lives at the repo root rather - than in this folder, which is arguable; see the pull request that added - this folder. - **[TODO.md](../TODO.md)** — what has shipped and what has not, in more detail than a roadmap usually gets. It is the honest answer to "does atgc do X yet". @@ -97,4 +94,5 @@ time. [Logs]: logs.md [Vendored dependencies]: vendored-dependencies.md [Module layout]: module-layout.md +[Testing]: testing.md [Writing documentation]: writing-documentation.md diff --git a/docs/testing.md b/docs/testing.md new file mode 100644 index 0000000..d1d1ec3 --- /dev/null +++ b/docs/testing.md @@ -0,0 +1,56 @@ +# Testing + +`cargo test` runs everything and takes about eight seconds. No spindle is +attached to this repo, so `prek run --all-files` plus `cargo test` locally is +the actual gate, whatever `.tangled/workflows/ci.yml` claims. + +**What earns a test.** Three things. Parsers of other people's data — every +`sh.tangled.*` record, DID document, Bobbin response and knot redirect is +written by software this project does not control, and the code is +deliberately permissive about them, which is exactly the kind of parsing that +fails silently. Things that are fiddly out of proportion to their size, like a +base64url decoder or character-safe truncation: short enough to look obviously +right, and with no natural manual check. And anything a commit message had to +justify, because the paragraph will not survive the next refactor but the test +will. Glue does not earn one — `main.rs` dispatch, the layout of a `println!`, +the four lines around a request whose parser is already tested. That kind of +coverage costs the same to maintain as a real test, goes red for unrelated +reasons, and creates the impression that a green suite means the tool works. + +**Hermetic, non-negotiable.** No network, no OAuth, no session, no reading or +writing the real `~/.config/atgc`, no touching the user's git config, no +assuming a particular Tangled repo exists. Two traps are specific to this +crate. `HOME` cannot be redirected inside the test process: that needs +`std::env::set_var`, which is `unsafe` in edition 2024, and the crate forbids +unsafe code — so a function that reads `HOME` itself is not unit-testable, and +the answer is to take the directory as a parameter and leave one untested +wrapper that names `$HOME`. And the working directory is process-wide while +cargo runs tests on threads, so `testutil::TempRepo` takes a +module-level mutex and must stay the only thing in the tree that moves the +process. Network access is allowed in exactly one place: a test marked +`#[ignore]` whose message says why. If an `#[ignore]` ever pins a known bug +rather than a slow socket, its message must say *that*, because "ignored" +reads as "unimportant" and a green suite standing over a broken function is +worse than a red one. + +**Fixtures for reads, mocks for writes.** Test a parser against real bytes +captured off the real service, never against a mock: a mock encodes what we +believe the service returns, so a test of a reader against it passes precisely +when our belief is self-consistent, which is the thing already known. A write +command is the opposite question — what atgc *sent*, in what order, as whom — +and that is atgc's own behaviour, which no fixture can observe. `tests/support/` +is the environment for it: mock services on loopback ports, a session that +restores with no network, and the real binary spawned as a child, which is +what makes an alternate `HOME` possible at all (`Command::env` is not +`unsafe`). Reach for it whenever the bug is about how two commands interact +rather than about one function's output — a round appended to the wrong +record, a chain linked to something outside its own batch, a write made with +the wrong account's credentials. + +**Keep this page this short.** Do not add per-feature notes, a list of what is +verified by hand, or an inventory of what is not covered. That kind of writing +goes stale the week after it lands and nothing checks it: this page once ran +to 535 lines and named six source files that no longer existed. A fact about +one test belongs in that test's doc comment; how a fixture was captured +belongs beside the code that loads it; a claim that something is unproven +belongs at the claim, in the code, where a reader is standing when it matters. diff --git a/src/clients/atproto/pds.rs b/src/clients/atproto/pds.rs index 54c91e0..0c7e094 100644 --- a/src/clients/atproto/pds.rs +++ b/src/clients/atproto/pds.rs @@ -484,7 +484,7 @@ mod tests { /// The values atgc actually sends must survive untouched. Encoding a DID's /// colons would still resolve, but every debug log and every capture - /// command in TESTING.md would stop matching what the code sends, which is + /// command in [`crate::docs::testing`] would stop matching what the code sends, which is /// how a URL bug hides. #[test] fn the_shapes_atgc_sends_pass_through_unencoded() { @@ -601,7 +601,7 @@ mod tests { /// sail straight past `blob_bounded`'s only check and `resp.bytes()` /// would then buffer however much the sender felt like providing. There /// is no fake HTTP response in this tree to drive `blob_bounded` itself - /// through — TESTING.md rules out mock servers on purpose — so this + /// through — [`crate::docs::testing`] rules out mock servers on purpose — so this /// pins the part that changed: the running total is what decides now, /// fed the same chunk lengths `resp.chunk()` would hand the real loop, /// with no header involved at any point. diff --git a/src/clients/endpoints.rs b/src/clients/endpoints.rs index 24204b7..32ab5ab 100644 --- a/src/clients/endpoints.rs +++ b/src/clients/endpoints.rs @@ -19,7 +19,7 @@ //! directory, Bobbin, appview and knot up on loopback ports and drives whole //! sequences of commands against them. Without a seam at each of these //! five names, a test can reach the acting account's PDS (that address -//! comes out of the session store) and nothing else. See TESTING.md. +//! comes out of the session store) and nothing else. See [`crate::docs::testing`]. //! //! What is deliberately *not* here: the acting account's PDS, a repo's knot //! as named by its `sh.tangled.repo` record, and the appview URL a resolve diff --git a/src/clients/git/ssh.rs b/src/clients/git/ssh.rs index 9649823..137d273 100644 --- a/src/clients/git/ssh.rs +++ b/src/clients/git/ssh.rs @@ -182,7 +182,7 @@ impl PushKey { /// /// Split out from the scan so the matching can be tested against a directory /// of real generated keys: `HOME` cannot be redirected in this crate's tests -/// (see TESTING.md), but a directory that is passed in can be anywhere. +/// (see [`crate::docs::testing`]), but a directory that is passed in can be anywhere. fn keys_in(dir: &Path) -> Vec<(PathBuf, String)> { let Ok(entries) = std::fs::read_dir(dir) else { return Vec::new(); @@ -486,7 +486,7 @@ mod tests { /// A real keypair, generated for the test, found in a directory that is /// not `~/.ssh` — which is the only way this can be tested at all, since - /// `HOME` is not redirectable here (TESTING.md). + /// `HOME` is not redirectable here ([`crate::docs::testing`]). /// /// The private half is what comes back, not the `.pub` that matched: /// `ssh -i` wants the key it will sign with. @@ -556,7 +556,7 @@ mod tests { } /// The base64 codec, which is hand-rolled and therefore the kind of - /// thing TESTING.md says earns a test on its own. Padded input is + /// thing [`crate::docs::testing`] says earns a test on its own. Padded input is /// accepted (a `.pub` blob usually is) and unpadded output is what a /// fingerprint wants. #[test] diff --git a/src/clients/http.rs b/src/clients/http.rs index 4d98c2e..39be213 100644 --- a/src/clients/http.rs +++ b/src/clients/http.rs @@ -207,7 +207,7 @@ mod tests { /// which on this machine ran to 30s. /// /// Ignored because it opens a socket and spends the timeout doing it, not - /// because it fails. TESTING.md allows network access in `#[ignore]`d + /// because it fails. [`crate::docs::testing`] allows network access in `#[ignore]`d /// tests and nowhere else, and a five-second test does not belong in a /// suite that finishes in one. Run with `cargo test -- --ignored`. /// diff --git a/src/clients/tangled/resolve.rs b/src/clients/tangled/resolve.rs index 07e8430..87d0278 100644 --- a/src/clients/tangled/resolve.rs +++ b/src/clients/tangled/resolve.rs @@ -383,7 +383,7 @@ mod tests { /// prove the rule above matches how Tangled actually routes — every URL /// shape in this module's table is a claim about somebody else's server. /// - /// Ignored because it touches the network, not because it fails. TESTING.md + /// Ignored because it touches the network, not because it fails. [`crate::docs::testing`] /// allows network access in `#[ignore]`d tests and nowhere else. Run with /// `cargo test -- --ignored`. #[tokio::test] diff --git a/src/cmd/about.rs b/src/cmd/about.rs index fa492f3..1207d2f 100644 --- a/src/cmd/about.rs +++ b/src/cmd/about.rs @@ -302,7 +302,7 @@ mod tests { /// Only the unset case is exercised: setting an environment variable is /// `unsafe` in edition 2024, and this crate forbids `unsafe_code`, so a /// test cannot put a value there to read back. The parsing above it is - /// covered by inspection and by the manual checks in TESTING.md. + /// covered by inspection and by resizing a real terminal. #[test] fn an_unset_dimension_is_not_asked_for() { assert_eq!(env_dimension("ATGC_TEST_DIMENSION_THAT_IS_NOT_SET"), None); diff --git a/src/cmd/completion.rs b/src/cmd/completion.rs index 13f061e..00af151 100644 --- a/src/cmd/completion.rs +++ b/src/cmd/completion.rs @@ -64,7 +64,7 @@ mod tests { /// containing `at://` URIs and a bare `*`, all of which are metacharacters /// in at least one of the five output languages. Whether the resulting /// script is syntactically valid is not checkable here — that needs the - /// shell itself, and is in TESTING.md's manual list — but a generator that + /// shell itself — but a generator that /// gives up outright is, and that is what this pins. #[test] fn generates_for_every_shell() { diff --git a/src/cmd/pr/read.rs b/src/cmd/pr/read.rs index f6b8cb1..f92243f 100644 --- a/src/cmd/pr/read.rs +++ b/src/cmd/pr/read.rs @@ -3424,8 +3424,8 @@ mod tests { assert_eq!(row.rounds, 1); } - /// The exact key set a `jq` pipeline would see — the shape TESTING.md - /// asks this kind of test to pin, not just the values inside it. + /// /// The exact key set a `jq` pipeline would see, pinned as a shape rather + /// than as the values inside it: a renamed field breaks a caller silently. #[test] fn pull_row_json_serializes_with_the_documented_field_names() { let item = &bobbin_items()[0]; diff --git a/src/config/account/selection.rs b/src/config/account/selection.rs index 3efc2f9..9ffb4f6 100644 --- a/src/config/account/selection.rs +++ b/src/config/account/selection.rs @@ -422,7 +422,7 @@ pub struct CheckoutAccount { /// /// The split is not tidiness. `read` goes to the environment, the checkout /// and `~/.config/atgc/accounts.json`, and none of those can be redirected -/// in-process (TESTING.md: `HOME` needs `set_var`, which the crate forbids). +/// in-process ([`crate::docs::testing`]: `HOME` needs `set_var`, which the crate forbids). /// So the reading half is only ever verified by hand and the deciding half /// is tested over every combination. #[derive(Debug, Clone, Default, PartialEq, Eq)] @@ -900,7 +900,7 @@ mod tests { /// The DID branch of `resolve_spec` returns before any request is made, /// which is what makes it testable at all — the handle branch goes to the - /// network and is covered by the manual checks in TESTING.md. + /// network and is only checkable by running the tool. #[tokio::test] async fn a_did_selector_matches_a_held_session() { let accounts = [known(DID, Some("permadeath.com")), known(OTHER, None)]; diff --git a/src/config/dir.rs b/src/config/dir.rs index 29a0590..ce77429 100644 --- a/src/config/dir.rs +++ b/src/config/dir.rs @@ -129,7 +129,7 @@ mod tests { /// the platform default mode and `chmod`ed to 0600 only after the /// tokens were already on disk — a real window, on a shared machine, /// where another local user could read them. This is the part of the - /// fix checkable without `$HOME` (see TESTING.md): pass an explicit + /// fix checkable without `$HOME` (see [`crate::docs::testing`]): pass an explicit /// path in a throwaway directory and confirm 0600 lands on the file /// that `path` resolves to, both when the file is new and when an /// atomic write is replacing one that already exists — the second diff --git a/src/config/lock.rs b/src/config/lock.rs index 08dbda6..65bc9ea 100644 --- a/src/config/lock.rs +++ b/src/config/lock.rs @@ -417,7 +417,7 @@ mod tests { /// increment vanishes — so an exact final count is the assertion that /// the exclusion held every single time, not merely most of the time. /// `take` itself is not called because it resolves `$HOME`, which a - /// hermetic test cannot redirect (see TESTING.md); this drives the same + /// hermetic test cannot redirect (see [`crate::docs::testing`]); this drives the same /// file and the same `try_lock` through a temp path. #[test] fn the_lock_stops_a_concurrent_read_modify_write_losing_an_update() { diff --git a/src/docs.rs b/src/docs.rs index 3591b57..fc15a8a 100644 --- a/src/docs.rs +++ b/src/docs.rs @@ -42,6 +42,10 @@ pub mod pull_requests { #![doc = include_str!("../docs/pull-requests.md")] } +pub mod testing { + #![doc = include_str!("../docs/testing.md")] +} + pub mod sessions { #![doc = include_str!("../docs/sessions.md")] } diff --git a/src/term/hyperlink.rs b/src/term/hyperlink.rs index c0221bf..65a2412 100644 --- a/src/term/hyperlink.rs +++ b/src/term/hyperlink.rs @@ -26,7 +26,7 @@ use std::io::IsTerminal; /// A free function of three bools rather than a struct read from the /// environment so a test can hold all eight combinations still instead of /// mutating `TERM` or `NO_COLOR` on the process — which `cargo test`'s -/// shared, threaded environment makes unsafe to do at all (see TESTING.md). +/// shared, threaded environment makes unsafe to do at all (see [`crate::docs::testing`]). pub fn escapes_wanted(is_terminal: bool, no_color: bool, term_is_dumb: bool) -> bool { is_terminal && !no_color && !term_is_dumb } diff --git a/src/term/say.rs b/src/term/say.rs index 7069cf5..35a9329 100644 --- a/src/term/say.rs +++ b/src/term/say.rs @@ -234,7 +234,7 @@ pub fn init(quiet: u8, debug: bool) { } /// The one decision, kept pure so tests can hold it still rather than -/// mutating the environment of a threaded test binary (see TESTING.md). +/// mutating the environment of a threaded test binary (see [`crate::docs::testing`]). /// /// `ATGC_LOG` is a comma-separated list of `` — which moves the /// default — and `=`, which moves one topic; `off` silences diff --git a/src/testutil.rs b/src/testutil.rs index 44d6e09..7f9d5bb 100644 --- a/src/testutil.rs +++ b/src/testutil.rs @@ -1,6 +1,6 @@ //! Shared scaffolding for tests. Compiled only under `cfg(test)`. //! -//! Everything here exists to keep tests hermetic — see TESTING.md. Two +//! Everything here exists to keep tests hermetic — see [`crate::docs::testing`]. Two //! things live here: a throwaway git repo with the process parked inside it, //! which several modules need because the code under test shells out to git //! in the working directory rather than against an explicit path, and a diff --git a/tests/exit_status.rs b/tests/exit_status.rs index e433565..5de1756 100644 --- a/tests/exit_status.rs +++ b/tests/exit_status.rs @@ -6,11 +6,11 @@ //! `CARGO_BIN_EXE_atgc` is the binary cargo built for this run, so what is //! measured here is what a script measures. //! -//! **Hermetic, which limits what can be checked here.** TESTING.md forbids a -//! test that reads the real `~/.config/atgc` or opens a socket, and the -//! statuses that need a network round trip — `Denied`, `Unreachable` — cannot -//! be produced without one. Those are covered by `exit::classify`'s own -//! tests. What is left, and what is worth having at this level, is the +//! **Hermetic, which limits what can be checked here.** docs/testing.md +//! forbids a test that reads the real `~/.config/atgc` or opens a socket, and +//! the statuses that need a network round trip — `Denied`, `Unreachable` — +//! cannot be produced without one. Those are covered by `exit::classify`'s +//! own tests. What is left, and what is worth having at this level, is the //! wiring: that `main` returns a status at all, that a success is `0`, that //! the two paths to `2` agree, and that a failure keeps stdout empty. //! diff --git a/tests/pr_flows.rs b/tests/pr_flows.rs index 9c765b0..c0e423d 100644 --- a/tests/pr_flows.rs +++ b/tests/pr_flows.rs @@ -6,7 +6,7 @@ //! `pr resubmit` and `pr edit` both read a record, change one thing and put //! the whole thing back, and the bug that shape invites is losing whatever //! it did not mean to touch — the rounds a resubmit is not appending to, the -//! title an edit is not editing. TESTING.md names this as the one place a +//! title an edit is not editing. docs/testing.md names this as the one place a //! bug could overwrite a real pull, and lists it under what stays manual; //! this is that check, automated. //! @@ -146,7 +146,7 @@ fn a_dry_run_create_sends_nothing() { /// A round is *appended*. The earlier rounds are still there, still point at /// the blobs they always did, and the title and body are untouched. /// -/// This is the read-modify-write that TESTING.md singles out as the one +/// This is the read-modify-write that docs/testing.md singles out as the one /// place a bug could overwrite a real pull. The failure it guards is not a /// crash: it is a record that comes back with one round where it had two, /// which every listing then reports as a perfectly healthy pull. diff --git a/tests/support/account.rs b/tests/support/account.rs index 0674c49..4fec75d 100644 --- a/tests/support/account.rs +++ b/tests/support/account.rs @@ -69,7 +69,7 @@ impl Account { /// this tree — and the mock PDS grants every method, so a scope string here /// would assert only that this file and `clients/tangled/scope.rs` were /// copied from each other. What a session's scopes actually buy is decided -/// at a real PDS, which is `scope.rs`'s own tests and TESTING.md's manual +/// at a real PDS, which is `scope.rs`'s own tests and docs/testing.md's manual /// list. pub fn install(config_dir: &Path, accounts: &[Account], pds: &str) { std::fs::create_dir_all(config_dir).expect("config dir"); diff --git a/tests/support/mod.rs b/tests/support/mod.rs index 7a7ba43..0887148 100644 --- a/tests/support/mod.rs +++ b/tests/support/mod.rs @@ -4,7 +4,7 @@ //! //! # Why this exists beside the unit tests //! -//! TESTING.md argues against mocking a service in order to test a parser, +//! docs/testing.md argues against mocking a service in order to test a parser, //! and that argument still holds: a fixture asserting what Bobbin returns //! passes precisely when our belief about Bobbin is self-consistent, which //! is the thing already known. -- 2.51.2