Skip to content

Commit de09f1a

Browse files
jeremymanningclaude
andcommitted
Issue #130: Add handoff plan for the pytest config-shadowing fix
pytest.ini uses [tool:pytest], a header valid only in setup.cfg. pytest selects the file anyway and then stops searching, so pyproject.toml's [tool.pytest.ini_options] is never read either -- two config files, zero effective configuration. No markers registered, --strict-markers inert, testpaths inert, addopts inert. The handoff documents a landmine worth flagging here: naively fixing this breaks bare `pytest` outright. master's pyproject sets testpaths = ["tests/unit", "tests/integration"], and pytest populates config.args from testpaths when no paths are given -- so the #109 billable-test guard, which inspects config.args, refuses the run. Verified: $ mv pytest.ini /tmp && pytest --co -q ERROR: Refusing to run 'tests/integration': ... The fix is to key the guard off config.invocation_params.args (what the user actually typed) rather than config.args (which includes resolved testpaths), and to narrow testpaths to ["tests"]. Both behaviours were verified against pytest 8.4.2, as were the pytestconfig.inipath / getini("markers") APIs the proposed regression test relies on. Also records the measured marker gap: pyproject declares 4 markers, tests use 7 non-builtin ones, so `expensive`, `dartmouth_network` and `performance` must be declared before --strict-markers is enabled, and `cleanup` (test_kubernetes_performance_benchmarks.py:1038) is declared nowhere at all. No behaviour change in this commit -- documentation only. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012gTBDPK16HUZ3kHQ2QyjuU
1 parent a9393b7 commit de09f1a

1 file changed

Lines changed: 307 additions & 0 deletions

File tree

notes/handoff_130_pytest_config.md

