docs(pacquet): drop unfounded set_current_dir example from DI guidance

The previous commit cited `set_current_dir` and "the umask" as
illustrative examples in both the style guide and the AGENTS rule 7
summary. Neither is actually used anywhere in the pacquet codebase
today — only `env::set_var` is. Speculative examples in a style
guide invite contributors to chase patterns that don't exist, so
narrow the wording to the one slot we have a concrete example for
while keeping the principle generalisable (the bullet still says
"any future analogue").

---
Written by an agent (Claude Code, claude-opus-4-7).
This commit is contained in:
Claude
2026-05-18 13:56:05 +00:00
parent 96040d7934
commit d930beda53
2 changed files with 2 additions and 2 deletions

View File

@@ -51,7 +51,7 @@ Before writing code for a feature, bug fix, or behavior change:
fixture can't reach portably: filesystem error kinds
(`PermissionDenied`, `ENOSPC`, …), deterministic time, shared
process-global state a test would otherwise mutate
(`env::set_var`, `set_current_dir`, the umask, …), or the
(`env::set_var` is the one example we have today), or the
external-service happy paths in features like `pnpm login` (2FA)
and `pnpm publish` (OIDC / provenance) when those land. See
[Dependency injection for tests](./CODE_STYLE_GUIDE.md#dependency-injection-for-tests)

View File

@@ -512,7 +512,7 @@ The dependency-injection seam described below is the **narrow second route**. Re
- **Filesystem error branches the host OS won't reproduce portably.** `PermissionDenied`, `ENOSPC`, a directory that disappears mid-walk, a chmod that fails after the file exists — provoking these on real disks is platform-specific, racy, or both. A fake that returns the exact `io::ErrorKind` is the only portable way to drive the branch.
- **Deterministic time.** Asserting that `prunedAt` equals a specific HTTP-date (RFC 7231 IMF-fixdate, what `httpdate::fmt_http_date` emits), or that a throttled emitter fires on the second sample, needs the clock to be a known value. The real `SystemTime::now` makes those assertions flaky.
- **Shared process-global state that tests would otherwise mutate.** When a branch depends on a single per-process slot — environment variables, the current working directory, the umask, signal handlers, the global allocator — the only way to exercise it without DI is to write to that slot, and the write is observed by every other test in the same process. A serialisation lock (the `EnvGuard` pattern that pnpm/pacquet#343 + pnpm/pnpm#11718 retired from `default_store_dir`) restores correctness only by forcing the affected tests to run single-threaded, and leaves an `unsafe { env::set_var(...) }` block at the call site. A capability-trait fake keeps the read deterministic and the mutation contained to the test that needs it, so nextest's in-process parallelism stays useful and `unsafe` stays out of test code. The reverse follows too: a real-fixture happy path should not be promoted to DI just because it crosses a process-global slot — only the *mutation* side of the slot is the problem; reading whatever the shell already has is fine.
- **Shared process-global state that tests would otherwise mutate.** When a branch depends on a single per-process slot — environment variables today; any future analogue like a global allocator or signal-handler table tomorrow — the only way to exercise it without DI is to write to that slot, and the write is observed by every other test in the same process. A serialisation lock (the `EnvGuard` pattern that pnpm/pacquet#343 + pnpm/pnpm#11718 retired from `default_store_dir`) restores correctness only by forcing the affected tests to run single-threaded, and leaves an `unsafe { env::set_var(...) }` block at the call site. A capability-trait fake keeps the read deterministic and the mutation contained to the test that needs it, so nextest's in-process parallelism stays useful and `unsafe` stays out of test code. The reverse follows too: a real-fixture happy path should not be promoted to DI just because it crosses a process-global slot — only the *mutation* side of the slot is the problem; reading whatever the shell already has is fine.
- **External-service happy paths that can't be staged in CI.** Upstream pnpm has features whose *normal* flow depends on real external systems — `pnpm login`'s 2FA prompt round-trip, the OIDC token exchange and provenance attestation in `pnpm publish`, and similar — where the happy path itself is what needs faking, not just the error path. When pacquet ports those features, DI is the right tool for their tests too. (As of writing, these are not yet ported; this exception is documented so the convention is in place when they land.)
- **Unreachable-by-design preconditions.** When a function declares a capability bound but a specific test exercises a branch that never reaches that capability, the fake satisfies the bound with `unreachable!` and documents the precondition. See the worked example below.