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.