Issue #130: pytest.ini was dead config shadowing pyproject.toml

.claude/commands/testing/prime.md

@@ -92,7 +92,7 @@ environment:
 framework: pytest
 test_command: pytest
 test_directory: tests
-config_file: pytest.ini
+config_file: pyproject.toml   # [tool.pytest.ini_options]; see #130
 options:
   - -v
   - --tb=short

.gitignore

@@ -28,6 +28,8 @@ test-reports/
 coverage.json
 htmlcov/
 .tox/
+# Written by the benchmark tests in tests/real_world/ at run time.
+performance_test_results/
 
 # Personal configuration files
 clustrix.yml

MIGRATION.md

@@ -76,17 +76,34 @@ from clustrix.filesystem import cluster_ls, cluster_find
 **Pytest configuration updated** to properly discover tests:
 
 ```toml
-# pyproject.toml
+# pyproject.toml -- the project's only pytest config file
 [tool.pytest.ini_options]
-testpaths = ["tests/unit", "tests/integration"]
+testpaths = ["tests"]
+addopts = "-v --tb=short --strict-markers"
 markers = [
     "real_world: marks tests as real world tests",
     "slow: marks tests as slow",
-    "unit: marks tests as unit tests", 
+    "unit: marks tests as unit tests",
     "integration: marks tests as integration tests",
+    "expensive: marks tests that provision billable resources",
+    "dartmouth_network: marks tests needing the Dartmouth campus network",
+    "performance: marks performance benchmark tests",
 ]
 ```
 
