From 0b8f308b17f801faf4a8db75be1c57f8a8c1c3a9 Mon Sep 17 00:00:00 2001 From: nimbrel <331679006+nimbrel@users.noreply.github.com> Date: Sat, 26 Sep 2026 09:58:49 +0000 Subject: [PATCH 1/6] fix(ci): take default tests from testpaths, and requirements from beside them Without --tests, ci ran tests/ (or the root) even when pytest's testpaths named another directory, and installed tests/requirements.txt even when --tests selected other tests. The default now follows testpaths, the requirements file is looked up beside the selected tests, and the summary names the choice. Signed-off-by: nimbrel <331679006+nimbrel@users.noreply.github.com> --- CHANGELOG.md | 4 ++ docs/ci.md | 14 ++++-- python/ftcheck/ci/pipeline.py | 86 ++++++++++++++++++++++++++++++++--- python/ftcheck/cli.py | 3 +- tests/test_ci_pipeline.py | 62 +++++++++++++++++++++++++ 5 files changed, 158 insertions(+), 11 deletions(-) create mode 100644 tests/test_ci_pipeline.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 54d37ba..c9fac77 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,10 @@ (the crate root, in the image) it became a finding's primary location and merge key. - **Races outside the extension are located at the plain access**, not at an atomic helper from CPython's `pyatomic*.h` on the other side. +- **`ci` runs the tests the project configures.** With no `--tests`, the default is now + pytest's `testpaths` (then `tests/`, then the project root), and the summary says which + was chosen. Test requirements come from a `requirements.txt` beside the selected tests: + `--tests other` no longer installs `tests/requirements.txt`. ### Changed diff --git a/docs/ci.md b/docs/ci.md index 837c449..9662f2d 100644 --- a/docs/ci.md +++ b/docs/ci.md @@ -57,9 +57,15 @@ $ docker run --rm --security-opt seccomp=unconfined \ **Test dependencies** are installed automatically when the project declares them: a [PEP 735](https://peps.python.org/pep-0735/) dependency group named `test`, `tests` or `testing`; else an optional-dependencies extra of those names (installed on the -instrumented wheel, never from source); else a `requirements` file such as -`tests/requirements.txt`, `requirements-test.txt` or `requirements-dev.txt`; else a `dev` -group. Add anything else with `--with PACKAGE`; turn this off with `--no-test-deps`. +instrumented wheel, never from source); else a `requirements.txt` beside the tests being +run (`tests/requirements.txt` by default, `other/requirements.txt` with `--tests other`); +else a root `requirements` file such as `requirements-test.txt` or `requirements-dev.txt`; +else a `dev` group. The summary names the source used. Add anything else with +`--with PACKAGE`; turn this off with `--no-test-deps`. + +**Which tests run.** `--tests` when given; else pytest's own `testpaths` (from +`pytest.ini`, `pyproject.toml`, `tox.ini` or `setup.cfg`, globs expanded); else `tests/`; +else the project root. The summary says which, and why. **Tests that are not thread-safe** — using `mocker`, `capsys`, signal handlers, a fixed port, a shared temporary file — fail when every test body runs in N threads, and the run @@ -97,7 +103,7 @@ start, or every run downloads your dependencies again. |---|---|---| | `PATH` | `.` | Project root, or the crate directory when it is not at the root | | `--threads N` | 8 | Threads per test body | -| `--tests PATH` | `tests/` if present, else `.` | Test paths, relative to the project; repeatable | +| `--tests PATH` | `testpaths`, else `tests/`, else `.` | Test paths, relative to the project; repeatable | | `--pytest-arg ARG` | — | One argument passed to pytest; repeatable | | `--with PACKAGE` / `--extra NAME` | — | Extra packages in the venv / install the wheel with an extra | | `--no-test-deps` | — | Do not install declared test dependencies | diff --git a/python/ftcheck/ci/pipeline.py b/python/ftcheck/ci/pipeline.py index 4c216e1..c6737cb 100644 --- a/python/ftcheck/ci/pipeline.py +++ b/python/ftcheck/ci/pipeline.py @@ -411,7 +411,7 @@ def prepare( return None if install_runner and opts.test_deps: - declared = _declared_test_dependencies(opts.project) + declared = _declared_test_dependencies(opts.project, _select_tests(opts)[0]) if declared: label, extra_args = declared if extra_args and extra_args[0] == "{wheel}": @@ -432,7 +432,71 @@ def prepare( return Prepared(python=python, wheel=wheel, tsan_dir=tsan_dir) -def _declared_test_dependencies(project: pathlib.Path) -> tuple[str, list[str]] | None: +def _pytest_testpaths(project: pathlib.Path) -> tuple[list[str], str] | None: + """`testpaths` from the project's pytest configuration, and the file it is in. + + Files are tried in pytest's own order, and the first one that configures + pytest decides, even if it sets no `testpaths`. + """ + import configparser + import tomllib + + for name, section in ( + ("pytest.ini", "pytest"), (".pytest.ini", "pytest"), ("pyproject.toml", None), + ("tox.ini", "pytest"), ("setup.cfg", "tool:pytest"), + ): # fmt: skip + path = project / name + if not path.is_file(): + continue + if section is None: + try: + tool = tomllib.loads(path.read_text()).get("tool", {}) + except (OSError, tomllib.TOMLDecodeError): + continue + table = tool.get("pytest") + if not isinstance(table, dict): + continue + # pytest 9's native `[tool.pytest]`, or the older `ini_options`. + table = table.get("ini_options", table) + value = table.get("testpaths") + else: + parser = configparser.ConfigParser(interpolation=None) + try: + parser.read(path) + except (OSError, configparser.Error): + continue + if not parser.has_section(section): + continue + value = parser.get(section, "testpaths", fallback=None) + if isinstance(value, str): + value = value.split() + return list(value or []), name + return None + + +def _select_tests(opts: Options) -> tuple[list[str], str]: + """The test paths to run, and why these: `--tests`, pytest's `testpaths`, + `tests/`, else the project root.""" + if opts.tests: + return list(opts.tests), "--tests" + configured = _pytest_testpaths(opts.project) + if configured: + patterns, source = configured + # pytest expands globs in testpaths and ignores entries that match nothing. + paths = [] + for pattern in patterns: + found = sorted(glob.glob(pattern, root_dir=opts.project)) + paths += [p for p in found if p not in paths] + if paths: + return paths, f"testpaths in {source}" + if (opts.project / "tests").is_dir(): + return ["tests"], "tests/ directory" + return ["."], "no tests/ directory" + + +def _declared_test_dependencies( + project: pathlib.Path, tests: list[str] | None = None +) -> tuple[str, list[str]] | None: # Returns (label, pip arguments); ["{wheel}", extra] means "the built wheel # with this extra", resolved by the caller, which knows the wheel. """Where the project declares its test dependencies, as pip arguments. @@ -440,8 +504,9 @@ def _declared_test_dependencies(project: pathlib.Path) -> tuple[str, list[str]] Neither project in a first-user trial got past test collection: the venv held only the wheel and the runner. In order: a PEP 735 dependency group (`test`, `tests`, `testing`), an optional-dependencies extra of those - names, a conventional requirements file; `dev` last, as it often carries - whole toolchains. `--no-test-deps` turns this off. + names, a `requirements.txt` beside the selected tests (`tests` defaults + to `tests/`), a conventional requirements file at the root; `dev` last, + as it often carries whole toolchains. `--no-test-deps` turns this off. """ import tomllib @@ -462,7 +527,15 @@ def _declared_test_dependencies(project: pathlib.Path) -> tuple[str, list[str]] # Applied to the instrumented wheel, never `.[name]`: installing # from the source tree would build a second, uninstrumented copy. return f"extra '{name}'", ["{wheel}", name] - for rel in ("tests/requirements.txt", "requirements-test.txt", "requirements-tests.txt", + # Beside the tests being run, never a `tests/` that is not among them. + beside = [] + for test in tests if tests is not None else ["tests"]: + folder = test if (project / test).is_dir() else os.path.dirname(test) + rel = os.path.normpath(os.path.join(folder, "requirements.txt")) + # The root's requirements.txt is the package's own, not its tests'. + if os.path.dirname(rel) and rel not in beside: + beside.append(rel) + for rel in (*beside, "requirements-test.txt", "requirements-tests.txt", "test-requirements.txt", "requirements/test.txt", "requirements/tests.txt", "requirements-dev.txt", "requirements/dev.txt"): # fmt: skip if (project / rel).is_file(): @@ -531,7 +604,8 @@ def run(env: Environment, opts: Options) -> Outcome: out.stage = "test" junit = work / "pytest-junit.xml" - tests = opts.tests or (["tests"] if (opts.project / "tests").is_dir() else ["."]) + tests, chosen = _select_tests(opts) + out.notes.append(f"running {' '.join(tests)} ({chosen})") cmd = [ prepared.python, "-m", "pytest", *tests, f"--parallel-threads={opts.threads}", diff --git a/python/ftcheck/cli.py b/python/ftcheck/cli.py index f7e5fb6..189928b 100644 --- a/python/ftcheck/cli.py +++ b/python/ftcheck/cli.py @@ -71,7 +71,8 @@ def build_parser() -> _Parser: action="append", default=[], metavar="PATH", - help="test paths, relative to the project (default: tests/ if present)", + help="test paths, relative to the project (default: pytest's testpaths, else tests/ " + "if present, else the project root)", ) ci_parser.add_argument( "--extra", action="append", default=[], help="install the project with this extra" diff --git a/tests/test_ci_pipeline.py b/tests/test_ci_pipeline.py new file mode 100644 index 0000000..16c9b07 --- /dev/null +++ b/tests/test_ci_pipeline.py @@ -0,0 +1,62 @@ +# SPDX-License-Identifier: MIT OR Apache-2.0 +"""The ci pipeline's inputs: which tests run, what is installed, what is kept.""" +import pathlib + +from ftcheck.ci.pipeline import Options, _declared_test_dependencies, _select_tests + + +def opts(project: pathlib.Path, **kw) -> Options: + return Options(project=project, work_dir=project / "work", **kw) + + +def test_default_tests_come_from_testpaths(tmp_path): + """Regression: `testpaths` was ignored, so a suite outside `tests/` never ran.""" + (tmp_path / "tests").mkdir() + (tmp_path / "python" / "pkg" / "tests").mkdir(parents=True) + (tmp_path / "pyproject.toml").write_text( + '[tool.pytest.ini_options]\ntestpaths = ["python/pkg/tests", "missing"]\n' + ) + paths, source = _select_tests(opts(tmp_path)) + assert paths == ["python/pkg/tests"], "a testpaths entry that does not exist is dropped" + assert source == "testpaths in pyproject.toml" + + +def test_testpaths_are_read_from_ini_files_and_globs(tmp_path): + (tmp_path / "suite_a").mkdir() + (tmp_path / "suite_b").mkdir() + (tmp_path / "pytest.ini").write_text("[pytest]\ntestpaths =\n suite_*\n") + assert _select_tests(opts(tmp_path))[0] == ["suite_a", "suite_b"] + (tmp_path / "pytest.ini").unlink() + (tmp_path / "setup.cfg").write_text("[tool:pytest]\ntestpaths = suite_b\n") + assert _select_tests(opts(tmp_path))[0] == ["suite_b"] + + +def test_without_testpaths_tests_dir_then_the_root(tmp_path): + assert _select_tests(opts(tmp_path)) == (["."], "no tests/ directory") + (tmp_path / "tests").mkdir() + assert _select_tests(opts(tmp_path)) == (["tests"], "tests/ directory") + + +def test_explicit_tests_win(tmp_path): + (tmp_path / "pyproject.toml").write_text('[tool.pytest.ini_options]\ntestpaths = ["a"]\n') + (tmp_path / "a").mkdir() + paths, source = _select_tests(opts(tmp_path, tests=["other"])) + assert paths == ["other"] and source == "--tests" + + +def test_requirements_come_from_beside_the_selected_tests(tmp_path): + """Regression: `tests/requirements.txt` was installed even with `--tests other/`.""" + (tmp_path / "tests").mkdir() + (tmp_path / "tests" / "requirements.txt").write_text("hypothesis\n") + (tmp_path / "other").mkdir() + assert _declared_test_dependencies(tmp_path, ["other"]) is None + (tmp_path / "other" / "requirements.txt").write_text("pytest-timeout\n") + assert _declared_test_dependencies(tmp_path, ["other/test_x.py"]) == ( + "other/requirements.txt", + ["-r", "other/requirements.txt"], + ) + assert _declared_test_dependencies(tmp_path, ["tests"]) == ( + "tests/requirements.txt", + ["-r", "tests/requirements.txt"], + ) + From c71be7e5ec726f6f3a5626ed6830d21ab147e349 Mon Sep 17 00:00:00 2001 From: nimbrel <331679006+nimbrel@users.noreply.github.com> Date: Sat, 26 Sep 2026 09:59:32 +0000 Subject: [PATCH 2/6] fix(ci): override a project's maturin strip setting CARGO_PROFILE_RELEASE_STRIP only reaches Cargo's profile; strip = true under [tool.maturin] is applied by maturin itself and still erased every frame name. maturin 1.12 added MATURIN_STRIP to override pyproject.toml, so the build sets it. The stripped-library warning stays for older maturin, and _stripped no longer reads an unreadable file as stripped. Signed-off-by: nimbrel <331679006+nimbrel@users.noreply.github.com> --- CHANGELOG.md | 2 ++ docs/ci.md | 8 +++--- python/ftcheck/ci/pipeline.py | 7 ++++- tests/test_ci_pipeline.py | 51 +++++++++++++++++++++++++++++++++++ 4 files changed, 64 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c9fac77..54ac325 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,8 @@ pytest's `testpaths` (then `tests/`, then the project root), and the summary says which was chosen. Test requirements come from a `requirements.txt` beside the selected tests: `--tests other` no longer installs `tests/requirements.txt`. +- **`strip = true` under `[tool.maturin]` is overridden**, not only warned about: the + instrumented build sets `MATURIN_STRIP=false` (maturin 1.12 or later). ### Changed diff --git a/docs/ci.md b/docs/ci.md index 9662f2d..f3fb692 100644 --- a/docs/ci.md +++ b/docs/ci.md @@ -82,9 +82,11 @@ be passed this way. **A session that aborts** part-way — a pytest `INTERNALERROR` — exits `3`, not `4`: the suite did not run, so nothing can be said about it. pytest's own summary line is printed. -**Stripped builds.** If the extension is stripped (`strip = true` under `[tool.maturin]` or -in the release profile), every frame of yours in a report is nameless; ftcheck warns. Set -`strip = false` for ftcheck runs. +**Stripped builds.** A stripped extension makes every frame of yours in a report +nameless, so the build overrides both places a project can ask for it: the release +profile (`CARGO_PROFILE_RELEASE_STRIP=false`) and `strip = true` under `[tool.maturin]` +(`MATURIN_STRIP=false`, which needs maturin 1.12 or later; the image carries a newer one). +If the extension is stripped anyway, ftcheck warns. **setuptools-rust projects** are not built as they stand — ftcheck builds with maturin and says so. A `[tool.maturin]` section naming the module and any Cargo features is enough. diff --git a/python/ftcheck/ci/pipeline.py b/python/ftcheck/ci/pipeline.py index c6737cb..d6a91a1 100644 --- a/python/ftcheck/ci/pipeline.py +++ b/python/ftcheck/ci/pipeline.py @@ -257,7 +257,8 @@ def _stripped(path: pathlib.Path) -> bool: sections = subprocess.run( [readelf, "-S", str(path)], capture_output=True, text=True, check=False ).stdout - return ".symtab" not in sections + # No section headers at all means readelf could not read it: not stripped. + return "Section Headers" in sections and ".symtab" not in sections def _count_tests(junit: pathlib.Path) -> tuple[int, int, int]: @@ -320,6 +321,10 @@ def prepare( # A project's `strip = true` would erase every frame name from the # reports (seen on two public projects). "CARGO_PROFILE_RELEASE_STRIP": "false", + # ...and so would `strip = true` under [tool.maturin], which maturin + # applies itself; since 1.12 this variable overrides pyproject.toml. + # An older maturin ignores it, and the stripped-library warning remains. + "MATURIN_STRIP": "false", # C and C++ built by build scripts (the `cc` crate honours these) must be # instrumented too, and by the clang that built the interpreter: gcc's # libtsan is a different runtime from the one already loaded. diff --git a/tests/test_ci_pipeline.py b/tests/test_ci_pipeline.py index 16c9b07..ed5cde5 100644 --- a/tests/test_ci_pipeline.py +++ b/tests/test_ci_pipeline.py @@ -1,7 +1,11 @@ # SPDX-License-Identifier: MIT OR Apache-2.0 """The ci pipeline's inputs: which tests run, what is installed, what is kept.""" import pathlib +import shutil +import subprocess +import pytest +from ftcheck.ci import pipeline from ftcheck.ci.pipeline import Options, _declared_test_dependencies, _select_tests @@ -60,3 +64,50 @@ def test_requirements_come_from_beside_the_selected_tests(tmp_path): ["-r", "tests/requirements.txt"], ) + + +class Recorder: + """Stands in for `_stream`: records each command and its environment.""" + + def __init__(self, fail_on: str = "maturin"): + self.calls: list[tuple[list[str], dict]] = [] + self.fail_on = fail_on + + def __call__(self, cmd, log_path, **kwargs): + self.calls.append((cmd, kwargs.get("env") or {})) + return 1 if self.fail_on in cmd[0] else 0 + + +def environment(): + from ftcheck.ci.environment import Environment + + return Environment(python="python3", toolchain="nightly", interpreter={"executable": "python3"}) + + +def test_the_build_overrides_a_project_strip_setting(tmp_path, monkeypatch): + """`strip = true` under [tool.maturin] erased every frame name; maturin + >= 1.12 lets MATURIN_STRIP override pyproject.toml.""" + recorder = Recorder() + monkeypatch.setattr(pipeline, "_stream", recorder) + out = pipeline.Outcome() + assert pipeline.prepare(environment(), opts(tmp_path), out) is None + (build_cmd, build_env), = recorder.calls + assert build_cmd[:2] == ["maturin", "build"] + assert build_env["MATURIN_STRIP"] == "false" + assert build_env["CARGO_PROFILE_RELEASE_STRIP"] == "false" + + +@pytest.mark.skipif(not (shutil.which("readelf") and shutil.which("objcopy")), reason="binutils") +def test_a_stripped_library_is_recognised(tmp_path): + import ftcheck._ftcheck as ext + + unstripped = tmp_path / "unstripped.so" + shutil.copy(ext.__file__, unstripped) + if pipeline._stripped(unstripped): + pytest.skip("the installed extension is itself stripped") + stripped = tmp_path / "stripped.so" + subprocess.run(["objcopy", "--strip-all", str(unstripped), str(stripped)], check=True) + assert pipeline._stripped(stripped) + not_elf = tmp_path / "not-elf.so" + not_elf.write_text("not a shared library\n") + assert not pipeline._stripped(not_elf), "unreadable is not the same as stripped" From e1f7fe6e9de715b02c04eb4da7eaeec5862bdd86 Mon Sep 17 00:00:00 2001 From: nimbrel <331679006+nimbrel@users.noreply.github.com> Date: Sat, 26 Sep 2026 10:00:23 +0000 Subject: [PATCH 3/6] fix(ci): install [build-system].requires for the build A build script that imports Python packages (cffi and setuptools, for one) failed, because nothing installed the project's build requirements. They are now installed, less maturin, into a build venv on the TSan interpreter that is first on PATH for maturin build. The test venv stays as it was. Signed-off-by: nimbrel <331679006+nimbrel@users.noreply.github.com> --- CHANGELOG.md | 2 + docs/ci.md | 3 ++ python/ftcheck/ci/pipeline.py | 78 +++++++++++++++++++++++++++++------ tests/test_ci_pipeline.py | 35 ++++++++++++++++ 4 files changed, 105 insertions(+), 13 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 54ac325..b90b48c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,6 +21,8 @@ `--tests other` no longer installs `tests/requirements.txt`. - **`strip = true` under `[tool.maturin]` is overridden**, not only warned about: the instrumented build sets `MATURIN_STRIP=false` (maturin 1.12 or later). +- **Build scripts that import Python packages build.** `[build-system].requires`, less + maturin, is installed into a build venv that is first on `PATH` for the build. ### Changed diff --git a/docs/ci.md b/docs/ci.md index f3fb692..94675a3 100644 --- a/docs/ci.md +++ b/docs/ci.md @@ -39,6 +39,9 @@ $ docker run --rm --security-opt seccomp=unconfined \ C and C++ compiled by build scripts get `-fsanitize=thread` through `CFLAGS`/`CXXFLAGS`, compiled by **the same clang that built the interpreter**. Line tables are on, so every Rust frame has a file and line. + A build script that imports Python packages gets them: `[build-system].requires` + (less maturin, which the image provides) is installed into a separate build venv on + the TSan interpreter, first on `PATH` for the build. 3. **Isolated install.** A venv on the TSan interpreter, with the wheel, `pytest` and `pytest-run-parallel`. 4. **The suite, concurrently.** `pytest --parallel-threads=N` (default 8) runs each test diff --git a/python/ftcheck/ci/pipeline.py b/python/ftcheck/ci/pipeline.py index d6a91a1..f95592b 100644 --- a/python/ftcheck/ci/pipeline.py +++ b/python/ftcheck/ci/pipeline.py @@ -248,6 +248,29 @@ def _build_backend(project: pathlib.Path) -> str | None: return None +def _build_requirements(project: pathlib.Path) -> list[str]: + """`[build-system].requires`, less maturin itself. + + A build script can import Python packages (cffi and setuptools, on a + public project), and nothing installed them. maturin is left out: the + image's own binary builds, and a pip-installed one would shadow it. + """ + import tomllib + + try: + data = tomllib.loads((project / "pyproject.toml").read_text()) + except (OSError, tomllib.TOMLDecodeError): + return [] + requires = data.get("build-system", {}).get("requires", []) + kept = [] + for req in requires: + name = re.match(r"\s*([A-Za-z0-9][A-Za-z0-9._-]*)", req) + if name and re.sub(r"[-_.]+", "-", name.group(1)).lower() == "maturin": + continue + kept.append(req) + return kept + + def _stripped(path: pathlib.Path) -> bool: import shutil @@ -275,6 +298,15 @@ def _count_tests(junit: pathlib.Path) -> tuple[int, int, int]: return tests - skipped, failed, errors +def _make_venv(env: Environment, venv: pathlib.Path, log: pathlib.Path, setup_env: dict) -> bool: + """A venv on the TSan interpreter, unless one is already there.""" + if (venv / "bin" / "python").exists(): + return True + _log(f"creating {venv.name}") + cmd = [env.interpreter["executable"], "-m", "venv", str(venv)] + return _stream(cmd, log, env=setup_env) == 0 + + @dataclass class Prepared: """An instrumented build installed into a venv, ready to be exercised.""" @@ -331,6 +363,35 @@ def prepare( "CFLAGS": (os.environ.get("CFLAGS", "") + " -fsanitize=thread -g").strip(), "CXXFLAGS": (os.environ.get("CXXFLAGS", "") + " -fsanitize=thread -g").strip(), } + # Setup runs under TSan too — it is the same interpreter — but its reports + # are CPython's and pip's, not the project's, so they go to a separate log. + setup_env = { + **os.environ, + "TSAN_OPTIONS": _tsan_options(env, work / "setup-tsan", opts.suppressions), + } + build_requires = _build_requirements(opts.project) + if build_requires: + # A venv of their own, on PATH for the build: the test venv then holds + # only what the project declares for its tests. + build_venv = work / "build-venv" + deps_log = work / "build-deps.log" + _log(f"installing the build requirements ({', '.join(build_requires)})") + if not _make_venv(env, build_venv, deps_log, setup_env): + out.error = "could not create a venv for the build requirements" + out.log = _tail(deps_log) + return None + cmd = [ + str(build_venv / "bin" / "python"), "-m", "pip", "install", "--quiet", + "--disable-pip-version-check", *build_requires, + ] # fmt: skip + if _stream(cmd, deps_log, env=setup_env) != 0: + out.error = "could not install the project's [build-system].requires" + out.log = _tail(deps_log) + out.log_path = str(deps_log) + return None + build_env["PATH"] = os.pathsep.join( + [str(build_venv / "bin"), os.environ.get("PATH", os.defpath)] + ) if env.cc: build_env["CC"] = env.cc if env.cxx: @@ -380,21 +441,12 @@ def prepare( previous.mkdir(exist_ok=True) for f in old_logs: f.rename(previous / f.name) - # Setup runs under TSan too — it is the same interpreter — but its reports - # are CPython's and pip's, not the project's, so they go to a separate log. - setup_env = { - **os.environ, - "TSAN_OPTIONS": _tsan_options(env, work / "setup-tsan", opts.suppressions), - } install_log = work / "install.log" python = str(venv / "bin" / "python") - if not pathlib.Path(python).exists(): - _log("creating venv") - cmd = [env.interpreter["executable"], "-m", "venv", str(venv)] - if _stream(cmd, install_log, env=setup_env) != 0: - out.error = "could not create a venv on the TSan interpreter" - out.log = _tail(install_log) - return None + if not _make_venv(env, venv, install_log, setup_env): + out.error = "could not create a venv on the TSan interpreter" + out.log = _tail(install_log) + return None spec = str(wheel) + (f"[{','.join(opts.extras)}]" if opts.extras else "") _log(f"installing {wheel.name} and the test runner") cmd = [ diff --git a/tests/test_ci_pipeline.py b/tests/test_ci_pipeline.py index ed5cde5..71387c8 100644 --- a/tests/test_ci_pipeline.py +++ b/tests/test_ci_pipeline.py @@ -111,3 +111,38 @@ def test_a_stripped_library_is_recognised(tmp_path): not_elf = tmp_path / "not-elf.so" not_elf.write_text("not a shared library\n") assert not pipeline._stripped(not_elf), "unreadable is not the same as stripped" + + +def test_build_requirements_are_installed_for_build_scripts(tmp_path, monkeypatch): + """Regression: a build script needing Python packages (cffi, setuptools) + failed, because [build-system].requires was never installed.""" + (tmp_path / "pyproject.toml").write_text( + '[build-system]\nrequires = ["maturin>=1.5,<2", "cffi>=1.0", "setuptools"]\n' + 'build-backend = "maturin"\n' + ) + recorder = Recorder() + monkeypatch.setattr(pipeline, "_stream", recorder) + assert pipeline.prepare(environment(), opts(tmp_path), pipeline.Outcome()) is None + (venv_cmd, _), (pip_cmd, _), (build_cmd, build_env) = recorder.calls + build_venv = tmp_path / "work" / "build-venv" + assert venv_cmd == ["python3", "-m", "venv", str(build_venv)] + assert pip_cmd[0] == str(build_venv / "bin" / "python") + assert pip_cmd[-2:] == ["cffi>=1.0", "setuptools"], "maturin is the image's, not pip's" + assert build_cmd[0] == "maturin" + assert build_env["PATH"].split(":")[0] == str(build_venv / "bin") + + +def test_no_build_requirements_beyond_maturin_means_no_build_venv(tmp_path, monkeypatch): + (tmp_path / "pyproject.toml").write_text('[build-system]\nrequires = ["maturin"]\n') + recorder = Recorder() + monkeypatch.setattr(pipeline, "_stream", recorder) + pipeline.prepare(environment(), opts(tmp_path), pipeline.Outcome()) + assert [cmd[0] for cmd, _ in recorder.calls] == ["maturin"] + + +def test_a_failed_build_requirement_install_names_its_stage(tmp_path, monkeypatch): + (tmp_path / "pyproject.toml").write_text('[build-system]\nrequires = ["cffi"]\n') + monkeypatch.setattr(pipeline, "_stream", Recorder(fail_on="build-venv")) + out = pipeline.Outcome() + assert pipeline.prepare(environment(), opts(tmp_path), out) is None + assert out.stage == "build" and "[build-system].requires" in out.error From d77e675c6f7f0f228d5c59c4b91cbbd9ac49ddae Mon Sep 17 00:00:00 2001 From: nimbrel <331679006+nimbrel@users.noreply.github.com> Date: Sat, 26 Sep 2026 10:01:29 +0000 Subject: [PATCH 4/6] feat(ci): note when importing the module re-enables the GIL Every run forces PYTHON_GIL=0, so a module without gil_used = false was tested in a mode its users never get, and nothing said so. After install, each extension module is imported once without PYTHON_GIL; if the GIL comes back on, ci and stress add a note. stress now prints the pipeline's notes. Signed-off-by: nimbrel <331679006+nimbrel@users.noreply.github.com> --- CHANGELOG.md | 7 ++++++ docs/ci.md | 5 ++++- python/ftcheck/ci/pipeline.py | 42 +++++++++++++++++++++++++++++++++++ python/ftcheck/report/text.py | 4 +++- tests/test_ci_pipeline.py | 37 ++++++++++++++++++++++++++++++ 5 files changed, 93 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b90b48c..48bb9a1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -24,6 +24,13 @@ - **Build scripts that import Python packages build.** `[build-system].requires`, less maturin, is installed into a build venv that is first on `PATH` for the build. +### Added + +- **A note when a module re-enables the GIL.** `ci` and `stress` import each extension + module once without `PYTHON_GIL`; if the GIL comes back on (no `gil_used = false`), the + summary says the results hold only with `PYTHON_GIL=0`. `stress` now prints the + pipeline's notes. + ### Changed - Race messages name the threads (`by thread T5 (ftm-refill)`), the text summary labels diff --git a/docs/ci.md b/docs/ci.md index 94675a3..28fd8aa 100644 --- a/docs/ci.md +++ b/docs/ci.md @@ -48,7 +48,10 @@ $ docker run --rm --security-opt seccomp=unconfined \ body in N threads simultaneously. A module-level instance your tests touch is therefore **shared across threads** — which is the situation the GIL used to make safe. `PYTHON_GIL=0` is forced, so an extension that does not declare `gil_used = false` - cannot quietly turn the GIL back on and hide every race. + cannot quietly turn the GIL back on and hide every race. Because users' interpreters do not + force it, each extension module is first imported once without `PYTHON_GIL`; if that + turns the GIL back on, a note says so (in `stress` too): the results then describe the + module only with the GIL forced off. 5. **Collect.** TSan logs are parsed, each report is attributed, and repeats of the same race — from either side of the pair, from any threads — collapse into one finding. 6. **Report.** Terminal summary, `--format json`, `--sarif`, `--junit`. The lint and the diff --git a/python/ftcheck/ci/pipeline.py b/python/ftcheck/ci/pipeline.py index f95592b..81a8bea 100644 --- a/python/ftcheck/ci/pipeline.py +++ b/python/ftcheck/ci/pipeline.py @@ -298,6 +298,44 @@ def _count_tests(junit: pathlib.Path) -> tuple[int, int, int]: return tests - skipped, failed, errors +# Printed True only on a free-threaded build whose GIL came back on import. +_GIL_PROBE = ( + "import importlib, sys, sysconfig\n" + "importlib.import_module(sys.argv[1])\n" + "print(bool(sysconfig.get_config_var('Py_GIL_DISABLED')) and sys._is_gil_enabled())\n" +) + + +def _reenables_gil(python: str, module: str, env: dict, cwd) -> bool | None: + """Whether importing `module` turns the GIL back on; None if the probe failed. + + Run without PYTHON_GIL, which is how the module's users will import it. + """ + probe_env = {k: v for k, v in env.items() if k != "PYTHON_GIL"} + try: + proc = subprocess.run( + [python, "-c", _GIL_PROBE, module], + capture_output=True, text=True, env=probe_env, cwd=cwd, timeout=300, check=False, + ) # fmt: skip + except (OSError, subprocess.TimeoutExpired): + return None + last = proc.stdout.strip().splitlines()[-1:] + if proc.returncode != 0 or not last: + return None + return {"True": True, "False": False}.get(last[0]) + + +def _gil_notes(python: str, modules: list[str], env: dict, cwd) -> list[str]: + return [ + f"`{module}` does not declare `gil_used = false`, so a free-threaded interpreter " + "re-enables the GIL when it is imported. This run forced the GIL off " + "(PYTHON_GIL=0): its results describe the module only when it is imported that " + "way. Declare `#[pymodule(gil_used = false)]` once the module is thread-safe." + for module in modules + if _reenables_gil(python, module, env, cwd) + ] + + def _make_venv(env: Environment, venv: pathlib.Path, log: pathlib.Path, setup_env: dict) -> bool: """A venv on the TSan interpreter, unless one is already there.""" if (venv / "bin" / "python").exists(): @@ -486,6 +524,10 @@ def prepare( return None out.notes.append(f"installed the project's test dependencies from {label}") + # Every run forces PYTHON_GIL=0; say when that hides what users would get. + out.notes.extend( + _gil_notes(python, native_modules(wheel), {**setup_env, "PYTHONSAFEPATH": "1"}, work) + ) return Prepared(python=python, wheel=wheel, tsan_dir=tsan_dir) diff --git a/python/ftcheck/report/text.py b/python/ftcheck/report/text.py index 8ac6737..a97ccad 100644 --- a/python/ftcheck/report/text.py +++ b/python/ftcheck/report/text.py @@ -189,7 +189,7 @@ def render_ci(env, outcome, code: int, headline: str, threads: int, stream: Text file=stream, ) for note in outcome.notes: - print(note, file=stream) + print(_wrap(note, ""), file=stream) if outcome.pytest_exit in _PYTEST_DID_NOT_RUN: # Collection errors show up as tests in pytest's XML; "ran 8 tests" # when none ran told a first user nothing. @@ -259,6 +259,8 @@ def render_stress( if outcome is not None: if outcome.extension_modules: print(f"\nbuilt {', '.join(outcome.extension_modules)} with ThreadSanitizer", file=stream) + for note in outcome.notes: + print(_wrap(note, ""), file=stream) if outcome.error: print(f"\n{outcome.error}", file=stream) for line in (outcome.log or "").splitlines(): diff --git a/tests/test_ci_pipeline.py b/tests/test_ci_pipeline.py index 71387c8..7f9bbf5 100644 --- a/tests/test_ci_pipeline.py +++ b/tests/test_ci_pipeline.py @@ -146,3 +146,40 @@ def test_a_failed_build_requirement_install_names_its_stage(tmp_path, monkeypatc out = pipeline.Outcome() assert pipeline.prepare(environment(), opts(tmp_path), out) is None assert out.stage == "build" and "[build-system].requires" in out.error + + +def test_a_module_that_re_enables_the_gil_is_noted(monkeypatch): + """Stock free-threaded CPython re-enables the GIL for a module that does not + declare `gil_used = false`; ftcheck forces it off, and must say so.""" + monkeypatch.setattr(pipeline, "_reenables_gil", lambda python, m, env, cwd: m == "pkg._a") + (note,) = pipeline._gil_notes("python", ["pkg._a", "pkg._b"], {}, ".") + assert "`pkg._a`" in note and "gil_used = false" in note and "PYTHON_GIL=0" in note + monkeypatch.setattr(pipeline, "_reenables_gil", lambda *a: None) + assert pipeline._gil_notes("python", ["pkg._a"], {}, ".") == [], "inconclusive is silent" + + +def test_the_gil_probe_runs_without_the_forced_setting(tmp_path): + import os + import sys + + (tmp_path / "gil_probe_target.py").write_text( + "import os\nassert 'PYTHON_GIL' not in os.environ, 'the probe must not force it'\n" + ) + env = {**os.environ, "PYTHONPATH": str(tmp_path), "PYTHON_GIL": "0"} + assert pipeline._reenables_gil(sys.executable, "gil_probe_target", env, tmp_path) is False + assert pipeline._reenables_gil(sys.executable, "no_such_module_xyz", env, tmp_path) is None + + +def test_stress_prints_the_pipeline_notes(): + import io + from types import SimpleNamespace + + from ftcheck.report import text + from ftcheck.stress import StressOutcome + + env = environment() + out = StressOutcome(notes=["`pkg._a` does not declare `gil_used = false`"]) + sopts = SimpleNamespace(seed=1, iterations=10, budget_seconds=5) + stream = io.StringIO() + text.render_stress(env, out, 0, "headline", sopts, 8, stream) + assert "`pkg._a` does not declare" in stream.getvalue() From 21f00e29ace6071d5631892a08835d5e93595b31 Mon Sep 17 00:00:00 2001 From: nimbrel <331679006+nimbrel@users.noreply.github.com> Date: Sat, 26 Sep 2026 10:02:42 +0000 Subject: [PATCH 5/6] fix(ci): keep each run's raw TSan logs in a directory of its own Only one earlier generation was kept (tsan-previous/), so running several seeds back to back overwrote the logs behind a finding. Each run now writes to tsan-runs/[-seed]/, the summary names it, and the last five runs are kept. Signed-off-by: nimbrel <331679006+nimbrel@users.noreply.github.com> --- CHANGELOG.md | 3 ++ docs/ci.md | 4 +++ python/ftcheck/ci/pipeline.py | 53 ++++++++++++++++++++++--------- python/ftcheck/stress/__init__.py | 2 +- tests/test_ci_pipeline.py | 14 ++++++++ 5 files changed, 60 insertions(+), 16 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 48bb9a1..bef499d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -23,6 +23,9 @@ instrumented build sets `MATURIN_STRIP=false` (maturin 1.12 or later). - **Build scripts that import Python packages build.** `[build-system].requires`, less maturin, is installed into a build venv that is first on `PATH` for the build. +- **Raw TSan logs are kept per run.** Each run writes to `tsan-runs/[-seed]/` + under the work directory, named in the summary; the last five are kept (previously one + generation, `tsan-previous/`, which back-to-back seeds overwrote). ### Added diff --git a/docs/ci.md b/docs/ci.md index 28fd8aa..70862e3 100644 --- a/docs/ci.md +++ b/docs/ci.md @@ -105,6 +105,10 @@ project root cannot shadow the instrumented wheel — common in maturin mixed la `target/ftcheck-stress`), so each builds once; mount a crate cache as in the README's quick start, or every run downloads your dependencies again. +**Raw logs.** Each run's ThreadSanitizer logs go to a directory of their own under the +work directory, `tsan-runs//` (`-seed` appended for `stress`), named in the +summary. The last five runs are kept, so back-to-back seeds do not overwrite each other. + ## Options | Option | Default | | diff --git a/python/ftcheck/ci/pipeline.py b/python/ftcheck/ci/pipeline.py index 81a8bea..1d05060 100644 --- a/python/ftcheck/ci/pipeline.py +++ b/python/ftcheck/ci/pipeline.py @@ -345,6 +345,32 @@ def _make_venv(env: Environment, venv: pathlib.Path, log: pathlib.Path, setup_en return _stream(cmd, log, env=setup_env) == 0 +# Raw TSan logs are the evidence behind a finding; a rerun must not erase them. +_KEEP_TSAN_RUNS = 5 + + +def _new_tsan_dir(work: pathlib.Path, label: str = "") -> pathlib.Path: + """A fresh `tsan-runs/[-label]/` for this run's raw logs. + + The last five runs are kept: one saved generation was not enough when + several seeds ran back to back. + """ + import shutil + from datetime import UTC, datetime + + runs = work / "tsan-runs" + runs.mkdir(parents=True, exist_ok=True) + name = datetime.now(UTC).strftime("%Y%m%dT%H%M%SZ") + (f"-{label}" if label else "") + path, n = runs / name, 1 + while path.exists(): + n += 1 + path = runs / f"{name}-{n}" + path.mkdir() + for old in sorted(d for d in runs.iterdir() if d.is_dir())[:-_KEEP_TSAN_RUNS]: + shutil.rmtree(old, ignore_errors=True) + return path + + @dataclass class Prepared: """An instrumented build installed into a venv, ready to be exercised.""" @@ -355,13 +381,18 @@ class Prepared: def prepare( - env: Environment, opts: Options, out: Outcome, install_runner: bool = True + env: Environment, + opts: Options, + out: Outcome, + install_runner: bool = True, + run_label: str = "", ) -> Prepared | None: """Stages 1 and 2: instrumented build, isolated install. None on failure. `install_runner=False` skips pytest and pytest-run-parallel, which only `ci` uses: `stress` runs no tests, and every download is one more thing a flaky - network can stall. + network can stall. `run_label` names this run's TSan log directory + (`stress` passes its seed). """ work = opts.work_dir work.mkdir(parents=True, exist_ok=True) @@ -466,19 +497,11 @@ def prepare( # --- 2. an isolated venv on the TSan interpreter --------------------------- out.stage = "install" venv = work / "venv" - tsan_dir = work / "tsan" - tsan_dir.mkdir(exist_ok=True) - # The previous run's raw logs move aside instead of being deleted — a rerun - # in the same work directory once erased the only logs of a confirmed race. - previous = work / "tsan-previous" - old_logs = [f for f in tsan_dir.glob("*") if f.is_file()] - if old_logs: - if previous.exists(): - for f in previous.glob("*"): - f.unlink() - previous.mkdir(exist_ok=True) - for f in old_logs: - f.rename(previous / f.name) + # A directory per run — a rerun in the same work directory once erased the + # only logs of a confirmed race. + tsan_dir = _new_tsan_dir(work, run_label) + _log(f"raw ThreadSanitizer logs go to {tsan_dir}") + out.notes.append(f"raw ThreadSanitizer logs: {tsan_dir}") install_log = work / "install.log" python = str(venv / "bin" / "python") if not _make_venv(env, venv, install_log, setup_env): diff --git a/python/ftcheck/stress/__init__.py b/python/ftcheck/stress/__init__.py index db2d762..e52ccb4 100644 --- a/python/ftcheck/stress/__init__.py +++ b/python/ftcheck/stress/__init__.py @@ -89,7 +89,7 @@ def _log(message: str) -> None: def run(env: Environment, opts: Options, sopts: StressOptions, config: dict) -> StressOutcome: out = StressOutcome() - prepared = prepare(env, opts, out, install_runner=False) + prepared = prepare(env, opts, out, install_runner=False, run_label=f"seed{sopts.seed}") if prepared is None: return out diff --git a/tests/test_ci_pipeline.py b/tests/test_ci_pipeline.py index 7f9bbf5..7fbd61e 100644 --- a/tests/test_ci_pipeline.py +++ b/tests/test_ci_pipeline.py @@ -183,3 +183,17 @@ def test_stress_prints_the_pipeline_notes(): stream = io.StringIO() text.render_stress(env, out, 0, "headline", sopts, 8, stream) assert "`pkg._a` does not declare" in stream.getvalue() + + +def test_each_run_keeps_its_own_tsan_logs(tmp_path): + """Regression: one generation of raw logs was kept, so back-to-back seeds + overwrote the logs of the run before last.""" + made = [pipeline._new_tsan_dir(tmp_path, f"seed{n}") for n in range(7)] + assert len(set(made)) == 7, "runs in the same second still get their own directory" + kept = sorted(p.name for p in (tmp_path / "tsan-runs").iterdir()) + assert len(kept) == pipeline._KEEP_TSAN_RUNS == 5 + assert made[-1].is_dir() and made[-1].name.endswith("seed6") + assert not made[0].exists() + assert made[-1].name[:8].isdigit() and "Z" in made[-1].name, "named for the UTC time" + same = [pipeline._new_tsan_dir(tmp_path / "ci", "") for _ in range(2)] + assert same[0] != same[1] and all(p.is_dir() for p in same) From 9b92af3619d17cb4efc9be2c05a91ab1ae5388f1 Mon Sep 17 00:00:00 2001 From: nimbrel <331679006+nimbrel@users.noreply.github.com> Date: Sat, 26 Sep 2026 10:04:21 +0000 Subject: [PATCH 6/6] fix(ci): detect a pytest session that ends before running what it collected Only an INTERNALERROR was recognised as an aborted session. A session that ran fewer tests than it collected (JUnit tests, skipped and deselected included), without -x or --maxfail saying why, now reads as aborted and exits 3. The JUnit file is removed before each session, so one that dies before writing it no longer inherits the previous run's counts. Signed-off-by: nimbrel <331679006+nimbrel@users.noreply.github.com> --- CHANGELOG.md | 5 ++ docs/ci.md | 6 ++- python/ftcheck/ci/pipeline.py | 32 +++++++++++-- tests/test_ci_pipeline.py | 89 +++++++++++++++++++++++++++++++++++ 4 files changed, 125 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index bef499d..f2f76e0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -26,6 +26,11 @@ - **Raw TSan logs are kept per run.** Each run writes to `tsan-runs/[-seed]/` under the work directory, named in the summary; the last five are kept (previously one generation, `tsan-previous/`, which back-to-back seeds overwrote). +- **A pytest session that ends early is caught without an `INTERNALERROR`.** When the + session ran fewer tests than it collected, and not because of `-x` or `--maxfail`, `ci` + exits `3` ("pytest stopped after X of N collected tests"). A JUnit file left by an + earlier run is removed first, so a session that dies before writing one is never read + with the previous run's counts. ### Added diff --git a/docs/ci.md b/docs/ci.md index 70862e3..f457c78 100644 --- a/docs/ci.md +++ b/docs/ci.md @@ -85,8 +85,10 @@ def test_uses_a_signal_handler(): ... or deselect them: `--pytest-arg=-k --pytest-arg="not network"`. Any pytest argument can be passed this way. -**A session that aborts** part-way — a pytest `INTERNALERROR` — exits `3`, not `4`: the -suite did not run, so nothing can be said about it. pytest's own summary line is printed. +**A session that aborts** part-way exits `3`, not `4`: the suite did not run, so nothing +can be said about it. That is a pytest `INTERNALERROR`, or a session that ran fewer tests +than it collected without saying why (`-x` and `--maxfail` say why, and are a failing +suite). pytest's own summary line is printed. **Stripped builds.** A stripped extension makes every frame of yours in a report nameless, so the build overrides both places a project can ask for it: the release diff --git a/python/ftcheck/ci/pipeline.py b/python/ftcheck/ci/pipeline.py index 1d05060..272f8da 100644 --- a/python/ftcheck/ci/pipeline.py +++ b/python/ftcheck/ci/pipeline.py @@ -74,6 +74,7 @@ class Outcome: tests_collected: int = 0 tests_failed: int = 0 tests_errored: int = 0 + tests_skipped: int = 0 pytest_exit: int | None = None @@ -199,6 +200,9 @@ def _uninstrumented_libraries(wheel: pathlib.Path, work: pathlib.Path) -> list[s _COLLECTED = re.compile(r"[Cc]ollected (\d+) items?") +# The session's own count, printed before any test runs: the first line that +# starts with it (a test's captured output can print another). +_SESSION_COLLECTED = re.compile(r"^[Cc]ollected (\d+) items?", re.MULTILINE) _SUMMARY = re.compile(r"^=+ (.*(?:passed|failed|error|no tests ran).*) =+$", re.MULTILINE) @@ -218,6 +222,17 @@ def _read_session(out: Outcome, log: pathlib.Path) -> None: if "INTERNALERROR" in text: of = f" of {max(collected)} collected" if collected else "" out.aborted = f"pytest aborted with an INTERNALERROR after {out.tests_collected} tests{of}" + return + # A session can also end part-way without saying why. -x and --maxfail + # stop on purpose, and pytest prints "stopping after" when they do. + first = _SESSION_COLLECTED.search(text) + if out.pytest_exit not in (0, 1) or not first or "stopping after" in text: + return + # -q prints no bordered summary, so the deselected count is read anywhere. + deselected = re.findall(r"(\d+) deselected", text) + seen = out.tests_collected + out.tests_skipped + (int(deselected[-1]) if deselected else 0) + if seen < int(first[1]): + out.aborted = f"pytest stopped after {seen} of {first[1]} collected tests" def _build_backend(project: pathlib.Path) -> str | None: @@ -284,18 +299,21 @@ def _stripped(path: pathlib.Path) -> bool: return "Section Headers" in sections and ".symtab" not in sections -def _count_tests(junit: pathlib.Path) -> tuple[int, int, int]: - """(collected, failed, errored) from pytest's JUnit XML.""" +def _count_tests(junit: pathlib.Path) -> tuple[int, int, int, int]: + """(collected, failed, errored, skipped) from pytest's JUnit XML. + + `collected` leaves the skipped tests out: they exercised nothing. + """ try: root = ElementTree.parse(junit).getroot() except (OSError, ElementTree.ParseError): - return 0, 0, 0 + return 0, 0, 0, 0 suites = [root] if root.tag == "testsuite" else list(root.iter("testsuite")) tests = sum(int(s.get("tests", 0)) for s in suites) skipped = sum(int(s.get("skipped", 0)) for s in suites) failed = sum(int(s.get("failures", 0)) for s in suites) errors = sum(int(s.get("errors", 0)) for s in suites) - return tests - skipped, failed, errors + return tests - skipped, failed, errors, skipped # Printed True only on a free-threaded build whose GIL came back on import. @@ -726,6 +744,8 @@ def run(env: Environment, opts: Options) -> Outcome: out.stage = "test" junit = work / "pytest-junit.xml" + # A session that dies before writing it must not inherit the last run's counts. + junit.unlink(missing_ok=True) tests, chosen = _select_tests(opts) out.notes.append(f"running {' '.join(tests)} ({chosen})") cmd = [ @@ -744,7 +764,9 @@ def run(env: Environment, opts: Options) -> Outcome: cmd, test_log, cwd=opts.project, env=runtime_env(env, opts, prepared.tsan_dir) ) out.log = _tail(test_log) - out.tests_collected, out.tests_failed, out.tests_errored = _count_tests(junit) + out.tests_collected, out.tests_failed, out.tests_errored, out.tests_skipped = _count_tests( + junit + ) _read_session(out, test_log) collect(out, prepared.tsan_dir, opts.project, test_log.read_text(errors="replace")) diff --git a/tests/test_ci_pipeline.py b/tests/test_ci_pipeline.py index 7fbd61e..9e6c9b4 100644 --- a/tests/test_ci_pipeline.py +++ b/tests/test_ci_pipeline.py @@ -197,3 +197,92 @@ def test_each_run_keeps_its_own_tsan_logs(tmp_path): assert made[-1].name[:8].isdigit() and "Z" in made[-1].name, "named for the UTC time" same = [pipeline._new_tsan_dir(tmp_path / "ci", "") for _ in range(2)] assert same[0] != same[1] and all(p.is_dir() for p in same) + + +def session(tmp_path, log: str, **kw) -> pipeline.Outcome: + path = tmp_path / "pytest.log" + path.write_text(log) + out = pipeline.Outcome(**kw) + pipeline._read_session(out, path) + return out + + +def test_a_session_that_ends_early_without_an_internalerror_is_detected(tmp_path): + """A session that stops part-way without an INTERNALERROR read as a + suite that ran 2 tests and failed one.""" + out = session( + tmp_path, + "Collected 40 items to run in parallel\n" + "==================== 1 passed, 1 failed in 3.21s ====================\n", + tests_collected=2, + pytest_exit=1, + ) + assert out.aborted == "pytest stopped after 2 of 40 collected tests" + + from ftcheck.ci import verdict + from ftcheck.exit_codes import UNAVAILABLE + + out.stage = "done" + assert verdict(environment(), out).code == UNAVAILABLE, "exit 3, not a suite failure" + + +def test_a_complete_session_is_not_called_early(tmp_path): + # Skipped and thread-unsafe tests are in the JUnit count, not in the + # "to run in parallel" count. + assert session( + tmp_path, + "Collected 6 items to run in parallel\n" + "============ 5 passed, 1 skipped, 1 failed in 0.10s ============\n", + tests_collected=6, tests_skipped=1, pytest_exit=1, + ).aborted is None # fmt: skip + # pytest's own count comes before deselection. + assert session( + tmp_path, + "collected 7 items / 1 deselected / 6 selected\n" + "====== 4 passed, 1 skipped, 1 deselected, 1 failed in 0.12s ======\n", + tests_collected=5, tests_skipped=1, pytest_exit=1, + ).aborted is None # fmt: skip + # -x / --maxfail stop on purpose: a failing suite, not an abort. + assert session( + tmp_path, + "Collected 6 items to run in parallel\n" + "!!!!!!!!!!!!!!!!!!!! stopping after 1 failures !!!!!!!!!!!!!!!!!!!!\n" + "==================== 1 passed, 1 failed in 0.07s ====================\n", + tests_collected=2, pytest_exit=1, + ).aborted is None # fmt: skip + + +def test_the_junit_count_keeps_skipped_tests(tmp_path): + junit = tmp_path / "j.xml" + junit.write_text( + '' + "" + ) + assert pipeline._count_tests(junit) == (5, 1, 0, 2) + + +def test_a_previous_runs_junit_is_never_read_as_this_ones(tmp_path, monkeypatch): + """A session killed before pytest wrote its XML must not inherit the counts + of the run before it.""" + from ftcheck.ci import verdict + from ftcheck.ci.environment import Check + from ftcheck.exit_codes import UNAVAILABLE + + o = opts(tmp_path) + o.work_dir.mkdir() + (o.work_dir / "pytest-junit.xml").write_text( + '' + ) + prepared = pipeline.Prepared(python="python", wheel=tmp_path / "x.whl", tsan_dir=tmp_path) + monkeypatch.setattr(pipeline, "prepare", lambda *a, **k: prepared) + + def died_quietly(cmd, log_path, **kwargs): + log_path.write_text("Collected 3 items to run in parallel\n") + return 0 + + monkeypatch.setattr(pipeline, "_stream", died_quietly) + env = environment() + env.checks.append(Check("x", True, "ok")) + out = pipeline.run(env, o) + assert out.tests_collected == 0 + assert verdict(env, out).code == UNAVAILABLE