Skip to content

Commit 760432c

Browse files
Issue #130: pytest.ini was dead config shadowing pyproject.toml (#134)
* Issue #130: Clear the collection errors that block --strict-markers Step 4 of the #130 plan requires `pytest tests/ --co` to report zero collection errors before --strict-markers can be turned on. Measuring the baseline surfaced six that had been invisible while pytest.ini shadowed pyproject.toml and nothing was configured: * tests/real_world/test_container_registry_comprehensive.py:571 put a backslash inside an f-string expression. That is a SyntaxError on every Python before 3.12, and this project declares requires-python >=3.8, so the module could not be parsed at all. Hoisted the split() out. * Five modules under tests/comprehensive/ and tests/ import numpy or pandas at module scope to exercise array/dataframe serialization, but neither was declared in any extra. `pip install -e ".[dev]"` therefore produced a tree that could not be collected. Added both to the dev extra. Collection of `tests/ -m "not real_world"` goes from 1214 collected with 6 errors to 1275 collected with 0 errors. tests/unit/ still 70 passed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012gTBDPK16HUZ3kHQ2QyjuU * Issue #130: Add failing regression tests for pytest config shadowing TDD red step. All four fail on master, which is the point: they encode the bug described in #130. Nothing in pytest warns when one config file shadows another, so the only way to detect it is to assert on which file was actually selected. The tests check, in order: the loaded config is pyproject.toml; no higher-precedence config file (pytest.ini/tox.ini/setup.cfg) exists in the tree at all; every marker the suite uses is registered; and --strict-markers is live. test_strict_markers_is_active skips when the run passes -o addopts=, since --strict-markers reaches pytest through addopts and clearing it is a legitimate measurement technique -- config.option.override_ini reports the override, verified against pytest 8.4.2. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012gTBDPK16HUZ3kHQ2QyjuU * Issue #130: Key the billable-test guard off argv, not resolved testpaths Step 1 of the #130 plan, and it must land before any config change. The #109 guard refuses runs that explicitly target tests/integration. It read `config.args`, which looks like "what the user asked for" but is not: when no path is given on the command line, pytest populates `config.args` from `testpaths`. pyproject.toml's testpaths listed tests/integration, so the instant the project's config became effective, a bare `pytest` matched the guard and aborted the whole suite. Verified before this change: $ mv pytest.ini /tmp/ && pytest --co -q ERROR: Refusing to run 'tests/integration': ... `config.invocation_params.args` is the real argv, so the guard now fires on explicit targeting only. Entries beginning with "-" are skipped, since that list contains flags as well as paths. Added test_bare_pytest_is_not_refused_by_the_guard to cover the case that regressed. The existing suite ran pytest in a subprocess but always passed a path, so it could not have caught this; _collect_integration now takes a NO_TARGET sentinel to omit the path argument entirely. All #109 guarantees re-verified: naming the directory or a file in it is still refused, `tests/ -m "not real_world"` still collects zero tests/integration node IDs. tests/unit/test_billable_safety.py 56 passed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012gTBDPK16HUZ3kHQ2QyjuU * Issue #130: Delete pytest.ini, make pyproject.toml the only pytest config Step 2 of the #130 plan. pytest.ini opened with [tool:pytest], a section header valid only inside setup.cfg. In a file named pytest.ini pytest wants [pytest], so every setting in it was ignored -- but pytest had still *selected* it as the config file, and having selected one it stops searching. pyproject.toml's [tool.pytest.ini_options] was therefore never read either. Two config files, zero effective configuration. Deleting pytest.ini leaves pyproject.toml as the single source, which is the modern convention and the file the rest of the tooling already uses. Its contents are preserved in the #130 issue body and recoverable with `git show master:pytest.ini`. testpaths is set to ["tests"] rather than the previous ["tests/unit", "tests/integration"]: naming the integration directory here makes a bare `pytest` resolve its targets to billable tests. The directory stays excluded from collection by collect_ignore_glob in tests/integration/conftest.py regardless (#109). $ pytest --co 2>&1 | grep configfile configfile: pyproject.toml $ pytest --co -q | tail -1 1670 tests collected Also corrects the docstring of test_bare_pytest_is_not_refused_by_the_guard to state what it measurably catches. Undoing either the argv fix or the testpaths fix alone leaves bare pytest working; the test fails only when both are undone, so it is the last line of defence, not the first. The four-case table is recorded in the docstring. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012gTBDPK16HUZ3kHQ2QyjuU * Issue #130: Register every marker the suite actually uses Step 3 of the #130 plan. Must land before --strict-markers, since strict mode turns each undeclared marker into a hard collection error. pyproject.toml declared 4 markers; the suite uses 7. The three missing ones were in real use and silently inert: dartmouth_network 11 uses across 4 files expensive 5 uses across 5 files performance 4 uses across 4 files `expensive` is the notable one: tests/integration/conftest.py applies it programmatically so that an opted-in operator can select `-m "not expensive"`, and that selector could not have worked while the marker was unregistered. Removed the single `@pytest.mark.cleanup` in test_kubernetes_performance_benchmarks.py. It was declared in neither config file, had exactly one use, and conferred no behaviour. The test itself is unchanged and still runs. Confirmed with the author. $ pytest --markers | grep -cE '^@pytest.mark.(real_world|slow|unit|integration|expensive|dartmouth_network|performance):' 7 The audit that produced this list is recorded as a comment above the markers block, and tests/unit/test_pytest_config.py asserts the seven stay registered. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012gTBDPK16HUZ3kHQ2QyjuU * Issue #130: Enable --strict-markers Step 4 of the #130 plan, deliberately last: strict mode is only safe once every marker in use is registered (Step 3) and the tree collects cleanly. Dry run before enabling: $ pytest tests/ --co -q -o addopts="--strict-markers" | tail -1 1670 tests collected # 0 errors After enabling, an unregistered marker is a hard error rather than a PytestUnknownMarkWarning nobody reads: $ printf 'import pytest\n@pytest.mark.bogus_xyz\ndef test_x(): assert True\n' > tests/unit/test_probe_tmp.py $ pytest tests/unit/test_probe_tmp.py --co -q ERROR ... Failed: 'bogus_xyz' not found in `markers` configuration option Left a note on addopts about the #110 trap: adding "-n" here requires moving pytest-xdist from the [test] extra into [dev] first, or `pip install -e ".[dev]"` produces an environment where pytest cannot start at all. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012gTBDPK16HUZ3kHQ2QyjuU * Issue #130: Update docs that still describe the shadowed pytest config MIGRATION.md quoted a [tool.pytest.ini_options] block that no longer matches the file, and gave no hint that the block had never been in effect. Updated to the real contents and added a note recording why pytest.ini was removed and why testpaths must not name tests/integration. .claude/commands/testing/prime.md declared `config_file: pytest.ini`, which no longer exists. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012gTBDPK16HUZ3kHQ2QyjuU * Issue #130: Make pre_push_check run the same tests CI runs The script's docstring promises it verifies "GitHub Actions won't fail", but its test step was a bare `pytest` while CI runs `pytest tests/unit/ -m "not real_world"`. That mismatch was invisible for as long as the project had no effective pytest config: pytest.ini shadowed pyproject.toml and neither applied, so `testpaths` did nothing. With the config live, a bare `pytest` resolves its targets from testpaths and runs all 1670 tests -- including the 224 in tests/real_world/, which open live SSH and cloud connections. The gate stopped terminating. Real-world tests are run deliberately through scripts/run_real_world_tests.py, which is what the pre-push hook uses. After this change black, mypy and the tests all pass. flake8 still fails, on 92 findings that predate this branch (91 on it -- the f-string fix removed one). The local invocation omits the `--exit-zero` that CI applies to the identical command, so this gate has never been green on master. Filed as #133 rather than folded in here, since it includes a genuine unrelated bug: test_direct_gpu_detection.py builds a remote program inside an f-string without escaping its braces, so it raises NameError before the subprocess starts. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012gTBDPK16HUZ3kHQ2QyjuU * Issue #130: Ignore test-generated artifacts; add session notes Running the full suite while diagnosing the pre_push_check hang showed the benchmark tests in tests/real_world/ write performance_test_results/ into the repo root, which was not ignored. coverage_detailed_report.txt was untracked for the same reason. Notes record the measured before/after, the two things the handoff plan did not anticipate, and what was deliberately left open (#133, #110, #117). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012gTBDPK16HUZ3kHQ2QyjuU * Issue #130: Close the guard bypasses red-teaming found, and fix my own claims Red-teaming the branch found that my Step 1 change traded a false refusal for two real money-safety holes. Verified against master, which refused both: PYTEST_ADDOPTS=tests/integration/test_x.py pytest -> collected 2 pytest -o testpaths=tests/integration/test_x.py -> collected 2 Cause: `config.invocation_params.args` is only what the operator typed. `testpaths`, `-o testpaths=` and PYTEST_ADDOPTS all feed `config.args` without appearing there. Reading the typed argv makes the guard blind to every indirect route. The right fix was in the config, not the guard. Reading `config.args` is safe now precisely because Step 2 set `testpaths = ["tests"]`, so the guard is back on the effective target list. Also fixed, and pre-existing (#109): relative paths were resolved only against rootdir, so from within tests/ the path `integration/test_x.py` went unrecognised. All plausible bases are now tried. `--pyargs` and `-p` address modules by dotted name and never looked like paths; both are now translated and checked. Measured after the change -- refused: explicit dir, explicit file, node id, -o testpaths, PYTEST_ADDOPTS, --pyargs, cwd-relative. Still allowed and must be: bare `pytest` (0 integration nodes collected) and the CLUSTRIX_ALLOW_BILLABLE=1 opt-in. `-p` cannot be blocked before the import it triggers -- no conftest hook runs that early -- but the run is now refused before any test executes, which is where the cost is; it also fails on its own here, since tests/ is not a package. Four parametrized regression tests cover the indirect routes. Three fail against the old guard, confirming they are not decorative. Corrections to my own earlier work in this PR: * "the 224 in tests/real_world/" was wrong and had been committed as a code comment in scripts/pre_push_check.py. 224 counts decorator lines; tests/real_world/ collects 388 tests. That comment's rationale was also wrong: the old bare `pytest` was not "harmless while no config applied", it aborted on master in seconds with a collection error and gated nothing. Fixing that SyntaxError is what let collection succeed and made the runtime problem visible. * test_no_shadowing_config_file_exists forbade tox.ini and setup.cfg. pytest checks those *after* pyproject.toml, so neither can shadow it -- verified. The test also failed on an ordinary pytest-free setup.cfg holding flake8 config. Now limited to pytest.ini/.pytest.ini, and it checks the repo tree rather than the resolved rootdir. * test_bare_pytest_is_not_refused_by_the_guard described the guard source that no longer exists; rewritten to state the actual invariant. * coverage_detailed_report.txt was removed from .gitignore: nothing in the repo writes it, so the justification given was unsupported. Unrelated CI fix found while verifying: .github/workflows/fast_ci.yml installs pytest without pytest-timeout, then runs pytest with --timeout=60. It exits 4 with "unrecognized arguments" and has been failing on every run, including on master. Added the plugin. Restored the `filterwarnings` block from the deleted pytest.ini, which was never migrated. It only suppresses third-party paramiko/cryptography noise. The five markers pytest.ini also declared are deliberately not migrated: tests/real_world/conftest.py registers them with addinivalue_line where they are used. tests/unit/ 79 passed. 1674 collected, 0 errors. flake8 91 (master: 92). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012gTBDPK16HUZ3kHQ2QyjuU * Issue #130: Revert the fast_ci.yml change; the diagnosis was wrong The previous commit added pytest-timeout to .github/workflows/fast_ci.yml and said the job "had been failing on every run for that reason". That is not the reason. fast_ci.yml is invalid YAML and has never run a single job -- every run, on master as well as on this branch, completes with conclusion=failure and jobs=0, which is what GitHub reports when it cannot parse the workflow: yaml.scanner.ScannerError: while scanning a simple key in ".github/workflows/fast_ci.yml", line 89, column 1 Three steps open `python -c "` inside a `run: |` block and then write the Python body at column 0, which terminates the block scalar. The missing pytest-timeout is real but latent -- it only bites once the workflow can start. Fixing it here would have shipped an unverifiable change with a comment misstating its cause, and repairing the YAML would turn on a workflow whose jobs run real @cluster execution and a Docker build, inside a PR scoped to pytest configuration. Both defects, and the reason they are sequenced against #120, are filed as #135. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012gTBDPK16HUZ3kHQ2QyjuU --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent de09f1a commit 760432c