+> **Note (see #130).** This block only became effective later. A `pytest.ini`
+> in the repo root used the section header `[tool:pytest]`, which is valid only
+> in `setup.cfg`; pytest still selected that file and stopped searching, so
+> nothing here was applied. `pytest.ini` has since been deleted and
+> `pyproject.toml` is now the single source. Do not reintroduce `pytest.ini`,
+> `tox.ini` or `setup.cfg` -- pytest prefers all three over `pyproject.toml`
+> and would silently shadow it again.
+>
+> `testpaths` is `["tests"]`, not `["tests/unit", "tests/integration"]`: with no
+> path on the command line pytest resolves its targets from `testpaths`, and
+> naming the integration directory there points a bare `pytest` at tests that
+> provision billable cloud resources.
+
 ### CI/CD Updates
 **GitHub Actions workflows updated** for new structure:
 - Test paths fixed to use `tests/unit/` and `tests/integration/`

notes/session_130_pytest_config_execution.md

@@ -0,0 +1,112 @@
+# Session log: executing the #130 plan
+
+**Date:** 2026-08-17 · **Branch:** `fix/130-pytest-config` · **Base:** `master` @ `a9393b7`
+**Plan followed:** `notes/handoff_130_pytest_config.md`
+
+Every command below was run in a throwaway venv, since the repo has no usable
+one by default (#110):
+
+```bash
+python3 -m venv /tmp/v130 && /tmp/v130/bin/pip install -e ".[dev]"
+```
+
+`scripts/pre_push_check.py` shells out to bare `pytest`/`mypy`/`flake8` from
+`PATH`, so it must be run as
+`PATH="/tmp/v130/bin:$PATH" /tmp/v130/bin/python scripts/pre_push_check.py`.
+Running it with the venv interpreter alone is not enough and fails with
+`ModuleNotFoundError: paramiko`.
+
+## What landed, in order
+
+| Commit | Step | What |
+|-|-|-|
+| `9c18cb3` | 0.5 | Cleared 6 collection errors blocking Step 4 |
+| `176d87e` | 5 (first) | Regression tests, TDD red |
+| `9c85296` | 1 | Guard reads argv, not resolved testpaths |
+| `d1d6090` | 2 | Deleted pytest.ini; `testpaths = ["tests"]` |
+| `6d39bd4` | 3 | Registered all 7 markers; dropped `cleanup` |
+| `d93f933` | 4 | Enabled `--strict-markers` |
+| `43e7bf3` | 6 | MIGRATION.md + prime.md |
+| `14222d0` | 6 | pre_push_check runs CI's test command |
+
+## Things the plan did not anticipate
+
+**Step 0 baseline had 6 collection errors.** The plan said to record the
+number; it did not say the number came with errors attached. They had to be
+fixed before Step 4 could be verified at all, because Step 4's gate is
+"0 errors under `--strict-markers`".
+
+- `tests/real_world/test_container_registry_comprehensive.py:571` had a
+  backslash inside an f-string expression — a `SyntaxError` on every Python
+  before 3.12, while `requires-python` says `>=3.8`. An AST parse over all of
+  `tests/` and `clustrix/` confirmed it was the only one in the tree.
+- `numpy` and `pandas` were undeclared but imported at module scope by five
+  modules. Added to the `[dev]` extra.
+
+Result: 1214 collected + 6 errors → 1275 collected + 0 errors.
+
+**The plan's own regression test needed correcting.** It proposed asserting
+`pytestconfig.inipath.name == "pyproject.toml"`, which is right, but the
+docstring I first wrote for the bare-pytest guard test overclaimed. Measured:
+
+| guard reads | testpaths | bare pytest |
+|-|-|-|
+| `invocation_params.args` | `["tests"]` | ok |
+| `invocation_params.args` | `["tests/unit","tests/integration"]` | ok |
+| `config.args` | `["tests"]` | ok |
+| `config.args` | `["tests/unit","tests/integration"]` | **REFUSED** |
+
+So the two fixes are each independently sufficient, and the test guards the
+*combination*. Both are kept as belt and braces, and the table is recorded in
+the test's docstring.
+
+**`--strict-markers` needs care with `-o addopts=`.** It reaches pytest through
+`addopts`, so `test_strict_markers_is_active` skips when
+`config.option.override_ini` contains an `addopts=` entry. Verified against
+pytest 8.4.2: that attribute holds `['addopts=']`.
+
+**`pre_push_check.py` ran a bare `pytest`.** Harmless only while no config was
+effective. Once `testpaths` went live it resolved to all 1670 tests including
+the 224 network-bound ones in `tests/real_world/`, and stopped terminating. It
+also left artifacts behind: it modified checked-in files under
+`tests/real_world/screenshots/` and created an untracked
+`performance_test_results/`. Now runs `pytest tests/unit/ -m "not real_world"`,
+matching `.github/workflows/tests.yml`; the generated paths are gitignored.
+
+## Definition of done — measured
+
+```
+configfile                                       pyproject.toml
+shadowing config files in repo                   0
+bare pytest                                      1670 collected, no abort
+pytest tests/integration/<file>                  refused (#109 holds)
+tests/ -m "not real_world" integration node IDs  0
+markers registered                               7 / 7
+collection errors under --strict-markers         0
+tests/unit/                                      75 passed
+fresh venv, [dev] only, pytest tests/ --co       1670 collected, 0 errors
+```
+
+## Left open deliberately
+
+**#133 (filed).** `scripts/pre_push_check.py` still cannot exit 0: flake8
+reports 92 findings on `master` (91 on this branch). CI runs the identical
+flake8 command with `--exit-zero`, so it has never been green anywhere. 75 are
+`E402` from deliberate `sys.path` setup. The rest are not stylistic — the
+notable one is `tests/integration/test_direct_gpu_detection.py:42`, which
+builds a remote Python program inside an f-string without escaping its braces,
+so `{torch.__version__}`, `{i}`, `{props.name}`, `{e}` are interpolated in the
+*local* scope and the module raises `NameError` before the subprocess starts.
+It survives because the #109 guard keeps `tests/integration/` out of CI.
+
+Not folded into this PR: different concern, gated directory, and it would bury
+the #130 change.
+
+**#110 corrected**, not closed. Its `addopts`/xdist premise quoted the epic
+branch, not `master`, and its `sklearn` collection-error claim is wrong (every
+`sklearn` import under `tests/` is inside a function body). Two of its
+acceptance criteria are now met. Comment:
+https://github.com/ContextLab/clustrix/issues/110#issuecomment-5317453905
+
+**#117** untouched, as instructed — `tests/real_world/conftest.py:223` still
+calls `is_dartmouth_network()` at collection time.

pyproject.toml

@@ -83,6 +83,11 @@ cloud = [
 dev = [
     "pytest>=6.0",
     "pytest-cov>=2.0",
+    # tests/comprehensive/* and several tests/*_real.py modules import numpy and
+    # pandas at module scope to exercise array/dataframe serialization. Without
+    # them those modules raise ModuleNotFoundError during collection (see #130).
+    "numpy>=1.19",
+    "pandas>=1.1",
     "black==25.1.0; python_version >= '3.9'",  # pinned - unbounded ">=" let CI pull 26.5.1 (see #110)
     "flake8>=3.8", 
     "mypy>=0.812",
@@ -181,16 +186,51 @@ module = "tests.*"
 ignore_errors = true
 
 [tool.pytest.ini_options]
-testpaths = ["tests/unit", "tests/integration"]
+# This is the project's ONLY pytest config. Do not add pytest.ini, tox.ini or
+# setup.cfg: pytest selects the first config file it finds by precedence and
+# then stops searching, so any of those would silently shadow this block --
+# which is exactly what happened for months under #130. Enforced by
+# tests/unit/test_pytest_config.py.
+#
+# testpaths must NOT list tests/integration. Those tests provision billable
+# cloud resources and are gated by tests/integration/conftest.py (#109); naming
+# the directory here would make a bare `pytest` resolve to it.
+testpaths = ["tests"]
 python_files = "test_*.py"
 python_classes = "Test*" 
 python_functions = "test_*"
-addopts = "-v --tb=short"
+# --strict-markers makes an undeclared marker a collection error rather than a
+# warning. Keep it: without it a typo'd marker silently does nothing and the
+# test it decorates stops being selectable with no signal at all.
+# Do NOT add "-n" here without first moving pytest-xdist from the [test] extra
+# into [dev] -- otherwise `pip install -e ".[dev]"` yields an env where pytest
+# cannot start (see #110).
+addopts = "-v --tb=short --strict-markers"
+# Every marker used anywhere under tests/ must appear here: --strict-markers
+# turns an undeclared marker into a hard collection error. Verify with
+#     grep -rhoE "@pytest\.mark\.[a-zA-Z_]+" tests/ | sort -u
+# and note that tests/unit/test_pytest_config.py asserts these stay registered.
 markers = [
     "real_world: marks tests as real world tests (deselect with '-m \"not real_world\"')",
     "slow: marks tests as slow (deselect with '-m \"not slow\"')",
     "unit: marks tests as unit tests",
     "integration: marks tests as integration tests",
+    "expensive: marks tests that provision billable resources (applied automatically to tests/integration; see #109)",
+    "dartmouth_network: marks tests that require access to the Dartmouth campus network",
+    "performance: marks performance benchmark tests",
+]
+# Carried over from the deleted pytest.ini, where it never took effect. These
+# suppress third-party noise only; nothing here hides a warning raised by
+# clustrix itself.
+# NB: markers visual/ssh_required/aws_required/azure_required/gcp_required are
+# deliberately NOT listed. tests/real_world/conftest.py registers them with
+# addinivalue_line at the point they are used, so --strict-markers is satisfied
+# there and they stay scoped to the suite that actually defines them.
+filterwarnings = [
+    "ignore::DeprecationWarning",
+    "ignore::PendingDeprecationWarning",
+    "ignore::UserWarning:paramiko.*",
+    "ignore::UserWarning:cryptography.*",
 ]
 
 [tool.coverage.run]

scripts/pre_push_check.py

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

tests/conftest.py

@@ -15,6 +15,66 @@ def _billable_tests_enabled():
     return os.environ.get(_OPT_IN_VAR, "").strip().lower() in _TRUTHY
 
 
+def _iter_candidate_targets(config):
+    """Yield every path-ish string this run could end up collecting from.
+
+    Reads `config.args` -- the *effective* target list -- rather than
+    `config.invocation_params.args`, which is only what the operator literally
+    typed. The difference is the whole ballgame, because there are three ways
+    to aim pytest at a directory without typing its path:
+
+        PYTEST_ADDOPTS=tests/integration/test_x.py pytest
+        pytest -o testpaths=tests/integration/test_x.py
+        (testpaths in pyproject.toml)
+
+    All three land in `config.args` and none appear in `invocation_params.args`.
+    Red-teaming confirmed the first two collected billable modules when the
+    guard read the typed argv.
+
+    Reading `config.args` is only safe because `testpaths` is `["tests"]`. If
+    it ever names tests/integration again, a bare `pytest` will be refused --
+    loudly and correctly, since testpaths would then be pointing every default
+    run at billable tests.
+    """
+    for arg in config.args:
+        yield str(arg).split("::")[0]
+    # --pyargs addresses modules by dotted name, which never looks like a path.
+    if getattr(config.option, "pyargs", False):
+        for arg in config.args:
+            yield str(arg).split("::")[0].replace(".", os.sep)
+    # -p imports a plugin by dotted name, before collection begins.
+    for plugin in getattr(config.option, "plugins", None) or []:
+        yield str(plugin).replace(".", os.sep)
+
+
+def _targets_integration_dir(candidate, config):
+    """True if `candidate` names tests/integration under any sane resolution.
+
+    Paths on the command line resolve against the invocation directory, while
+    `testpaths` resolves against rootdir. Those differ whenever pytest is run
+    from a subdirectory, and resolving against only one of them was a real
+    hole: from `tests/`, `pytest integration/test_x.py` was not recognised.
+
+    Both bases are tried. A guard protecting real money should over-match
+    rather than under-match.
+    """
+    raw = pathlib.Path(candidate)
+    if raw.is_absolute():
+        attempts = [raw]
+    else:
+        invocation_dir = getattr(config.invocation_params, "dir", None)
+        bases = [invocation_dir, pathlib.Path.cwd(), pathlib.Path(str(config.rootpath))]
+        attempts = [pathlib.Path(str(base)) / raw for base in bases if base]
+    for attempt in attempts:
+        try:
+            resolved = attempt.resolve()
+        except OSError:  # pragma: no cover - defensive, unresolvable path
+            continue
+        if resolved == _INTEGRATION_DIR or _INTEGRATION_DIR in resolved.parents:
+            return True
+    return False
+
+
 def pytest_configure(config):
     """Refuse to start when a run explicitly targets tests/integration.
 
@@ -34,22 +94,21 @@ def pytest_configure(config):
     keep working and silently skip the directory, while someone who asked for
     these tests by name gets told why they got nothing, rather than an
     inscrutable empty run.
+
+    Target discovery is in `_iter_candidate_targets` and path matching is in
+    `_targets_integration_dir`; both carry the reasoning for why they look
+    where they do. In short: read the *effective* target list, and resolve
+    relative paths against every plausible base.
     """
     if _billable_tests_enabled():
         return
-    for arg in config.args:
-        # strip pytest's "::TestClass::test_name" node-id suffix
-        candidate = pathlib.Path(str(arg).split("::")[0])
-        if not candidate.is_absolute():
-            candidate = (pathlib.Path(str(config.rootpath)) / candidate).resolve()
-        else:
-            candidate = candidate.resolve()
-        if candidate == _INTEGRATION_DIR or _INTEGRATION_DIR in candidate.parents:
+    for candidate in _iter_candidate_targets(config):
+        if _targets_integration_dir(candidate, config):
             raise pytest.UsageError(
-                f"Refusing to run {arg!r}: tests/integration provisions real, "
-                f"billable cloud resources (AWS EKS/EC2). Set {_OPT_IN_VAR}=1 to "
-                f"run them deliberately, e.g.\n"
-                f"    {_OPT_IN_VAR}=1 pytest {arg}"
+                f"Refusing to run {candidate!r}: tests/integration provisions "
+                f"real, billable cloud resources (AWS EKS/EC2). Set "
+                f"{_OPT_IN_VAR}=1 to run them deliberately, e.g.\n"
+                f"    {_OPT_IN_VAR}=1 pytest {candidate}"
             )