mirror of
https://github.com/pnpm/pnpm.git
synced 2026-07-20 20:52:33 -04:00
docs(pacquet): add shared-process-state DI exception to the style guide
The "Dependency injection for tests" gating list in
`pacquet/CODE_STYLE_GUIDE.md` enumerated four reasons to reach past
real fixtures for the DI seam: filesystem error kinds, deterministic
time, external-service happy paths, and unreachable-by-design
preconditions. It did not cover the case this PR retired from
`default_store_dir`: tests that mutate a single per-process slot
(env vars, cwd, umask, signal handlers, …) and so race with every
other test in the same process.
The pre-existing `EnvGuard` workaround restored correctness only by
holding a binary-wide mutex around `unsafe { env::set_var(...) }`,
which forecloses parallelism inside the affected tests and leaves
`unsafe` in the test source. A capability-trait fake keeps the read
deterministic and the mutation contained to the test that needs it.
Adds that bullet to the style guide between "Deterministic time" and
"External-service happy paths," and updates rule 7 in
`pacquet/AGENTS.md` (and its `CLAUDE.md` / `GEMINI.md` symlink
targets) so the summary mirrors the longer list.
---
Written by an agent (Claude Code, claude-opus-4-7).
This commit is contained in:
@@ -49,7 +49,9 @@ Before writing code for a feature, bug fix, or behavior change:
|
||||
binary. Use the DI seam — a capability trait on the `Host`
|
||||
provider, threaded as `Sys: <Bounds>` — only for branches a real
|
||||
fixture can't reach portably: filesystem error kinds
|
||||
(`PermissionDenied`, `ENOSPC`, …), deterministic time, or the
|
||||
(`PermissionDenied`, `ENOSPC`, …), deterministic time, shared
|
||||
process-global state a test would otherwise mutate
|
||||
(`env::set_var`, `set_current_dir`, the umask, …), 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)
|
||||
|
||||
@@ -512,6 +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.
|
||||
- **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.
|
||||
|
||||
|
||||
Reference in New Issue
Block a user