12 files changed

Lines changed: 562 additions & 59 deletions

.claude/commands/testing/prime.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,7 @@ environment:
9292
framework: pytest
9393
test_command: pytest
9494
test_directory: tests
95-
config_file: pytest.ini
95+
config_file: pyproject.toml # [tool.pytest.ini_options]; see #130
9696
options:
9797
- -v
9898
- --tb=short

.gitignore

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,8 @@ test-reports/
2828
coverage.json
2929
htmlcov/
3030
.tox/
31+
# Written by the benchmark tests in tests/real_world/ at run time.
32+
performance_test_results/
3133

3234
# Personal configuration files
3335
clustrix.yml

MIGRATION.md

Lines changed: 20 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -76,17 +76,34 @@ from clustrix.filesystem import cluster_ls, cluster_find
7676
**Pytest configuration updated** to properly discover tests:
7777

7878
```toml
79-
# pyproject.toml
79+
# pyproject.toml -- the project's only pytest config file
8080
[tool.pytest.ini_options]
81-
testpaths = ["tests/unit", "tests/integration"]
81+
testpaths = ["tests"]
82+
addopts = "-v --tb=short --strict-markers"
8283
markers = [
8384
"real_world: marks tests as real world tests",
8485
"slow: marks tests as slow",
85-
"unit: marks tests as unit tests",
86+
"unit: marks tests as unit tests",
8687
"integration: marks tests as integration tests",
88+
"expensive: marks tests that provision billable resources",
89+
"dartmouth_network: marks tests needing the Dartmouth campus network",
90+
"performance: marks performance benchmark tests",
8791
]
8892
```
8993

