diff --git a/CHANGELOG.md b/CHANGELOG.md index 54d37ba..f2f76e0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,29 @@ (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`. +- **`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. +- **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 + +- **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 diff --git a/docs/ci.md b/docs/ci.md index 837c449..f457c78 100644 --- a/docs/ci.md +++ b/docs/ci.md @@ -39,13 +39,19 @@ $ 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 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 @@ -57,9 +63,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 @@ -73,12 +85,16 @@ 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.** 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. @@ -91,13 +107,17 @@ 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 | | |---|---|---| | `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..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: @@ -248,6 +263,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 @@ -257,21 +295,98 @@ 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, int]: + """(collected, failed, errored, skipped) from pytest's JUnit XML. -def _count_tests(junit: pathlib.Path) -> tuple[int, int, int]: - """(collected, failed, errored) 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. +_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(): + return True + _log(f"creating {venv.name}") + cmd = [env.interpreter["executable"], "-m", "venv", str(venv)] + 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 @@ -284,13 +399,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) @@ -320,12 +440,45 @@ 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. "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: @@ -362,34 +515,17 @@ 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) - # 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), - } + # 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 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 = [ @@ -411,7 +547,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}": @@ -429,10 +565,78 @@ 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) -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 +644,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 +667,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 +744,10 @@ 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 ["."]) + # 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 = [ prepared.python, "-m", "pytest", *tests, f"--parallel-threads={opts.threads}", @@ -548,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/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/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/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 new file mode 100644 index 0000000..9e6c9b4 --- /dev/null +++ b/tests/test_ci_pipeline.py @@ -0,0 +1,288 @@ +# 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 + + +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"], + ) + + + +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" + + +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 + + +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() + + +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) + + +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