Skip to content

Commit 68d19d1

Browse files
committed
docs(adr): order-independent tests and parallelism
why: Record the decision behind the test-isolation fixes and the opt-in xdist mode so the rationale and rejected alternatives survive. what: - Add ADR 0002 (order-independence invariant + opt-in parallelism, with alternatives, consequences, and prior art) - Register it in the ADR index toctree
1 parent de3011e commit 68d19d1

2 files changed

Lines changed: 119 additions & 0 deletions

File tree

Lines changed: 118 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,118 @@
1+
(adr-order-independent-tests)=
2+
3+
# ADR 0002: Order-independent tests with opt-in parallelism
4+
5+
## Status
6+
7+
Accepted. 2026-06-28.
8+
9+
## Context
10+
11+
The test suite is subprocess-bound: it spawns real `git`, `hg`, and `svn`
12+
processes and writes working copies to disk. Wall-time lives in process
13+
spawning and filesystem I/O, not Python, so the single largest lever for a
14+
faster suite is running independent tests across CPU cores.
15+
16+
Parallel execution (and, equivalently, shuffled execution) is only safe when
17+
the suite is **order-independent**: every test must pass regardless of what ran
18+
before it. The suite was not. Two pieces of shared mutable state passed in the
19+
fixed collection order but failed once tests were reordered:
20+
21+
- `libvcs.url.registry.VCSRegistry` stored its `parser_map` in a class
22+
variable, so constructing any second registry mutated the module-level
23+
`registry` singleton — whichever test built a custom registry first changed
24+
URL detection for every later test.
25+
- The `git_repo` / `hg_repo` pytest fixtures handed the first consumer a live
26+
handle to a session-cached master checkout. The first test to use the fixture
27+
could mutate that cache (add a remote, switch a branch), leaking state into
28+
every later test that copied it.
29+
30+
These were invisible under the default order and surfaced only when a
31+
shuffled run (`pytest-randomly`) or a parallel run (`pytest-xdist`) changed
32+
which tests ran together and in what sequence. They are also genuine bugs for
33+
downstream consumers (e.g. vcspull) that build on `VCSRegistry` and the
34+
fixtures, independent of how the tests run.
35+
36+
## Decision
37+
38+
Two coupled commitments — the invariant is the prerequisite for the mechanism.
39+
40+
### Tests are order-independent
41+
42+
No shared mutable state may leak across tests. Coupling is fixed at the source,
43+
never hidden behind a fixed order or a co-locating scheduler:
44+
45+
- `VCSRegistry.parser_map` is a per-instance attribute; each registry is
46+
independent and the global `registry` is never mutated by constructing
47+
another.
48+
- The `git_repo` / `hg_repo` / `svn_repo` fixtures treat the master checkout as
49+
a pristine, read-only cache and hand every consumer — including the first —
50+
its own copy. A test may mutate its checkout freely without affecting any
51+
other.
52+
53+
The expectation is documented in {ref}`workflow` so contributors keep new tests
54+
order-independent, with a shuffled-run check (`uv run --with pytest-randomly
55+
py.test -p randomly`).
56+
57+
### Parallelism is opt-in, not the default
58+
59+
`pytest-xdist` is a development dependency exposed via `just test-parallel`
60+
(`uv run py.test -n auto`). The default `uv run py.test` stays serial.
61+
62+
- The worker count is **not** hardcoded. `-n auto` adapts to the machine; the
63+
operator caps it when needed. A subprocess-bound suite oversubscribes on
64+
high-core machines, so a fixed count committed to `addopts` would be wrong
65+
somewhere.
66+
- The default scheduler (`load`) is used. A co-locating scheduler (`loadfile`)
67+
was trialled as a workaround for the order-coupling, but once the coupling is
68+
fixed it is unnecessary, so it is not committed.
69+
70+
## Alternatives considered
71+
72+
| Approach | order-safe | default unchanged | portable | chosen |
73+
|----------|:----------:|:-----------------:|:--------:|:------:|
74+
| Fix coupling at source + opt-in `-n auto` (`load`) | yes | yes | yes | **yes** |
75+
| Keep `--dist=loadfile` workaround, leave coupling | masks it | yes | yes | no |
76+
| `-n auto` in `addopts` (always parallel) | needs fix | no | no | no |
77+
| Hardcode a worker count (e.g. `-n 12`) | needs fix | no | no | no |
78+
| `pytest-randomly` as a committed CI gate | detects only | yes | yes | deferred |
79+
80+
The decisive point: a co-locating scheduler only *hides* order-coupling, and it
81+
was measured to be no faster than the default scheduler once the coupling was
82+
fixed — so the coupling is fixed and the workaround dropped. `pytest-randomly`
83+
detects order-coupling but does not parallelize; it is used transiently for
84+
checks (`uv run --with pytest-randomly`) rather than committed.
85+
86+
## Consequences
87+
88+
### Positive
89+
90+
- The suite passes under serial, shuffled, and parallel (`-n auto`) execution.
91+
- Two real isolation bugs in shipped code (`VCSRegistry`, the repo fixtures)
92+
are fixed, benefiting downstream consumers regardless of parallelism.
93+
- A faster opt-in run is available without changing the stable default.
94+
95+
### Tradeoffs
96+
97+
- Parallel wall-time varies with machine load, and `-n auto` can oversubscribe
98+
a subprocess-bound suite on high-core machines — the operator picks the
99+
worker count.
100+
- Contributors must keep new tests order-independent (reset global state in
101+
teardown; isolate per-test resources).
102+
103+
### Risks
104+
105+
- A future order-coupling could reintroduce flakiness that appears only under
106+
parallel or shuffled runs. Mitigation: the order-independence expectation is
107+
documented with a shuffled-run check, so the regression is reproducible.
108+
109+
## Prior art
110+
111+
- `pytest-xdist` provides the `load`, `loadscope`, and `loadfile` schedulers;
112+
its guidance is that tests must be independent for `load` to be safe.
113+
- `pytest-randomly` randomizes order specifically to surface inter-test
114+
coupling.
115+
- The broader convention across test suites: parallel execution requires
116+
order-independent tests, and shared mutable state (global registries, cached
117+
fixtures handed out by reference) is the usual cause of order-dependent
118+
failures.

docs/project/adr/index.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,4 +8,5 @@ Significant design decisions and their rationale.
88
:maxdepth: 1
99
1010
0001-faithful-subprocess-output-capture
11+
0002-order-independent-tests
1112
```

0 commit comments

Comments
 (0)