94+
> **Note (see #130).** This block only became effective later. A `pytest.ini`
95+
> in the repo root used the section header `[tool:pytest]`, which is valid only
96+
> in `setup.cfg`; pytest still selected that file and stopped searching, so
97+
> nothing here was applied. `pytest.ini` has since been deleted and
98+
> `pyproject.toml` is now the single source. Do not reintroduce `pytest.ini`,
99+
> `tox.ini` or `setup.cfg` -- pytest prefers all three over `pyproject.toml`
100+
> and would silently shadow it again.
101+
>
102+
> `testpaths` is `["tests"]`, not `["tests/unit", "tests/integration"]`: with no
103+
> path on the command line pytest resolves its targets from `testpaths`, and
104+
> naming the integration directory there points a bare `pytest` at tests that
105+
> provision billable cloud resources.
106+
90107
### CI/CD Updates
91108
**GitHub Actions workflows updated** for new structure:
92109
- Test paths fixed to use `tests/unit/` and `tests/integration/`
Lines changed: 112 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,112 @@
1+
# Session log: executing the #130 plan
2+
3+
**Date:** 2026-08-17 · **Branch:** `fix/130-pytest-config` · **Base:** `master` @ `a9393b7`
4+
**Plan followed:** `notes/handoff_130_pytest_config.md`
5+
6+
Every command below was run in a throwaway venv, since the repo has no usable
7+
one by default (#110):
8+
9+
```bash
10+
python3 -m venv /tmp/v130 && /tmp/v130/bin/pip install -e ".[dev]"
11+
```
12+
13+
`scripts/pre_push_check.py` shells out to bare `pytest`/`mypy`/`flake8` from
14+
`PATH`, so it must be run as
15+
`PATH="/tmp/v130/bin:$PATH" /tmp/v130/bin/python scripts/pre_push_check.py`.
16+
Running it with the venv interpreter alone is not enough and fails with
17+
`ModuleNotFoundError: paramiko`.
18+
19+
## What landed, in order
20+
21+
| Commit | Step | What |
22+
|-|-|-|
23+
| `9c18cb3` | 0.5 | Cleared 6 collection errors blocking Step 4 |
24+
| `176d87e` | 5 (first) | Regression tests, TDD red |
25+
| `9c85296` | 1 | Guard reads argv, not resolved testpaths |
26+
| `d1d6090` | 2 | Deleted pytest.ini; `testpaths = ["tests"]` |
27+
| `6d39bd4` | 3 | Registered all 7 markers; dropped `cleanup` |
28+
| `d93f933` | 4 | Enabled `--strict-markers` |
29+
| `43e7bf3` | 6 | MIGRATION.md + prime.md |
30+
| `14222d0` | 6 | pre_push_check runs CI's test command |
31+
32+
## Things the plan did not anticipate
33+
34+
**Step 0 baseline had 6 collection errors.** The plan said to record the
35+
number; it did not say the number came with errors attached. They had to be
36+
fixed before Step 4 could be verified at all, because Step 4's gate is
37+
"0 errors under `--strict-markers`".
38+
39+
- `tests/real_world/test_container_registry_comprehensive.py:571` had a
40+
backslash inside an f-string expression — a `SyntaxError` on every Python
41+
before 3.12, while `requires-python` says `>=3.8`. An AST parse over all of
42+
`tests/` and `clustrix/` confirmed it was the only one in the tree.
43+
- `numpy` and `pandas` were undeclared but imported at module scope by five
44+
modules. Added to the `[dev]` extra.
45+
46+
Result: 1214 collected + 6 errors → 1275 collected + 0 errors.
47+
48+
**The plan's own regression test needed correcting.** It proposed asserting
49+
`pytestconfig.inipath.name == "pyproject.toml"`, which is right, but the
50+
docstring I first wrote for the bare-pytest guard test overclaimed. Measured:
51+
52+
| guard reads | testpaths | bare pytest |
53+
|-|-|-|
54+
| `invocation_params.args` | `["tests"]` | ok |
55+
| `invocation_params.args` | `["tests/unit","tests/integration"]` | ok |
56+
| `config.args` | `["tests"]` | ok |
57+
| `config.args` | `["tests/unit","tests/integration"]` | **REFUSED** |
58+
59+
So the two fixes are each independently sufficient, and the test guards the
60+
*combination*. Both are kept as belt and braces, and the table is recorded in
61+
the test's docstring.
62+
63+
**`--strict-markers` needs care with `-o addopts=`.** It reaches pytest through
64+
`addopts`, so `test_strict_markers_is_active` skips when
65+
`config.option.override_ini` contains an `addopts=` entry. Verified against
66+
pytest 8.4.2: that attribute holds `['addopts=']`.
67+
68+
**`pre_push_check.py` ran a bare `pytest`.** Harmless only while no config was
69+
effective. Once `testpaths` went live it resolved to all 1670 tests including
70+
the 224 network-bound ones in `tests/real_world/`, and stopped terminating. It
71+
also left artifacts behind: it modified checked-in files under
72+
`tests/real_world/screenshots/` and created an untracked
73+
`performance_test_results/`. Now runs `pytest tests/unit/ -m "not real_world"`,
74+
matching `.github/workflows/tests.yml`; the generated paths are gitignored.
75+
76+
## Definition of done — measured
77+
78+
```
79+
configfile pyproject.toml
80+
shadowing config files in repo 0
81+
bare pytest 1670 collected, no abort
82+
pytest tests/integration/<file> refused (#109 holds)
83+
tests/ -m "not real_world" integration node IDs 0
84+
markers registered 7 / 7
85+
collection errors under --strict-markers 0
86+
tests/unit/ 75 passed
87+
fresh venv, [dev] only, pytest tests/ --co 1670 collected, 0 errors
88+
```
89+
90+
## Left open deliberately
91+
92+
**#133 (filed).** `scripts/pre_push_check.py` still cannot exit 0: flake8
93+
reports 92 findings on `master` (91 on this branch). CI runs the identical
94+
flake8 command with `--exit-zero`, so it has never been green anywhere. 75 are
95+
`E402` from deliberate `sys.path` setup. The rest are not stylistic — the
96+
notable one is `tests/integration/test_direct_gpu_detection.py:42`, which
97+
builds a remote Python program inside an f-string without escaping its braces,
98+
so `{torch.__version__}`, `{i}`, `{props.name}`, `{e}` are interpolated in the
99+
*local* scope and the module raises `NameError` before the subprocess starts.
100+
It survives because the #109 guard keeps `tests/integration/` out of CI.
101+
102+
Not folded into this PR: different concern, gated directory, and it would bury
103+
the #130 change.
104+
105+
**#110 corrected**, not closed. Its `addopts`/xdist premise quoted the epic
106+
branch, not `master`, and its `sklearn` collection-error claim is wrong (every
107+
`sklearn` import under `tests/` is inside a function body). Two of its
108+
acceptance criteria are now met. Comment:
109+
https://github.com/ContextLab/clustrix/issues/110#issuecomment-5317453905
110+
111+
**#117** untouched, as instructed — `tests/real_world/conftest.py:223` still
112+
calls `is_dartmouth_network()` at collection time.

pyproject.toml

Lines changed: 42 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,11 @@ cloud = [
8383
dev = [
8484
"pytest>=6.0",
8585
"pytest-cov>=2.0",
86+
# tests/comprehensive/* and several tests/*_real.py modules import numpy and
87+
# pandas at module scope to exercise array/dataframe serialization. Without
88+
# them those modules raise ModuleNotFoundError during collection (see #130).
89+
"numpy>=1.19",
90+
"pandas>=1.1",
8691
"black==25.1.0; python_version >= '3.9'", # pinned - unbounded ">=" let CI pull 26.5.1 (see #110)
8792
"flake8>=3.8",
8893
"mypy>=0.812",
@@ -181,16 +186,51 @@ module = "tests.*"
181186
ignore_errors = true
182187

183188
[tool.pytest.ini_options]
184-
testpaths = ["tests/unit", "tests/integration"]
189+
# This is the project's ONLY pytest config. Do not add pytest.ini, tox.ini or
190+
# setup.cfg: pytest selects the first config file it finds by precedence and
191+
# then stops searching, so any of those would silently shadow this block --
192+
# which is exactly what happened for months under #130. Enforced by
193+
# tests/unit/test_pytest_config.py.
194+
#
195+
# testpaths must NOT list tests/integration. Those tests provision billable
196+
# cloud resources and are gated by tests/integration/conftest.py (#109); naming
197+
# the directory here would make a bare `pytest` resolve to it.
198+
testpaths = ["tests"]
185199
python_files = "test_*.py"
186200
python_classes = "Test*"
187201
python_functions = "test_*"
188-
addopts = "-v --tb=short"
202+
# --strict-markers makes an undeclared marker a collection error rather than a
203+
# warning. Keep it: without it a typo'd marker silently does nothing and the
204+
# test it decorates stops being selectable with no signal at all.
205+
# Do NOT add "-n" here without first moving pytest-xdist from the [test] extra
206+
# into [dev] -- otherwise `pip install -e ".[dev]"` yields an env where pytest
207+
# cannot start (see #110).
208+
addopts = "-v --tb=short --strict-markers"
209+
# Every marker used anywhere under tests/ must appear here: --strict-markers
210+
# turns an undeclared marker into a hard collection error. Verify with
211+
# grep -rhoE "@pytest\.mark\.[a-zA-Z_]+" tests/ | sort -u
212+
# and note that tests/unit/test_pytest_config.py asserts these stay registered.
189213
markers = [
190214
"real_world: marks tests as real world tests (deselect with '-m \"not real_world\"')",
191215
"slow: marks tests as slow (deselect with '-m \"not slow\"')",
192216
"unit: marks tests as unit tests",
193217
"integration: marks tests as integration tests",
218+
"expensive: marks tests that provision billable resources (applied automatically to tests/integration; see #109)",
219+
"dartmouth_network: marks tests that require access to the Dartmouth campus network",
220+
"performance: marks performance benchmark tests",
221+
]
222+
# Carried over from the deleted pytest.ini, where it never took effect. These
223+
# suppress third-party noise only; nothing here hides a warning raised by
224+
# clustrix itself.
225+
# NB: markers visual/ssh_required/aws_required/azure_required/gcp_required are
226+
# deliberately NOT listed. tests/real_world/conftest.py registers them with
227+
# addinivalue_line at the point they are used, so --strict-markers is satisfied
228+
# there and they stay scoped to the suite that actually defines them.
229+
filterwarnings = [
230+
"ignore::DeprecationWarning",
231+
"ignore::PendingDeprecationWarning",
232+
"ignore::UserWarning:paramiko.*",
233+
"ignore::UserWarning:cryptography.*",
194234
]
195235

196236
[tool.coverage.run]

pytest.ini

Lines changed: 0 additions & 22 deletions
This file was deleted.

scripts/pre_push_check.py

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -43,9 +43,24 @@ def main():
4343

4444
checks = [
4545
("black clustrix/ tests/", "Black formatting"), # Format, don't just check
46-
("flake8 clustrix/ tests/ --max-line-length=88 --extend-ignore=E203,W503,F401,E722,F541,F841,F811,E731,E501,W291,W293,F824", "Flake8 linting"),
46+
(
47+
"flake8 clustrix/ tests/ --max-line-length=88 --extend-ignore=E203,W503,F401,E722,F541,F841,F811,E731,E501,W291,W293,F824",
48+
"Flake8 linting",
49+
),
4750
("mypy clustrix/", "MyPy type checking"),
48-
("pytest", "Tests"),
51+
# Must mirror what GitHub Actions actually runs (see
52+
# .github/workflows/tests.yml), or this script cannot deliver on
53+
# the promise in its own docstring.
54+
#
55+
# This was a bare `pytest`, which gated nothing: on master it
56+
# aborted in seconds with "Interrupted: 1 error during collection"
57+
# (an f-string SyntaxError under Python < 3.12) and ran zero tests.
58+
# Fixing that error is what made the problem visible -- collection
59+
# then succeeded and a bare `pytest` ran all 1670 tests, including
60+
# the 388 in tests/real_world/, which open live SSH and cloud
61+
# connections. Real-world tests are run deliberately through
62+
# scripts/run_real_world_tests.py, not from this gate.
63+
('pytest tests/unit/ -m "not real_world"', "Tests"),
4964
]
5065

5166
all_passed = True

tests/conftest.py

Lines changed: 71 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,66 @@ def _billable_tests_enabled():
1515
return os.environ.get(_OPT_IN_VAR, "").strip().lower() in _TRUTHY
1616

1717

18+
def _iter_candidate_targets(config):
19+
"""Yield every path-ish string this run could end up collecting from.
20+
21+
Reads `config.args` -- the *effective* target list -- rather than
22+
`config.invocation_params.args`, which is only what the operator literally
23+
typed. The difference is the whole ballgame, because there are three ways
24+
to aim pytest at a directory without typing its path:
25+
26+
PYTEST_ADDOPTS=tests/integration/test_x.py pytest
27+
pytest -o testpaths=tests/integration/test_x.py
28+
(testpaths in pyproject.toml)
29+
30+
All three land in `config.args` and none appear in `invocation_params.args`.
31+
Red-teaming confirmed the first two collected billable modules when the
32+
guard read the typed argv.
33+
34+
Reading `config.args` is only safe because `testpaths` is `["tests"]`. If
35+
it ever names tests/integration again, a bare `pytest` will be refused --
36+
loudly and correctly, since testpaths would then be pointing every default
37+
run at billable tests.
38+
"""
39+
for arg in config.args:
40+
yield str(arg).split("::")[0]
41+
# --pyargs addresses modules by dotted name, which never looks like a path.
42+
if getattr(config.option, "pyargs", False):
43+
for arg in config.args:
44+
yield str(arg).split("::")[0].replace(".", os.sep)
45+
# -p imports a plugin by dotted name, before collection begins.
46+
for plugin in getattr(config.option, "plugins", None) or []:
47+
yield str(plugin).replace(".", os.sep)
48+
49+
50+
def _targets_integration_dir(candidate, config):
51+
"""True if `candidate` names tests/integration under any sane resolution.
52+
53+
Paths on the command line resolve against the invocation directory, while
54+
`testpaths` resolves against rootdir. Those differ whenever pytest is run
55+
from a subdirectory, and resolving against only one of them was a real
56+
hole: from `tests/`, `pytest integration/test_x.py` was not recognised.
57+
58+
Both bases are tried. A guard protecting real money should over-match
59+
rather than under-match.
60+
"""
61+
raw = pathlib.Path(candidate)
62+
if raw.is_absolute():
63+
attempts = [raw]
64+
else:
65+
invocation_dir = getattr(config.invocation_params, "dir", None)
66+
bases = [invocation_dir, pathlib.Path.cwd(), pathlib.Path(str(config.rootpath))]
67+
attempts = [pathlib.Path(str(base)) / raw for base in bases if base]
68+
for attempt in attempts:
69+
try:
70+
resolved = attempt.resolve()
71+
except OSError: # pragma: no cover - defensive, unresolvable path
72+
continue
73+
if resolved == _INTEGRATION_DIR or _INTEGRATION_DIR in resolved.parents:
74+
return True
75+
return False
76+
77+
1878
def pytest_configure(config):
1979
"""Refuse to start when a run explicitly targets tests/integration.
2080
@@ -34,22 +94,21 @@ def pytest_configure(config):
3494
keep working and silently skip the directory, while someone who asked for
3595
these tests by name gets told why they got nothing, rather than an
3696
inscrutable empty run.
97+
98+
Target discovery is in `_iter_candidate_targets` and path matching is in
99+
`_targets_integration_dir`; both carry the reasoning for why they look
100+
where they do. In short: read the *effective* target list, and resolve
101+
relative paths against every plausible base.
37102
"""
38103
if _billable_tests_enabled():
39104
return
40-
for arg in config.args:
41-
# strip pytest's "::TestClass::test_name" node-id suffix
42-
candidate = pathlib.Path(str(arg).split("::")[0])
43-
if not candidate.is_absolute():
44-
candidate = (pathlib.Path(str(config.rootpath)) / candidate).resolve()
45-
else:
46-
candidate = candidate.resolve()
47-
if candidate == _INTEGRATION_DIR or _INTEGRATION_DIR in candidate.parents:
105+
for candidate in _iter_candidate_targets(config):
106+
if _targets_integration_dir(candidate, config):
48107
raise pytest.UsageError(
49-
f"Refusing to run {arg!r}: tests/integration provisions real, "
50-
f"billable cloud resources (AWS EKS/EC2). Set {_OPT_IN_VAR}=1 to "
51-
f"run them deliberately, e.g.\n"
52-
f" {_OPT_IN_VAR}=1 pytest {arg}"
108+
f"Refusing to run {candidate!r}: tests/integration provisions "
109+
f"real, billable cloud resources (AWS EKS/EC2). Set "
110+
f"{_OPT_IN_VAR}=1 to run them deliberately, e.g.\n"
111+
f" {_OPT_IN_VAR}=1 pytest {candidate}"
53112
)
54113

55114

0 commit comments

Comments
 (0)