Lines changed: 307 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,307 @@
1+
# Handoff: fix #130 — pytest.ini is dead config shadowing pyproject.toml
2+
3+
**Written:** 2026-08-17 · **For:** a fresh session picking this up cold
4+
**Issue:** ContextLab/clustrix#130 · **Parent:** #108 · **Related:** #110, #113, #115, #117
5+
6+
Everything below was verified on `master` at `a9393b7`. Commands are copy-pasteable.
7+
Where a fact is stated, the command that established it is given so you can re-verify
8+
rather than trust this document.
9+
10+
---
11+
12+
## 1. What is wrong
13+
14+
`pytest.ini` line 1 is `[tool:pytest]`. That section header is valid **only in
15+
`setup.cfg`**. In a file named `pytest.ini`, pytest requires `[pytest]`.
16+
17+
pytest still *selects* `pytest.ini` as its config file — and having selected one, it
18+
**stops searching**. So `pyproject.toml`'s `[tool.pytest.ini_options]` is never read
19+
either.
20+
21+
Net result: two config files, **zero** effective configuration.
22+
23+
```bash
24+
head -1 pytest.ini # -> [tool:pytest]
25+
pytest tests/unit/ --co 2>&1 | grep configfile # -> configfile: pytest.ini
26+
pytest --markers | grep -c '^@pytest.mark.real_world' # -> 0 (nothing registered)
27+
```
28+
29+
Strict-marker check (should error if strict mode were live, but does not):
30+
31+
```bash
32+
printf 'import pytest\n@pytest.mark.bogus_xyz\ndef test_x(): assert True\n' > tests/unit/test_probe_tmp.py
33+
pytest tests/unit/test_probe_tmp.py --co -q # -> "1 test collected"
34+
rm tests/unit/test_probe_tmp.py
35+
```
36+
37+
### Inert today
38+
39+
| Setting | Declared in | Effect today |
40+
|-|-|-|
41+
| `addopts` | both | none |
42+
| `testpaths` | both | none — bare `pytest` walks the whole repo |
43+
| `markers` | both | **none registered** |
44+
| `filterwarnings` | both | none |
45+
| `--strict-markers` | pytest.ini `addopts` | inert |
46+
47+
---
48+
49+
## 2. Why it matters
50+
51+
1. **It invalidates a stated premise of #110.** #110 says `pip install -e ".[dev]"`
52+
yields an env where pytest cannot start because `addopts` requires `pytest-xdist`.
53+
That is true only where `pytest.ini` is *absent* — the closed epic branch deletes it.
54+
On `master`, `addopts` never applies at all, and `master`'s pyproject `addopts` is
55+
just `-v --tb=short` (no `-n 4`). Update #110 to say which config was live.
56+
2. **It is why #109's billable-test gate could not use markers.** Applying
57+
`pytest.mark.expensive` today emits `PytestUnknownMarkWarning` and is not a usable
58+
selector. The gate uses `collect_ignore_glob` + a `pytest_configure` guard instead.
59+
3. **Inert `--strict-markers` hides typos.** One is already in the tree:
60+
`tests/real_world/test_kubernetes_performance_benchmarks.py:1038` uses
61+
`@pytest.mark.cleanup`, which is declared in **neither** file and does nothing.
62+
63+
---
64+
65+
## 3. ⚠️ The landmine — read before changing anything
66+
67+
**Naively fixing this breaks `pytest` completely.** Verified:
68+
69+
```bash
70+
mv pytest.ini /tmp/ && pytest --co -q ; mv /tmp/pytest.ini .
71+
```
72+
```
73+
ERROR: Refusing to run 'tests/integration': tests/integration provisions real,
74+
billable cloud resources (AWS EKS/EC2). Set CLUSTRIX_ALLOW_BILLABLE=1 ...
75+
```
76+
77+
Why: `master`'s pyproject has
78+
79+
```toml
80+
testpaths = ["tests/unit", "tests/integration"]
81+
```
82+
83+
When no paths are given on the command line, pytest populates `config.args` **from
84+
`testpaths`**. The #109 guard in `tests/conftest.py` inspects `config.args` and refuses
85+
any run targeting `tests/integration`. So the moment pyproject becomes live, bare
86+
`pytest` aborts.
87+
88+
### The clean fix
89+
90+
The guard's *intent* is "refuse when the user explicitly asks for these tests". That
91+
means it should key off what the user typed, not off resolved testpaths.
92+
`config.invocation_params.args` is exactly that. Verified:
93+
94+
| invocation | `config.args` | `config.invocation_params.args` |
95+
|-|-|-|
96+
| `pytest` (testpaths live) | `['tests/unit', 'tests/integration']` | *(no path entries)* |
97+
| `pytest tests/unit/` | `['tests/unit/']` | `['tests/unit/', ...flags]` |
98+
99+
Note `invocation_params.args` includes flags, so skip entries beginning with `-`.
100+
101+
Do **both** of these:
102+
- switch the guard to `config.invocation_params.args`, and
103+
- set `testpaths = ["tests"]` (integration stays excluded by `collect_ignore_glob`).
104+
105+
Belt and braces: `collect_ignore_glob` in `tests/integration/conftest.py` continues to
106+
handle directory traversal regardless.
107+
108+
---
109+
110+
## 4. Current state, measured
111+
112+
### Marker declarations disagree between the two files
113+
114+
| Source | Markers declared |
115+
|-|-|
116+
| `pytest.ini` (inert) | 14 |
117+
| `pyproject.toml` (shadowed) | **4**`real_world`, `slow`, `unit`, `integration` |
118+
119+
### Markers actually used in `tests/` (non-builtin)
120+
121+
```
122+
real_world 224 uses
123+
dartmouth_network 11 uses 4 files <- NOT in pyproject
124+
slow 6 uses
125+
expensive 5 uses 5 files <- NOT in pyproject
126+
performance 4 uses 4 files <- NOT in pyproject
127+
integration 2 uses
128+
cleanup 1 use 1 file <- NOT in either file; likely a typo
129+
```
130+
131+
Regenerate with:
132+
```bash
133+
python3 - <<'PY'
134+
import re, pathlib, tomllib
135+
d = tomllib.load(open("pyproject.toml","rb"))
136+
declared = {m.split(":")[0].strip() for m in d["tool"]["pytest"]["ini_options"]["markers"]}
137+
used = set()
138+
for p in pathlib.Path("tests").rglob("*.py"):
139+
used |= set(re.findall(r"@pytest\.mark\.([a-z_]+)", p.read_text(errors="replace")))
140+
builtin = {"parametrize","skip","skipif","xfail","usefixtures","filterwarnings","timeout","asyncio"}
141+
print("gap:", sorted(used - declared - builtin))
142+
PY
143+
```
144+
(needs Python 3.11+ for `tomllib`; on this machine use `/opt/homebrew/bin/python3.12`)
145+
146+
### Dependency note
147+
148+
`pytest-xdist` is in the `[test]` extra, **not `[dev]`**. `master`'s pyproject `addopts`
149+
does not use `-n`, so activating pyproject does **not** require xdist. Do not add `-n`
150+
to `addopts` without also moving xdist into `[dev]` — that is the #110 trap.
151+
152+
---
153+
154+
## 5. Plan
155+
156+
Land as **one PR**, but in the commit order below, verifying after each step. Do not
157+
combine steps: each one changes what the next one measures.
158+
159+
### Step 0 — baseline
160+
```bash
161+
git checkout master && git pull
162+
pytest tests/ -m "not real_world" --co -q -o addopts= 2>&1 | tail -1 # record this number
163+
pytest tests/unit/ -o addopts= -q 2>&1 | tail -1 # expect: 70 passed
164+
```
165+
Record both. Every later step must not reduce them.
166+
167+
### Step 1 — defuse the landmine (must be first)
168+
- In `tests/conftest.py`, change `pytest_configure` to iterate
169+
`config.invocation_params.args`, skipping entries starting with `-`.
170+
- Keep the existing message and the `CLUSTRIX_ALLOW_BILLABLE` opt-in.
171+
- Update the docstring: it currently says "args", which will no longer be accurate.
172+
173+
Verify — all of these must still hold:
174+
```bash
175+
pytest tests/integration/test_timeout_mechanism.py --co -q # refused
176+
pytest tests/integration/ --co -q # refused
177+
pytest tests/ -m "not real_world" --co -q -o addopts= | grep -c '^tests/integration/' # 0
178+
pytest tests/unit/ -o addopts= -q | tail -1 # still 70 passed
179+
```
180+
181+
### Step 2 — pick one config source
182+
- **Delete `pytest.ini`.** Keep `pyproject.toml` (modern convention, single source).
183+
- Set `testpaths = ["tests"]`.
184+
- Do **not** add `--strict-markers` yet.
185+
186+
Verify:
187+
```bash
188+
pytest --co 2>&1 | grep configfile # -> pyproject.toml
189+
pytest --co -q 2>&1 | tail -1 # bare pytest works, collects, does not abort
190+
pytest tests/unit/ -o addopts= -q | tail -1
191+
```
192+
193+
### Step 3 — reconcile the markers
194+
Add to pyproject's `markers`: `expensive`, `dartmouth_network`, `performance`.
195+
Decide on `cleanup` (`test_kubernetes_performance_benchmarks.py:1038`) — either declare
196+
it or delete the marker. It has one use and no meaning today; deleting is likely right,
197+
but check with the author first.
198+
199+
Verify:
200+
```bash
201+
pytest --markers | grep -cE '^@pytest.mark.(real_world|slow|unit|integration|expensive|dartmouth_network|performance):' # -> 7
202+
```
203+
204+
### Step 4 — enable `--strict-markers` LAST
205+
Add it to pyproject `addopts`.
206+
207+
Verify:
208+
```bash
209+
pytest tests/ --co -q -o addopts="--strict-markers" 2>&1 | tail -3 # 0 errors
210+
printf 'import pytest\n@pytest.mark.bogus_xyz\ndef test_x(): assert True\n' > tests/unit/test_probe_tmp.py
211+
pytest tests/unit/test_probe_tmp.py --co -q # MUST now error
212+
rm tests/unit/test_probe_tmp.py
213+
```
214+
215+
### Step 5 — regression guard
216+
Add a test asserting the active config is the expected file, so a stray config can never
217+
silently shadow it again:
218+
219+
```python
220+
def test_pytest_reads_the_intended_config(pytestconfig):
221+
assert pytestconfig.inipath is not None, "no config file loaded at all"
222+
assert pytestconfig.inipath.name == "pyproject.toml", (
223+
f"pytest loaded {pytestconfig.inipath}; a stray config file is shadowing "
224+
"pyproject.toml (see #130)"
225+
)
226+
227+
228+
def test_project_markers_are_registered(pytestconfig):
229+
# getini("markers") also returns plugin-provided markers such as
230+
# "timeout(timeout, method=None, ...): ..." -- strip the argspec as well as
231+
# the description before comparing names.
232+
registered = {
233+
entry.split(":")[0].split("(")[0].strip()
234+
for entry in pytestconfig.getini("markers")
235+
}
236+
for required in (
237+
"real_world",
238+
"expensive",
239+
"integration",
240+
"dartmouth_network",
241+
"performance",
242+
"slow",
243+
):
244+
assert required in registered, f"marker {required!r} is not registered"
245+
```
246+
Put it in `tests/unit/` — that is the only directory CI currently executes (#113).
247+
248+
**Both APIs were verified against pytest 8.4.2 on this repo**, so the snippet is known
249+
to work rather than assumed:
250+
251+
| state | `pytestconfig.inipath` | project markers in `getini("markers")` |
252+
|-|-|-|
253+
| today (broken) | `/Users/jmanning/clustrix/pytest.ini` | absent — only plugin markers |
254+
| after Step 2 | `/Users/jmanning/clustrix/pyproject.toml` | `real_world`, `slow`, `unit`, `integration` present |
255+
256+
So `test_pytest_reads_the_intended_config` fails today and passes after the fix — write
257+
it first and watch it fail, per the project's TDD rule.
258+
259+
### Step 6 — full check + docs
260+
```bash
261+
python scripts/pre_push_check.py # black, flake8, mypy, pytest — repeat until clean
262+
```
263+
Then update:
264+
- **#110** — correct the `addopts`/xdist premise; state which config was live.
265+
- **MIGRATION.md:80** — quotes a `[tool.pytest.ini_options]` block that no longer matches.
266+
- **`.claude/commands/testing/prime.md:95`** — says `config_file: pytest.ini`.
267+
- **#130** — close with before/after evidence.
268+
269+
---
270+
271+
## 6. Rollback
272+
273+
Each step is a separate commit, so `git revert` the offending one. Step 2 is the only
274+
destructive step (deleting `pytest.ini`); its content is reproduced verbatim in the issue
275+
body and recoverable via `git show master~N:pytest.ini`.
276+
277+
---
278+
279+
## 7. Definition of done
280+
281+
- [ ] `pytest --co 2>&1 | grep configfile` reports `pyproject.toml`
282+
- [ ] Exactly one pytest config file exists in the repo
283+
- [ ] Bare `pytest` runs without aborting
284+
- [ ] `pytest tests/integration/<anything>` is still refused (the #109 guarantee holds)
285+
- [ ] `pytest tests/ -m "not real_world"` collects no `tests/integration/` node IDs
286+
- [ ] All project markers registered; `--strict-markers` active; suite collects with 0 marker errors
287+
- [ ] `tests/unit/` still passes (≥70 tests)
288+
- [ ] Regression test added that fails if a config file shadows pyproject again
289+
- [ ] #110 corrected
290+
291+
---
292+
293+
## 8. Things that will trip you up
294+
295+
1. **`tomllib` needs Python ≥3.11.** The default interpreter here is 3.9. Use
296+
`/opt/homebrew/bin/python3.12` for the marker-gap script.
297+
2. **The repo has no usable venv by default**`import clustrix` fails everywhere
298+
because `paramiko` is missing (#110). Build a throwaway venv:
299+
`python3 -m venv /tmp/v && /tmp/v/bin/pip install -e ".[dev]" && /tmp/v/bin/pip install pytest-xdist pytest-timeout`
300+
3. **Pass `-o addopts=` when measuring**, so you compare like with like across steps.
301+
4. **CI's `black` is pinned to 25.1.0** (`pyproject.toml`). Do not "upgrade" it — an
302+
unbounded pin is what turned CI red before, and 26.5.1 reformats 19 unrelated files.
303+
5. **Do not enable `--strict-markers` before Step 3.** There are 4 undeclared markers in
304+
use; strict mode turns each into a hard collection error immediately.
305+
6. **`tests/real_world/conftest.py:223` calls `is_dartmouth_network()` at collection
306+
time** — live DNS plus a `ping` subprocess. If collection suddenly gets slow or hangs
307+
off-network, that is why. Tracked separately on #117; do not fix it here.

0 commit comments

Comments
 (0)