diff --git a/.github/actions/generate-coverage/README.md b/.github/actions/generate-coverage/README.md index 02dcf711..20c77ca8 100644 --- a/.github/actions/generate-coverage/README.md +++ b/.github/actions/generate-coverage/README.md @@ -14,12 +14,6 @@ installed automatically. If both configuration files are present, coverage is run for each language and the Cobertura reports are merged using `uvx merge-cobertura`. -If a Rust project enables Cranelift — via `.cargo/config.toml`, -`.cargo/config`, or a `[profile.*].codegen-backend` key in `Cargo.toml` — -the action automatically exports `CARGO_PROFILE_DEV_CODEGEN_BACKEND=llvm` -and `CARGO_PROFILE_TEST_CODEGEN_BACKEND=llvm` for the coverage runs so -`cargo llvm-cov` and its child Cargo processes stay on LLVM. - ## Flow ```mermaid @@ -41,6 +35,92 @@ flowchart TD K --> L[End] ``` +## Rust coverage environment propagation + +Figure: sequence diagram showing how `run_rust.py` derives coverage-specific +Cargo environment overrides for Cranelift-configured projects and passes them +into `_run_cargo`, including the optional cucumber.rs follow-up run. Internally +`_run_cargo` starts from the current process environment, removes inherited +codegen-backend-related variables first, and then merges +`get_cargo_coverage_env(manifest_path)` on top so workflow-level Cranelift +exports are not treated as the default coverage behaviour. + +```mermaid +sequenceDiagram + actor GitHubActions + participant run_rust_py as run_rust.py + participant get_cargo_coverage_env + participant _run_cargo + participant env_unsets + participant cargo + + GitHubActions->>run_rust_py: main(manifest_path, fmt, use_nextest, ...) + run_rust_py->>get_cargo_coverage_env: get_cargo_coverage_env(manifest_path) + get_cargo_coverage_env-->>run_rust_py: cargo_env + run_rust_py->>_run_cargo: _run_cargo(args, env_overrides=cargo_env, env_unsets=...) + _run_cargo->>env_unsets: scrub inherited backend vars + alt env_overrides is not None + _run_cargo->>_run_cargo: merge scrubbed os.environ with env_overrides + else + _run_cargo->>_run_cargo: use scrubbed os.environ unchanged + end + _run_cargo->>cargo: invoke cargo llvm-cov with env + cargo-->>_run_cargo: stdout + _run_cargo-->>run_rust_py: stdout + + opt with_cucumber_rs + run_rust_py->>get_cargo_coverage_env: get_cargo_coverage_env(manifest_path) + get_cargo_coverage_env-->>run_rust_py: cargo_env + run_rust_py->>_run_cargo: _run_cargo(cucumber_args, env_overrides=cargo_env, env_unsets=...) + _run_cargo->>env_unsets: scrub inherited backend vars + _run_cargo->>cargo: invoke cargo test with env + cargo-->>_run_cargo: stdout + _run_cargo-->>run_rust_py: stdout + end +``` + +## Cranelift codegen backend support + +The action automatically detects when a Rust repository configures the +Cranelift codegen backend in `.cargo/config.toml`, `.cargo/config`, or the +selected `Cargo.toml` profile sections. You do not need to enable a separate +input for this behaviour. + +When Cranelift is detected, coverage runs set +`CARGO_PROFILE_DEV_CODEGEN_BACKEND=llvm` and +`CARGO_PROFILE_TEST_CODEGEN_BACKEND=llvm` as environment overrides before +launching `cargo-llvm-cov`. That ensures nested `cargo` processes inherit LLVM, +which is required by `-Cinstrument-coverage`. Normal non-coverage builds are +not changed by the action. + +For a Cranelift-configured repository, the standard coverage invocation is +still enough: + +```yaml +- uses: ./.github/actions/generate-coverage + with: + output-path: coverage.xml + format: cobertura +``` + +```yaml +- uses: leynos/shared-actions/.github/actions/generate-coverage@v1 + with: + output-path: coverage.xml + format: cobertura +``` + +Known limitations: + +- Profile sections in the workspace root `Cargo.toml` are only detected when + `cargo-manifest` points to the workspace root manifest. If `cargo-manifest` + points to a workspace member, Cranelift configured solely in the workspace + root manifest profile will not be detected; use `.cargo/config.toml` in that + case. +- Detection uses two approaches: `.cargo/config.toml` and `.cargo/config` + scanning remains text/regex-based, and selected `Cargo.toml` profile + detection also uses a lightweight text scan. + ## Inputs | Name | Description | Required | Default | @@ -74,7 +154,14 @@ supported for Python projects. Mixed projects must use `cobertura`. ## Example ```yaml -- uses: ./.github/actions/generate-coverage@v1 +- uses: ./.github/actions/generate-coverage + with: + output-path: coverage.xml + format: cobertura +``` + +```yaml +- uses: leynos/shared-actions/.github/actions/generate-coverage@v1 with: output-path: coverage.xml format: cobertura @@ -83,7 +170,7 @@ supported for Python projects. Mixed projects must use `cobertura`. For a single feature: ```yaml -- uses: ./.github/actions/generate-coverage@v1 +- uses: ./.github/actions/generate-coverage with: output-path: coverage.xml features: logging @@ -92,7 +179,7 @@ For a single feature: For multiple features: ```yaml -- uses: ./.github/actions/generate-coverage@v1 +- uses: ./.github/actions/generate-coverage with: output-path: coverage.xml features: logging tracing @@ -102,7 +189,7 @@ For multiple features: Comma-separated feature list: ```yaml -- uses: ./.github/actions/generate-coverage@v1 +- uses: ./.github/actions/generate-coverage with: output-path: coverage.xml features: logging,tracing @@ -111,7 +198,7 @@ Comma-separated feature list: Enable ratcheting: ```yaml -- uses: ./.github/actions/generate-coverage@v1 +- uses: ./.github/actions/generate-coverage with: output-path: coverage.xml with-ratchet: true @@ -120,7 +207,7 @@ Enable ratcheting: Enable cucumber-rs: ```yaml -- uses: ./.github/actions/generate-coverage@v1 +- uses: ./.github/actions/generate-coverage with: output-path: coverage.xml with-cucumber-rs: true @@ -131,7 +218,7 @@ Enable cucumber-rs: Disable cargo-nextest: ```yaml -- uses: ./.github/actions/generate-coverage@v1 +- uses: ./.github/actions/generate-coverage with: output-path: coverage.xml use-cargo-nextest: false @@ -140,7 +227,7 @@ Disable cargo-nextest: Use a nested Cargo manifest: ```yaml -- uses: ./.github/actions/generate-coverage@v1 +- uses: ./.github/actions/generate-coverage with: output-path: coverage.xml cargo-manifest: rust-toy-app/Cargo.toml @@ -156,4 +243,8 @@ Coverage reports are archived as workflow artefacts named `-` segment. This prevents collisions across matrix jobs and distinguishes runs on different platforms. +Developer-facing design notes, including the rationale for Cranelift coverage +environment overrides, are available in +[`docs/generate-coverage-design.md`](../../../docs/generate-coverage-design.md). + Release history is available in [CHANGELOG](CHANGELOG.md). diff --git a/.github/actions/generate-coverage/scripts/_cargo_runner.py b/.github/actions/generate-coverage/scripts/_cargo_runner.py new file mode 100644 index 00000000..e38c20bb --- /dev/null +++ b/.github/actions/generate-coverage/scripts/_cargo_runner.py @@ -0,0 +1,434 @@ +"""Cargo subprocess plumbing for the generate-coverage action. + +This module encapsulates spawning ``cargo`` subprocesses with live console +streaming and captured stdout. It provides: + +- Environment construction (``_build_cargo_env``) — copies ``os.environ``, + applies ``env_unsets``, then merges ``env_overrides``. +- Output pumping — selector-based on POSIX (``_pump_cargo_output``) and + thread-based on Windows (``_pump_cargo_output_windows``), both bounded by + ``RUN_RUST_CARGO_WAIT_TIMEOUT``. +- Process lifecycle helpers — stream assertion, timeout enforcement, and + cleanup (``_assert_cargo_streams``, ``_raise_cargo_timeout``, + ``_wait_for_cargo``, ``_kill_cargo_process``). +- The primary entry point ``_run_cargo``, which orchestrates all of the + above and returns captured stdout as a string. + +All public symbols in this module are prefixed with ``_`` and are intended +for use only by ``run_rust.py``. +""" + +from __future__ import annotations + +import contextlib +import dataclasses +import os +import selectors +import shlex +import subprocess +import sys +import threading +import time +import traceback +import typing as typ + +import typer +from plumbum.cmd import cargo + + +@dataclasses.dataclass +class _CargoProcCtx: + """Cargo process handle together with its timing constraints.""" + + proc: subprocess.Popen[str] + deadline: float + wait_timeout: float + + +def _safe_close_text_stream(stream: typ.TextIO | None) -> None: + """Close ``stream`` while suppressing any cleanup errors.""" + if stream is None: + return + with contextlib.suppress(Exception): + stream.close() + + +def _is_debug_pump_enabled() -> bool: + """Return ``True`` if cargo pump-thread debug logging is requested.""" + return os.environ.get("RUN_RUST_DEBUG") == "1" or bool(os.environ.get("DEBUG_UTF8")) + + +def _pump_stream_thread( + src: typ.IO[str], + *, + to_stdout: bool, + stdout_lines: list[str], + thread_exceptions: list[Exception], +) -> None: + """Read lines from *src*, echo them, and capture stdout lines. + + Thread exceptions are appended to *thread_exceptions* rather than + propagated so that the monitoring loop can handle them. + """ + dest = sys.stdout if to_stdout else sys.stderr + try: + for line in iter(src.readline, ""): + dest.write(line) + dest.flush() + if to_stdout: + stdout_lines.append(line.rstrip("\r\n")) + except Exception as exc: # noqa: BLE001 + thread_exceptions.append(exc) + if _is_debug_pump_enabled(): + sys.stderr.write(f"Exception in pump thread: {exc}\n") + sys.stderr.write(traceback.format_exc()) + + +def _poll_pump_loop_iteration( + threads: list[threading.Thread], + ctx: _CargoProcCtx, + thread_exceptions: list[Exception], +) -> bool: + """Perform one monitoring iteration; return ``True`` when pumping is done.""" + if thread_exceptions: + with contextlib.suppress(Exception): + ctx.proc.kill() + return True + if not any(t.is_alive() for t in threads): + return True + remaining = max(0.0, ctx.deadline - time.monotonic()) + if remaining <= 0: + _raise_cargo_timeout(ctx.proc, wait_timeout=ctx.wait_timeout) + join_timeout = min(0.1, remaining) + for thread in threads: + thread.join(timeout=join_timeout) + return False + + +def _finalise_pump_threads( + threads: list[threading.Thread], + proc: subprocess.Popen[str], + thread_exceptions: list[Exception], +) -> None: + """Join threads, kill on timeout, and re-raise any captured thread exception.""" + timed_out = False + for thread in threads: + thread.join(timeout=5) + if thread.is_alive(): + timed_out = True + if timed_out: + with contextlib.suppress(Exception): + proc.kill() + thread_exceptions.append( + TimeoutError("cargo output pump threads did not terminate in time") + ) + if thread_exceptions: + with contextlib.suppress(Exception): + proc.wait(timeout=5) + raise thread_exceptions[0] + + +def _pump_cargo_output_windows( + stdout_stream: typ.IO[str], + stderr_stream: typ.IO[str], + ctx: _CargoProcCtx, +) -> list[str]: + """Pump cargo output on Windows using background threads.""" + thread_exceptions: list[Exception] = [] + stdout_lines: list[str] = [] + + threads = [ + threading.Thread( + name="cargo-stdout", + target=_pump_stream_thread, + args=(stdout_stream,), + kwargs={ + "to_stdout": True, + "stdout_lines": stdout_lines, + "thread_exceptions": thread_exceptions, + }, + ), + threading.Thread( + name="cargo-stderr", + target=_pump_stream_thread, + args=(stderr_stream,), + kwargs={ + "to_stdout": False, + "stdout_lines": stdout_lines, + "thread_exceptions": thread_exceptions, + }, + ), + ] + for thread in threads: + thread.start() + try: + while not _poll_pump_loop_iteration(threads, ctx, thread_exceptions): + pass + finally: + _finalise_pump_threads(threads, ctx.proc, thread_exceptions) + return stdout_lines + + +def _handle_cargo_output_event( + key: selectors.SelectorKey, + stdout_lines: list[str], + sel: selectors.DefaultSelector, +) -> None: + """Process one selector event from a cargo output stream. + + Unregisters the stream when EOF is reached; otherwise echoes the line + and, for stdout, appends it to *stdout_lines*. + """ + stream = typ.cast("typ.TextIO", key.fileobj) + line = stream.readline() + if not line: + sel.unregister(stream) + return + if key.data == "stdout": + typer.echo(line, nl=False) + stdout_lines.append(line.rstrip("\r\n")) + else: + typer.echo(line, err=True, nl=False) + + +def _kill_cargo_process(proc: subprocess.Popen[str]) -> None: + """Kill *proc* and wait for termination, suppressing any errors.""" + with contextlib.suppress(Exception): + proc.kill() + with contextlib.suppress(Exception): + proc.wait(timeout=5) + + +def _pump_cargo_output_posix( + stdout_stream: typ.IO[str], + stderr_stream: typ.IO[str], + ctx: _CargoProcCtx, +) -> list[str]: + """Pump cargo output on POSIX using a selector-based event loop. + + Parameters + ---------- + stdout_stream: + The captured stdout text stream from the cargo process. + stderr_stream: + The captured stderr text stream from the cargo process. + ctx: + Shared context holding the process handle, deadline, and timeout. + + Returns + ------- + list[str] + Lines collected from stdout (newlines stripped). + """ + stdout_lines: list[str] = [] + sel = selectors.DefaultSelector() + try: + sel.register(stdout_stream, selectors.EVENT_READ, data="stdout") + sel.register(stderr_stream, selectors.EVENT_READ, data="stderr") + while sel.get_map(): + if time.monotonic() >= ctx.deadline: + _raise_cargo_timeout(ctx.proc, wait_timeout=ctx.wait_timeout) + timeout = max(0.0, ctx.deadline - time.monotonic()) + for key, _ in sel.select(timeout): + _handle_cargo_output_event(key, stdout_lines, sel) + except Exception: + _kill_cargo_process(ctx.proc) + raise + finally: + sel.close() + return stdout_lines + + +def _pump_cargo_output( + proc: subprocess.Popen[str], + *, + deadline: float, + wait_timeout: float, +) -> list[str]: + """Pump ``proc`` output streams to console and collect stdout lines. + + Delegates to ``_pump_cargo_output_windows`` on Windows and + ``_pump_cargo_output_posix`` elsewhere. + """ + if proc.stdout is None or proc.stderr is None: # pragma: no cover - defensive + message = ( + "cargo output streams must be captured.\n" + f"proc.stdout: {proc.stdout}\n" + f"proc.stderr: {proc.stderr}\n" + f"proc.args: {getattr(proc, 'args', None)}" + ) + raise RuntimeError(message) + + stdout_stream = proc.stdout + stderr_stream = proc.stderr + + ctx = _CargoProcCtx(proc=proc, deadline=deadline, wait_timeout=wait_timeout) + if os.name == "nt": + return _pump_cargo_output_windows(stdout_stream, stderr_stream, ctx) + return _pump_cargo_output_posix(stdout_stream, stderr_stream, ctx) + + +def _build_cargo_env( + env_overrides: typ.Mapping[str, str] | None, + env_unsets: typ.Iterable[str], +) -> dict[str, str]: + """Return the environment dict for a spawned cargo process. + + Starts from a copy of ``os.environ``, removes every key in + ``env_unsets``, then merges ``env_overrides`` (if provided). + """ + env = dict(os.environ) + for key in env_unsets: + env.pop(key, None) + if env_overrides is not None: + env.update(env_overrides) + return env + + +def _assert_cargo_streams(proc: subprocess.Popen[str]) -> None: + """Raise ``typer.Exit(1)`` if stdout or stderr were not captured. + + Kills and cleans up the process before raising so no resources leak. + """ + if proc.stdout is not None and proc.stderr is not None: + return + missing_streams = [] + if proc.stdout is None: + missing_streams.append("stdout") + if proc.stderr is None: + missing_streams.append("stderr") + missing = ", ".join(missing_streams) + message = f"cargo output streams not captured: missing {missing}" + with contextlib.suppress(Exception): + proc.kill() + with contextlib.suppress(Exception): + proc.wait(timeout=5) + _safe_close_text_stream(typ.cast("typ.TextIO | None", proc.stdout)) + _safe_close_text_stream(typ.cast("typ.TextIO | None", proc.stderr)) + typer.echo(f"::error::{message}", err=True) + raise typer.Exit(1) from None + + +def _raise_cargo_timeout( + proc: subprocess.Popen[str], *, wait_timeout: float +) -> typ.Never: + """Kill ``proc`` and raise ``typer.Exit(1)`` for a cargo timeout.""" + typer.echo( + f"::error::cargo did not exit within {wait_timeout}s; killing", + err=True, + ) + with contextlib.suppress(Exception): + proc.kill() + with contextlib.suppress(Exception): + proc.wait(timeout=5) + raise typer.Exit(1) from None + + +def _wait_for_cargo( + proc: subprocess.Popen[str], *, deadline: float, wait_timeout: float +) -> int: + """Wait for cargo to exit and return its return code. + + Kills the process and raises ``typer.Exit(1)`` if it does not exit + within ``RUN_RUST_CARGO_WAIT_TIMEOUT`` seconds (default 600). + """ + try: + return proc.wait(timeout=max(0.0, deadline - time.monotonic())) + except subprocess.TimeoutExpired: + _raise_cargo_timeout(proc, wait_timeout=wait_timeout) + + +def _spawn_cargo( + command: typ.Any, # noqa: ANN401 + env: dict[str, str], +) -> subprocess.Popen[str]: + """Spawn a ``cargo`` subprocess with the given environment. + + Handles both direct ``popen`` invocation and plumbum machine-env + contexts transparently. + """ + popen_kwargs: dict[str, typ.Any] = { + "stdin": subprocess.DEVNULL, + "stdout": subprocess.PIPE, + "stderr": subprocess.PIPE, + "text": True, + "encoding": "utf-8", + "errors": "replace", + } + machine_env = getattr(getattr(cargo, "machine", None), "env", None) + if machine_env is None: + return command.popen(**popen_kwargs, env=env) + with machine_env(): + machine_env.clear() + machine_env.update(env) + return command.popen(**popen_kwargs) + + +def _run_cargo( + args: list[str], + *, + env_overrides: typ.Mapping[str, str] | None = None, + env_unsets: typ.Iterable[str] = (), +) -> str: + """Run ``cargo`` with ``args`` streaming output and return ``stdout``. + + Builds a subprocess environment by copying ``os.environ``, removing every + key in ``env_unsets``, and — when ``env_overrides`` is not ``None`` — + merging its entries into that copy (overrides take precedence). The + resulting environment is passed to the spawned ``cargo`` process, whose + stdout and stderr are streamed to the current process. + + Parameters + ---------- + args : list[str] + Arguments forwarded verbatim to ``cargo``. + env_overrides : Mapping[str, str] | None, optional + Extra or replacement environment variables. When ``None`` (the + default), the environment is inherited unchanged except for any + ``env_unsets`` removals. + env_unsets : Iterable[str], optional + Variable names to remove from the inherited environment before + ``env_overrides`` are applied. Missing keys are silently ignored. + Unsets are performed before overrides, so ``env_overrides`` can + unconditionally set a variable that may or may not have been + inherited. + + Returns + ------- + str + Captured stdout from the ``cargo`` invocation. + """ + typer.echo(f"$ cargo {shlex.join(args)}") + env = _build_cargo_env(env_overrides, env_unsets) + try: + wait_timeout = float(os.getenv("RUN_RUST_CARGO_WAIT_TIMEOUT", "600")) + except ValueError as exc: + typer.echo( + "::error::RUN_RUST_CARGO_WAIT_TIMEOUT must be a number", + err=True, + ) + raise typer.Exit(1) from exc + deadline = time.monotonic() + wait_timeout + proc = _spawn_cargo(cargo[args], env) + try: + _assert_cargo_streams(proc) + stdout_lines = _pump_cargo_output( + proc, + deadline=deadline, + wait_timeout=wait_timeout, + ) + retcode = _wait_for_cargo( + proc, + deadline=deadline, + wait_timeout=wait_timeout, + ) + if retcode != 0: + typer.echo( + f"cargo {shlex.join(args)} failed with code {retcode}", + err=True, + ) + raise typer.Exit(code=retcode or 1) + return "\n".join(stdout_lines) + finally: + _safe_close_text_stream(proc.stdout) + _safe_close_text_stream(proc.stderr) diff --git a/.github/actions/generate-coverage/scripts/_cranelift.py b/.github/actions/generate-coverage/scripts/_cranelift.py new file mode 100644 index 00000000..252febce --- /dev/null +++ b/.github/actions/generate-coverage/scripts/_cranelift.py @@ -0,0 +1,155 @@ +"""Cranelift codegen-backend detection for the generate-coverage action. + +This module provides lightweight text-based detection of the Cranelift +codegen backend in a Cargo project and computes the environment variable +overrides required to force LLVM during coverage runs. + +Exported symbols: + +- ``_CARGO_COVERAGE_ENV_UNSETS`` — tuple of env-var names to strip from the + inherited environment before applying coverage overrides. +- ``get_cargo_coverage_env(manifest_path)`` — returns a dict of + ``CARGO_PROFILE_*_CODEGEN_BACKEND=llvm`` overrides when Cranelift is + detected; returns an empty dict otherwise. + +Detection strategy (in priority order): + +1. Search upward from the manifest directory for ``.cargo/config.toml`` or + ``.cargo/config`` and scan for ``codegen-backend = "cranelift"``. +2. Parse ``[profile.*]`` sections in the given ``Cargo.toml`` for the same + key. + +Known limitation: when ``manifest_path`` points to a workspace member, +profile settings in the workspace-root ``Cargo.toml`` are not scanned. +Use ``.cargo/config.toml`` at the workspace root to ensure detection in +that case. +""" + +from __future__ import annotations + +import re +import typing as typ + +if typ.TYPE_CHECKING: + from pathlib import Path + +_CARGO_COVERAGE_ENV_UNSETS = ( + "CARGO_PROFILE_DEV_CODEGEN_BACKEND", + "CARGO_PROFILE_TEST_CODEGEN_BACKEND", +) + + +def _cargo_config_contains_cranelift(candidate: Path) -> bool: + """Return ``True`` if *candidate* is a cargo config file that sets Cranelift. + + Returns ``False`` on any read or decode error. + """ + try: + content = candidate.read_text(encoding="utf-8") + except (OSError, UnicodeDecodeError): + return False + return bool( + re.search( + r'^[ \t]*codegen-backend\s*=\s*["\']cranelift["\']', + content, + flags=re.MULTILINE, + ) + ) + + +def _uses_cranelift_backend(manifest_path: Path) -> bool: + """Return ``True`` when the project configures the Cranelift codegen backend. + + Searches from the manifest directory upward for ``.cargo/config.toml`` + (or ``.cargo/config``) and checks whether any profile sets + ``codegen-backend = "cranelift"``. + """ + search_dir = manifest_path.resolve().parent + while True: + for name in ("config.toml", "config"): + candidate = search_dir / ".cargo" / name + if candidate.is_file() and _cargo_config_contains_cranelift(candidate): + return True + parent = search_dir.parent + if parent == search_dir: + break + search_dir = parent + return False + + +def _is_profile_section(section: str) -> bool: + """Return ``True`` if *section* is a Cargo profile section name. + + Matches both the bare ``[profile]`` table and dotted sub-tables such as + ``[profile.dev]`` and ``[profile.release]``. + """ + return section == "profile" or section.startswith("profile.") + + +def _manifest_uses_cranelift_backend(manifest_path: Path) -> bool: + """Return ``True`` when ``manifest_path`` configures Cranelift in profiles. + + Parameters + ---------- + manifest_path : Path + Path to the ``Cargo.toml`` manifest to inspect. + + Returns + ------- + bool + ``True`` if any ``[profile.*]`` section sets + ``codegen-backend = "cranelift"``; ``False`` otherwise. + """ + try: + content = manifest_path.read_text(encoding="utf-8") + except (OSError, UnicodeDecodeError): + return False + in_profile_section = False + for line in content.splitlines(): + stripped = line.strip() + if not stripped or stripped.startswith(("#", "//")): + continue + section_match = re.match(r"^\s*\[(?P
[^\]]+)\]\s*(?:#.*)?$", line) + if section_match is not None: + in_profile_section = _is_profile_section(section_match["section"]) + continue + if in_profile_section and re.match( + r"""^codegen-backend\s*=\s*["']cranelift["']""", + stripped, + ): + return True + return False + + +def get_cargo_coverage_env(manifest_path: Path) -> dict[str, str]: + """Return coverage-specific cargo env overrides for Cranelift projects. + + Detects whether the project identified by *manifest_path* uses the + Cranelift codegen backend (via ``.cargo/config*`` or ``Cargo.toml`` + profile sections). When Cranelift is detected, returns a dict that + forces LLVM for coverage-instrumented builds. Returns an empty dict + for non-Cranelift projects. + + Parameters + ---------- + manifest_path : Path + Path to the ``Cargo.toml`` manifest for the crate or workspace + root being instrumented. When this points to a workspace member, + only that member's manifest is scanned; the workspace-root profile + is not inspected via this path. + + Returns + ------- + dict[str, str] + ``{"CARGO_PROFILE_DEV_CODEGEN_BACKEND": "llvm", + "CARGO_PROFILE_TEST_CODEGEN_BACKEND": "llvm"}`` when Cranelift is + detected; ``{}`` otherwise. + """ + if not _uses_cranelift_backend( + manifest_path + ) and not _manifest_uses_cranelift_backend(manifest_path): + return {} + return { + "CARGO_PROFILE_DEV_CODEGEN_BACKEND": "llvm", + "CARGO_PROFILE_TEST_CODEGEN_BACKEND": "llvm", + } diff --git a/.github/actions/generate-coverage/scripts/run_rust.py b/.github/actions/generate-coverage/scripts/run_rust.py index 4fd7fb26..72336570 100644 --- a/.github/actions/generate-coverage/scripts/run_rust.py +++ b/.github/actions/generate-coverage/scripts/run_rust.py @@ -17,13 +17,15 @@ import subprocess import sys import threading -import tomllib -import traceback +import time import typing as typ from decimal import ROUND_HALF_UP, Decimal from pathlib import Path +import _cargo_runner import typer +from _cargo_runner import _run_cargo +from _cranelift import _CARGO_COVERAGE_ENV_UNSETS, get_cargo_coverage_env from cmd_utils_loader import run_cmd from coverage_parsers import get_line_coverage_percent_from_lcov from plumbum.cmd import cargo @@ -31,6 +33,7 @@ from shared_utils import read_previous_coverage logger = logging.getLogger(__name__) +_cargo_runner_run_cargo = _run_cargo try: # runtime import for graceful fallback from lxml import etree @@ -100,81 +103,50 @@ """ -_LLVM_CODEGEN_ENV = { - "CARGO_PROFILE_DEV_CODEGEN_BACKEND": "llvm", - "CARGO_PROFILE_TEST_CODEGEN_BACKEND": "llvm", -} - - -def _is_cranelift_backend(value: object) -> bool: - """Return ``True`` when *value* declares the Cranelift codegen backend.""" - if not isinstance(value, str): - return False - return value.strip().casefold() == "cranelift" - - -def _toml_uses_cranelift_backend(content: str) -> bool: - """Return ``True`` when parsed TOML enables Cranelift for dev or test.""" - try: - data = tomllib.loads(content) - except tomllib.TOMLDecodeError: - return False - profile = data.get("profile") - if not isinstance(profile, dict): - return False - for profile_name in ("dev", "test"): - section = profile.get(profile_name) - if not isinstance(section, dict): - continue - if _is_cranelift_backend(section.get("codegen-backend")): - return True - return False - - -def _manifest_uses_cranelift(manifest_path: Path) -> bool: - """Return ``True`` when *manifest_path* declares cranelift in any profile.""" - try: - data = tomllib.loads(manifest_path.read_text(encoding="utf-8")) - except (OSError, UnicodeDecodeError, tomllib.TOMLDecodeError): - return False - profiles = data.get("profile") - if not isinstance(profiles, dict): - return False - return any( - isinstance(section, dict) - and _is_cranelift_backend(section.get("codegen-backend")) - for section in profiles.values() - ) - - -def _uses_cranelift_backend(manifest_path: Path) -> bool: - """Return ``True`` when the project configures the Cranelift codegen backend. - - Checks ``[profile.*].codegen-backend`` in *manifest_path* itself, then - searches from the manifest directory upward for ``.cargo/config.toml`` - (or ``.cargo/config``) and checks whether any profile sets - ``codegen-backend = "cranelift"``. +def _run_cargo( + args: list[str], + *, + env_overrides: typ.Mapping[str, str] | None = None, + env_unsets: typ.Iterable[str] = (), +) -> str: + """Run ``cargo`` with ``args`` streaming output and return ``stdout``. + + Builds a subprocess environment by copying ``os.environ``, removing every + key in ``env_unsets``, and — when ``env_overrides`` is not ``None`` — + merging its entries into that copy (overrides take precedence). The + resulting environment is passed to the spawned ``cargo`` process, whose + stdout and stderr are streamed to the current process. + + Parameters + ---------- + args : list[str] + Arguments forwarded verbatim to ``cargo``. + env_overrides : Mapping[str, str] | None, optional + Extra or replacement environment variables. When ``None`` (the + default), the environment is inherited unchanged except for any + ``env_unsets`` removals. + env_unsets : Iterable[str], optional + Variable names to remove from the inherited environment before + ``env_overrides`` are applied. Missing keys are silently ignored. + Unsets are performed before overrides, so ``env_overrides`` can + unconditionally set a variable that may or may not have been + inherited. + + Returns + ------- + str + Captured stdout from the ``cargo`` invocation. """ - resolved = manifest_path.resolve() - if _manifest_uses_cranelift(resolved): - return True - - search_dir = resolved.parent - while True: - for name in ("config.toml", "config"): - candidate = search_dir / ".cargo" / name - if candidate.is_file(): - try: - content = candidate.read_text(encoding="utf-8") - except (OSError, UnicodeDecodeError): - continue - if _toml_uses_cranelift_backend(content): - return True - parent = search_dir.parent - if parent == search_dir: - break - search_dir = parent - return False + _cargo_runner.cargo = cargo + _cargo_runner.selectors = selectors + _cargo_runner.threading = threading + _cargo_runner.time = time + typ.cast("typ.Any", _cargo_runner).typer = typer + return _cargo_runner_run_cargo( + args, + env_overrides=env_overrides, + env_unsets=env_unsets, + ) def get_cargo_coverage_cmd( @@ -187,8 +159,7 @@ def get_cargo_coverage_cmd( use_nextest: bool, ) -> list[str]: """Return the cargo llvm-cov command arguments.""" - args: list[str] = [] - args.append("llvm-cov") + args = ["llvm-cov"] if use_nextest: args.append("nextest") args += ["--manifest-path", str(manifest_path), "--workspace", "--summary-only"] @@ -200,13 +171,6 @@ def get_cargo_coverage_cmd( return args -def get_cargo_coverage_env(manifest_path: Path) -> dict[str, str]: - """Return extra environment overrides needed for coverage runs.""" - if _uses_cranelift_backend(manifest_path): - return dict(_LLVM_CODEGEN_ENV) - return {} - - def extract_percent(output: str) -> str: """Return the coverage percentage extracted from ``output``.""" match = re.search( @@ -221,6 +185,7 @@ def extract_percent(output: str) -> str: def _format_percent(covered: int, total: int) -> str: + """Format coverage counts as a two-decimal-place percentage string.""" pct = Decimal(covered) * Decimal(100) / Decimal(total) return str(pct.quantize(Decimal("0.01"), rounding=ROUND_HALF_UP)) @@ -261,212 +226,6 @@ def get_line_coverage_percent_from_cobertura(xml_file: Path) -> str: return _format_percent(covered, total) -def _safe_close_text_stream(stream: typ.TextIO | None) -> None: - """Close ``stream`` while suppressing any cleanup errors.""" - if stream is None: - return - with contextlib.suppress(Exception): - stream.close() - - -def _pump_cargo_output_windows( - proc: subprocess.Popen[str], - stdout_stream: typ.IO[str], - stderr_stream: typ.IO[str], -) -> list[str]: - """Pump cargo output on Windows using background threads.""" - thread_exceptions: list[Exception] = [] - stdout_lines: list[str] = [] - - def pump(src: typ.IO[str], *, to_stdout: bool) -> None: - dest = sys.stdout if to_stdout else sys.stderr - try: - for line in iter(src.readline, ""): - dest.write(line) - dest.flush() - if to_stdout: - stdout_lines.append(line.rstrip("\r\n")) - except Exception as exc: # noqa: BLE001 - thread_exceptions.append(exc) - if os.environ.get("RUN_RUST_DEBUG") == "1" or os.environ.get("DEBUG_UTF8"): - sys.stderr.write(f"Exception in pump thread: {exc}\n") - sys.stderr.write(traceback.format_exc()) - - threads = [ - threading.Thread( - name="cargo-stdout", - target=pump, - args=(stdout_stream,), - kwargs={"to_stdout": True}, - ), - threading.Thread( - name="cargo-stderr", - target=pump, - args=(stderr_stream,), - kwargs={"to_stdout": False}, - ), - ] - for thread in threads: - thread.start() - while True: - if thread_exceptions: - with contextlib.suppress(Exception): - proc.kill() - break - if not any(t.is_alive() for t in threads): - break - for thread in threads: - thread.join(timeout=0.1) - timed_out = False - for thread in threads: - thread.join(timeout=5) - if thread.is_alive(): - timed_out = True - if timed_out: - with contextlib.suppress(Exception): - proc.kill() - thread_exceptions.append( - TimeoutError("cargo output pump threads did not terminate in time") - ) - if thread_exceptions: - with contextlib.suppress(Exception): - proc.wait(timeout=5) - raise thread_exceptions[0] - - return stdout_lines - - -def _pump_cargo_output(proc: subprocess.Popen[str]) -> list[str]: - """Pump ``proc`` output streams to console and collect stdout lines.""" - if proc.stdout is None or proc.stderr is None: # pragma: no cover - defensive - message = ( - "cargo output streams must be captured.\n" - f"proc.stdout: {proc.stdout}\n" - f"proc.stderr: {proc.stderr}\n" - f"proc.args: {getattr(proc, 'args', None)}" - ) - raise RuntimeError(message) - - stdout_stream = proc.stdout - stderr_stream = proc.stderr - stdout_lines: list[str] = [] - - if os.name == "nt": - return _pump_cargo_output_windows( - proc, - stdout_stream, - stderr_stream, - ) - - sel = selectors.DefaultSelector() - try: - sel.register(stdout_stream, selectors.EVENT_READ, data="stdout") - sel.register(stderr_stream, selectors.EVENT_READ, data="stderr") - - while sel.get_map(): - for key, _ in sel.select(): - stream = typ.cast("typ.TextIO", key.fileobj) - line = stream.readline() - if not line: - sel.unregister(stream) - continue - if key.data == "stdout": - typer.echo(line, nl=False) - stdout_lines.append(line.rstrip("\r\n")) - else: - typer.echo(line, err=True, nl=False) - except Exception: - with contextlib.suppress(Exception): - proc.kill() - with contextlib.suppress(Exception): - proc.wait(timeout=5) - raise - finally: - sel.close() - - return stdout_lines - - -def _build_cargo_command( - extra_env: dict[str, str] | None, -) -> tuple[str, typ.Any]: - """Return *(display_prefix, cargo_cmd)* reflecting *extra_env*.""" - if not extra_env: - return "", cargo - env_prefix = " ".join(f"{k}={shlex.quote(v)}" for k, v in extra_env.items()) + " " - return env_prefix, cargo.with_env(**extra_env) - - -def _abort_on_missing_streams(proc: subprocess.Popen[str]) -> None: - """Kill *proc* and exit with an error if its output streams were not captured.""" - if proc.stdout is not None and proc.stderr is not None: - return - missing_streams = [] - if proc.stdout is None: - missing_streams.append("stdout") - if proc.stderr is None: - missing_streams.append("stderr") - missing = ", ".join(missing_streams) - message = f"cargo output streams not captured: missing {missing}" - with contextlib.suppress(Exception): - proc.kill() - with contextlib.suppress(Exception): - proc.wait(timeout=5) - _safe_close_text_stream(typ.cast("typ.TextIO | None", proc.stdout)) - _safe_close_text_stream(typ.cast("typ.TextIO | None", proc.stderr)) - typer.echo(f"::error::{message}", err=True) - raise typer.Exit(1) from None - - -def _wait_for_cargo( - proc: subprocess.Popen[str], - args: list[str], - wait_timeout: float, -) -> None: - """Wait for *proc* to finish, raising ``typer.Exit`` on timeout or failure.""" - try: - retcode = proc.wait(timeout=wait_timeout) - except subprocess.TimeoutExpired: - typer.echo( - f"::error::cargo did not exit within {wait_timeout}s; killing", - err=True, - ) - with contextlib.suppress(Exception): - proc.kill() - with contextlib.suppress(Exception): - proc.wait(timeout=5) - raise typer.Exit(1) from None - if retcode != 0: - typer.echo( - f"cargo {shlex.join(args)} failed with code {retcode}", - err=True, - ) - raise typer.Exit(code=retcode or 1) - - -def _run_cargo(args: list[str], *, extra_env: dict[str, str] | None = None) -> str: - """Run ``cargo`` with ``args`` streaming output and return ``stdout``.""" - env_prefix, cargo_cmd = _build_cargo_command(extra_env) - typer.echo(f"$ {env_prefix}cargo {shlex.join(args)}") - proc = cargo_cmd[args].popen( - stdin=subprocess.DEVNULL, - stdout=subprocess.PIPE, - stderr=subprocess.PIPE, - text=True, - encoding="utf-8", - errors="replace", - ) - try: - _abort_on_missing_streams(proc) - stdout_lines = _pump_cargo_output(proc) - wait_timeout = float(os.getenv("RUN_RUST_CARGO_WAIT_TIMEOUT", "600")) - _wait_for_cargo(proc, args, wait_timeout) - return "\n".join(stdout_lines) - finally: - _safe_close_text_stream(proc.stdout) - _safe_close_text_stream(proc.stderr) - - def _merge_lcov(base: Path, extra: Path) -> None: """Merge two lcov files ensuring they end with ``end_of_record``.""" try: @@ -497,11 +256,11 @@ def run_cucumber_rs_coverage( features: str, *, manifest_path: Path, + cargo_env: typ.Mapping[str, str], with_default: bool, use_nextest: bool, cucumber_rs_features: str, cucumber_rs_args: str, - extra_env: dict[str, str] | None = None, ) -> None: """Run cucumber.rs coverage and merge results into ``out``.""" cucumber_file = out.with_name(f"{out.stem}.cucumber{out.suffix}") @@ -525,7 +284,11 @@ def run_cucumber_rs_coverage( if cucumber_rs_args: c_args += shlex.split(cucumber_rs_args) - _run_cargo(c_args, extra_env=extra_env) + _run_cargo( + c_args, + env_overrides=cargo_env, + env_unsets=_CARGO_COVERAGE_ENV_UNSETS, + ) if fmt == "cobertura": from plumbum.cmd import uvx @@ -625,12 +388,16 @@ def main( with_default=with_default, use_nextest=use_nextest, ) - extra_env = get_cargo_coverage_env(manifest_path) config_context = ( ensure_nextest_config() if use_nextest else contextlib.nullcontext() ) + cargo_env = get_cargo_coverage_env(manifest_path) with config_context: - stdout = _run_cargo(args, extra_env=extra_env) + stdout = _run_cargo( + args, + env_overrides=cargo_env, + env_unsets=_CARGO_COVERAGE_ENV_UNSETS, + ) if with_cucumber_rs and cucumber_rs_features: run_cucumber_rs_coverage( @@ -638,11 +405,11 @@ def main( fmt, features, manifest_path=manifest_path, + cargo_env=cargo_env, with_default=with_default, use_nextest=use_nextest, cucumber_rs_features=cucumber_rs_features, cucumber_rs_args=cucumber_rs_args, - extra_env=extra_env, ) if fmt == "lcov": percent = get_line_coverage_percent_from_lcov(out) diff --git a/.github/actions/generate-coverage/scripts/tests/test_run_rust_windows.py b/.github/actions/generate-coverage/scripts/tests/test_run_rust_windows.py index 1862c4ff..3bd77c46 100644 --- a/.github/actions/generate-coverage/scripts/tests/test_run_rust_windows.py +++ b/.github/actions/generate-coverage/scripts/tests/test_run_rust_windows.py @@ -4,6 +4,8 @@ import importlib.util import io +import sys +import time import typing as typ from pathlib import Path from types import ModuleType @@ -11,20 +13,23 @@ import pytest from syspath_hack import prepend_to_syspath -MODULE_PATH = Path(__file__).resolve().parent.parent / "run_rust.py" +MODULE_PATH = Path(__file__).resolve().parent.parent / "_cargo_runner.py" SCRIPT_DIR = MODULE_PATH.parent prepend_to_syspath(SCRIPT_DIR) -spec = importlib.util.spec_from_file_location("run_rust_module", MODULE_PATH) +spec = importlib.util.spec_from_file_location("cargo_runner_module", MODULE_PATH) if spec is None or spec.loader is None: # pragma: no cover - defensive import guard - load_error_message = "Unable to load run_rust module for testing" + load_error_message = "Unable to load _cargo_runner module for testing" raise RuntimeError(load_error_message) -run_rust_module = importlib.util.module_from_spec(spec) -if not isinstance(run_rust_module, ModuleType): # pragma: no cover - importlib contract +cargo_runner_module = importlib.util.module_from_spec(spec) +if not isinstance( + cargo_runner_module, ModuleType +): # pragma: no cover - importlib contract type_error_message = "module_from_spec did not return a ModuleType" raise TypeError(type_error_message) -spec.loader.exec_module(run_rust_module) -run_rust = run_rust_module +sys.modules[spec.name] = cargo_runner_module +spec.loader.exec_module(cargo_runner_module) +run_rust = cargo_runner_module class _SupportsKillWait(typ.Protocol): @@ -42,9 +47,9 @@ class _RunRustModule(typ.Protocol): def _pump_cargo_output_windows( self, - proc: _SupportsKillWait, stdout_stream: typ.IO[str], stderr_stream: typ.IO[str], + ctx: object, ) -> list[str]: """Mirror of the helper under test.""" @@ -57,6 +62,9 @@ def __init__(self) -> None: self.killed = False self.wait_timeouts: list[float | None] = [] + def poll(self) -> None: + return None + def kill(self) -> None: # pragma: no cover - exercised only on failure self.killed = True @@ -92,11 +100,10 @@ def test_pump_cargo_output_windows_streams( stdout_stream = io.StringIO(stdout_payload) stderr_stream = io.StringIO(stderr_payload) - lines = run_rust_typed._pump_cargo_output_windows( - dummy_proc, - stdout_stream, - stderr_stream, + ctx = run_rust._CargoProcCtx( + proc=dummy_proc, deadline=time.monotonic() + 1.0, wait_timeout=1.0 ) + lines = run_rust_typed._pump_cargo_output_windows(stdout_stream, stderr_stream, ctx) assert lines == expected assert captured_stdout.getvalue() == stdout_payload diff --git a/.github/actions/generate-coverage/tests/test_scripts.py b/.github/actions/generate-coverage/tests/test_scripts.py index ab5de733..e9ab890b 100644 --- a/.github/actions/generate-coverage/tests/test_scripts.py +++ b/.github/actions/generate-coverage/tests/test_scripts.py @@ -9,7 +9,6 @@ import io import itertools import os -import shutil import sys import typing as typ from pathlib import Path @@ -18,7 +17,7 @@ from plumbum import local from cmd_utils_importer import import_cmd_utils -from test_support.cmd_mox_stub_adapter import DefaultResponse +from test_support.cmd_mox_stub_adapter import Call, DefaultResponse from test_support.plumbum_helpers import run_plumbum_command if typ.TYPE_CHECKING: # pragma: no cover - type hints only @@ -30,12 +29,6 @@ RunResult = import_cmd_utils().RunResult -_LLVM_CODEGEN_ENV = { - "CARGO_PROFILE_DEV_CODEGEN_BACKEND": "llvm", - "CARGO_PROFILE_TEST_CODEGEN_BACKEND": "llvm", -} - - def _exit_code(exc: BaseException) -> int | None: """Extract an exit code from Typer or SystemExit exceptions.""" exit_code = getattr(exc, "exit_code", None) @@ -103,7 +96,10 @@ def _make_fake_cargo( """Return a fake ``cargo`` object yielding the given streams.""" class FakeProc: + """Minimal subprocess test double for cargo invocations.""" + def __init__(self) -> None: + """Initialise the stub with the configured streams.""" self.stdout = ( stdout if hasattr(stdout, "readline") @@ -121,24 +117,51 @@ def __init__(self) -> None: self.killed = False self.waited = False + def poll(self) -> None: + """Report that the process is still running.""" + def kill(self) -> None: + """Record that the process was killed when lifecycle tracking is enabled.""" if track_lifecycle: self.killed = True - def wait(self, timeout: float | None = None) -> int: + def wait( + self, + _timeout: float | None = None, + **_kwargs: float | None, + ) -> int: + """Return the configured return code immediately.""" if track_lifecycle: self.waited = True return returncode class FakeCargo: + """Minimal cargo command stub with plumbum-like behaviour.""" + def __init__(self) -> None: + """Initialise call-tracking state for the fake cargo command.""" self.last_proc: FakeProc | None = None + self.last_popen_kwargs: dict[str, object] | None = None + self.bound_env: dict[str, str] | None = None + + def with_env(self, **env: str) -> FakeCargo: + """Bind environment overrides and return the fake command.""" + self.bound_env = dict(env) + return self def __getitem__(self, _args: list[str]) -> object: + """Return a runner object for the given cargo arguments.""" cargo = self class Runner: + """Minimal runner object exposing ``popen``.""" + def popen(self, **_kw: object) -> FakeProc: + """Spawn and return the configured fake process.""" + kwargs = dict(_kw) + if cargo.bound_env is not None: + kwargs["env"] = dict(cargo.bound_env) + cargo.last_popen_kwargs = kwargs proc = FakeProc() cargo.last_proc = proc return proc @@ -148,6 +171,58 @@ def popen(self, **_kw: object) -> FakeProc: return FakeCargo() +class _WindowsPumpTimeoutFakeThread: + """Thread stub that stays alive for a fixed number of joins.""" + + def __init__( + self, + *, + name: str, + target: typ.Callable[..., object], + args: tuple[object, ...], + kwargs: dict[str, object], + ) -> None: + """Initialise the thread stub with ignored thread metadata.""" + _ = (name, target, args, kwargs) + self.join_calls = 0 + + def start(self) -> None: + """Pretend to start the thread.""" + + def is_alive(self) -> bool: + """Report liveness until enough joins have occurred.""" + return self.join_calls < 2 + + def join(self, timeout: float | None = None) -> None: + """Record a join call without blocking.""" + _ = timeout + self.join_calls += 1 + + +class _WindowsPumpTimeoutFakeRunner: + """Runner stub that returns the fake Windows process.""" + + def __init__(self, proc: _WindowsPumpFakeProc) -> None: + """Initialise the runner with the fake process to return.""" + self._proc = proc + + def popen(self, **_kw: object) -> _WindowsPumpFakeProc: + """Return the pre-created fake process.""" + return self._proc + + +class _WindowsPumpTimeoutFakeCargoCommand: + """Cargo command stub that yields the fake Windows runner.""" + + def __init__(self, proc: _WindowsPumpFakeProc) -> None: + """Initialise the cargo command with the fake process.""" + self._proc = proc + + def __getitem__(self, _args: list[str]) -> _WindowsPumpTimeoutFakeRunner: + """Return a runner for the requested cargo arguments.""" + return _WindowsPumpTimeoutFakeRunner(self._proc) + + @dataclasses.dataclass(frozen=True, slots=True) class RustCoverageConfig: """Configuration for rust coverage test runs.""" @@ -171,8 +246,8 @@ def _run_rust_coverage_test( shell_stubs: StubManager, config: RustCoverageConfig, *, - monkeypatch: pytest.MonkeyPatch | None = None, -) -> tuple[list[str], dict[str, str], Path, Path]: + monkeypatch: pytest.MonkeyPatch, +) -> tuple[list[str], Path, Path]: """Run ``run_rust.py`` with shared setup and return cargo argv + paths.""" out = tmp_path / "cov.lcov" gh = tmp_path / "gh.txt" @@ -197,8 +272,7 @@ def _run_rust_coverage_test( "GITHUB_OUTPUT": str(gh), } - if monkeypatch is not None: - monkeypatch.chdir(tmp_path) + monkeypatch.chdir(tmp_path) script = Path(__file__).resolve().parents[1] / "scripts" / "run_rust.py" returncode, stdout, _ = run_script(script, env) @@ -208,12 +282,36 @@ def _run_rust_coverage_test( calls = shell_stubs.calls_of("cargo") assert len(calls) == 1 - return calls[0].argv, calls[0].env, out, gh + return calls[0].argv, out, gh -def test_run_rust_success(tmp_path: Path, shell_stubs: StubManager) -> None: +def _run_rust_coverage_call( + tmp_path: Path, + shell_stubs: StubManager, + config: RustCoverageConfig, + *, + monkeypatch: pytest.MonkeyPatch, +) -> tuple[Call, Path, Path]: + """Run ``run_rust.py`` and return the recorded cargo call + output paths.""" + _cargo_args, out, gh = _run_rust_coverage_test( + tmp_path, + shell_stubs, + config, + monkeypatch=monkeypatch, + ) + calls = shell_stubs.calls_of("cargo") + return calls[0], out, gh + + +def test_run_rust_success( + tmp_path: Path, + shell_stubs: StubManager, + monkeypatch: pytest.MonkeyPatch, +) -> None: """Happy path for ``run_rust.py``.""" - cargo_args, cargo_env, out, gh = _run_rust_coverage_test( + monkeypatch.setenv("CARGO_PROFILE_DEV_CODEGEN_BACKEND", "cranelift") + monkeypatch.setenv("CARGO_PROFILE_TEST_CODEGEN_BACKEND", "cranelift") + cargo_call, out, gh = _run_rust_coverage_call( tmp_path, shell_stubs, RustCoverageConfig( @@ -221,7 +319,9 @@ def test_run_rust_success(tmp_path: Path, shell_stubs: StubManager) -> None: features="fast", with_default_features=False, ), + monkeypatch=monkeypatch, ) + cargo_args = cargo_call.argv expected_args = [ "llvm-cov", "--manifest-path", @@ -236,8 +336,8 @@ def test_run_rust_success(tmp_path: Path, shell_stubs: StubManager) -> None: str(out), ] assert cargo_args == expected_args - assert cargo_env.get("CARGO_PROFILE_DEV_CODEGEN_BACKEND") is None - assert cargo_env.get("CARGO_PROFILE_TEST_CODEGEN_BACKEND") is None + assert "CARGO_PROFILE_DEV_CODEGEN_BACKEND" not in cargo_call.env + assert "CARGO_PROFILE_TEST_CODEGEN_BACKEND" not in cargo_call.env data = gh.read_text().splitlines() assert f"file={out}" in data @@ -250,7 +350,7 @@ def test_run_rust_nextest_command( monkeypatch: pytest.MonkeyPatch, ) -> None: """``run_rust.py`` uses cargo llvm-cov nextest when enabled.""" - cargo_args, _cargo_env, out, _gh = _run_rust_coverage_test( + cargo_args, out, _gh = _run_rust_coverage_test( tmp_path, shell_stubs, RustCoverageConfig(use_nextest=True), @@ -272,13 +372,16 @@ def test_run_rust_nextest_command( def test_run_rust_uses_detected_manifest_path( - tmp_path: Path, shell_stubs: StubManager + tmp_path: Path, + shell_stubs: StubManager, + monkeypatch: pytest.MonkeyPatch, ) -> None: """Detected manifest path is propagated to cargo llvm-cov.""" - cargo_args, _cargo_env, _out, _gh = _run_rust_coverage_test( + cargo_args, _out, _gh = _run_rust_coverage_test( tmp_path, shell_stubs, RustCoverageConfig(use_nextest=False, manifest_path="rust-toy-app/Cargo.toml"), + monkeypatch=monkeypatch, ) assert "llvm-cov" in cargo_args mp_idx = cargo_args.index("--manifest-path") @@ -354,13 +457,17 @@ def _run_rust_main_variant( output = tmp_path / "cov.lcov" output.write_text("LF:10\nLH:10\n") github_output = tmp_path / "gh.txt" - recorded_args: list[str] = [] + recorded: dict[str, object] = {} def fake_run_cargo( - args: list[str], *, extra_env: dict[str, str] | None = None + args: list[str], + *, + env_overrides: typ.Mapping[str, str] | None = None, + env_unsets: typ.Iterable[str] = (), ) -> str: - recorded_args[:] = args - _ = extra_env + recorded["args"] = args + recorded["env_overrides"] = dict(env_overrides or {}) + recorded["env_unsets"] = tuple(env_unsets) return "Coverage: 100%" monkeypatch.setattr(run_rust_module, "_run_cargo", fake_run_cargo) @@ -379,7 +486,7 @@ def fake_run_cargo( with_cucumber_rs=False, baseline_file=None, ) - return recorded_args, github_output, output + return typ.cast("list[str]", recorded["args"]), github_output, output @pytest.mark.parametrize("use_nextest", [True, False]) @@ -412,172 +519,125 @@ def test_run_rust_main_nextest_variants( assert "Cargo.toml" in args -def test_run_rust_cranelift_project_uses_llvm_codegen( +def test_run_rust_cranelift_project_uses_llvm_codegen_env( tmp_path: Path, shell_stubs: StubManager, monkeypatch: pytest.MonkeyPatch, ) -> None: - """Coverage uses environment overrides for Cranelift-configured projects.""" - fixture_dir = ( - Path(__file__).resolve().parent / "fixtures" / "nightly-cranelift-project" + """Coverage exports LLVM env overrides for Cranelift-configured repos. + + When a Rust project uses the Cranelift codegen backend (configured in + .cargo/config.toml), the coverage action must still invoke cargo with + environment overrides that force nested cargo invocations spawned by + cargo-llvm-cov back onto LLVM, because source-based code coverage + (-C instrument-coverage) is an LLVM-only feature. + + In a real-world scenario, the Cranelift component would be installed via + ``rustup component add rustc-codegen-cranelift-preview``. + """ + # Simulate a Cranelift-configured project + cargo_config_dir = tmp_path / ".cargo" + cargo_config_dir.mkdir() + (cargo_config_dir / "config.toml").write_text( + "[unstable]\ncodegen-backend = true\n\n" + '[profile.dev]\ncodegen-backend = "cranelift"\n\n' + '[profile.test]\ncodegen-backend = "cranelift"\n', ) - shutil.copytree(fixture_dir, tmp_path, dirs_exist_ok=True) - cargo_args, cargo_env, _out, _gh = _run_rust_coverage_test( + cargo_call, _out, _gh = _run_rust_coverage_call( tmp_path, shell_stubs, RustCoverageConfig(use_nextest=True), monkeypatch=monkeypatch, ) - assert cargo_args[:2] == ["llvm-cov", "nextest"] - assert "--config" not in cargo_args - for key, value in _LLVM_CODEGEN_ENV.items(): - assert cargo_env[key] == value - - -def test_get_cargo_coverage_env_detects_cranelift_fixture( - run_rust_module: ModuleType, -) -> None: - """Cranelift fixtures resolve to cargo environment overrides.""" - fixture_dir = ( - Path(__file__).resolve().parent / "fixtures" / "nightly-cranelift-project" - ) - env = run_rust_module.get_cargo_coverage_env(fixture_dir / "Cargo.toml") - assert env == _LLVM_CODEGEN_ENV + assert cargo_call.argv[:2] == ["llvm-cov", "nextest"] + assert "--config" not in cargo_call.argv + assert cargo_call.env["CARGO_PROFILE_DEV_CODEGEN_BACKEND"] == "llvm" + assert cargo_call.env["CARGO_PROFILE_TEST_CODEGEN_BACKEND"] == "llvm" -def test_uses_cranelift_backend_detects_manifest_fixture( - run_rust_module: ModuleType, +def test_get_cargo_coverage_env_cranelift_variants( + tmp_path: Path, run_rust_module: ModuleType ) -> None: - """Cargo.toml profile settings alone trigger Cranelift detection.""" - fixture_dir = ( - Path(__file__).resolve().parent / "fixtures" / "cargo-toml-cranelift-project" - ) - assert run_rust_module._uses_cranelift_backend(fixture_dir / "Cargo.toml") is True - - -def test_get_cargo_coverage_env_non_cranelift_is_empty( - run_rust_module: ModuleType, tmp_path: Path -) -> None: - """Non-Cranelift projects do not receive extra cargo env overrides.""" + """Config or manifest Cranelift settings get overrides; LLVM repos do not.""" + cargo_config_dir = tmp_path / ".cargo" + cargo_config_dir.mkdir() + cargo_config = cargo_config_dir / "config.toml" manifest_path = tmp_path / "Cargo.toml" - manifest_path.write_text("[package]\nname='demo'\nversion='0.1.0'\n") - assert run_rust_module.get_cargo_coverage_env(manifest_path) == {} + manifest_path.write_text("[package]\nname = 'demo'\nversion = '0.1.0'\n") + cargo_config.write_text( + "[profile.dev]\ncodegen-backend = 'cranelift'\n", + encoding="utf-8", + ) + assert run_rust_module.get_cargo_coverage_env(manifest_path) == { + "CARGO_PROFILE_DEV_CODEGEN_BACKEND": "llvm", + "CARGO_PROFILE_TEST_CODEGEN_BACKEND": "llvm", + } -@pytest.mark.parametrize("profile_name", ["dev", "test"]) -def test_get_cargo_coverage_env_detects_manifest_only_cranelift( - run_rust_module: ModuleType, - tmp_path: Path, - profile_name: str, -) -> None: - """Manifest profile settings alone trigger LLVM codegen env overrides.""" - manifest_path = tmp_path / "Cargo.toml" + cargo_config.write_text( + "[profile.dev]\ncodegen-backend = 'llvm'\n", encoding="utf-8" + ) manifest_path.write_text( - "\n".join( - [ - "[package]", - "name='demo'", - "version='0.1.0'", - f"[profile.{profile_name}]", - 'codegen-backend = " Cranelift "', - "", - ] - ), + "[package]\nname = 'demo'\nversion = '0.1.0'\n\n" + '[profile.dev]\ncodegen-backend = "cranelift"\n', encoding="utf-8", ) + assert run_rust_module.get_cargo_coverage_env(manifest_path) == { + "CARGO_PROFILE_DEV_CODEGEN_BACKEND": "llvm", + "CARGO_PROFILE_TEST_CODEGEN_BACKEND": "llvm", + } - assert run_rust_module._uses_cranelift_backend(manifest_path) is True - assert run_rust_module.get_cargo_coverage_env(manifest_path) == _LLVM_CODEGEN_ENV - - -@dataclasses.dataclass(frozen=True, slots=True) -class _CucumberEnvScenario: - manifest_path: Path - extra_env: dict[str, str] | None - - -def _run_cucumber_coverage_and_capture_env( - run_rust_module: ModuleType, - monkeypatch: pytest.MonkeyPatch, - tmp_path: Path, - scenario: _CucumberEnvScenario, -) -> dict[str, str] | None: - """Stub ``_run_cargo``, invoke ``run_cucumber_rs_coverage``, return captured env.""" - out = tmp_path / "coverage.lcov" - out.write_text("TN:\nend_of_record\n", encoding="utf-8") - captured_env: dict[str, str] | None = None - - def fake_run_cargo( - _args: list[str], *, extra_env: dict[str, str] | None = None - ) -> str: - nonlocal captured_env - captured_env = extra_env - out.with_name(f"{out.stem}.cucumber{out.suffix}").write_text( - "TN:\nend_of_record\n", - encoding="utf-8", - ) - return "" - - monkeypatch.setattr(run_rust_module, "_run_cargo", fake_run_cargo) - run_rust_module.run_cucumber_rs_coverage( - out, - "lcov", - "", - manifest_path=scenario.manifest_path, - with_default=True, - use_nextest=False, - cucumber_rs_features="cucumber", - cucumber_rs_args="", - extra_env=scenario.extra_env, + manifest_path.write_text( + "[package]\nname = 'demo'\nversion = '0.1.0'\n\n" + "[profile.test]\ncodegen-backend = 'cranelift'\n", + encoding="utf-8", ) - return captured_env - + assert run_rust_module.get_cargo_coverage_env(manifest_path) == { + "CARGO_PROFILE_DEV_CODEGEN_BACKEND": "llvm", + "CARGO_PROFILE_TEST_CODEGEN_BACKEND": "llvm", + } -def test_run_cucumber_rs_coverage_passes_extra_env_for_cranelift( - run_rust_module: ModuleType, - monkeypatch: pytest.MonkeyPatch, - tmp_path: Path, -) -> None: - """cucumber.rs coverage forwards Cranelift env overrides to ``_run_cargo``.""" - manifest_path = ( - Path(__file__).resolve().parent - / "fixtures" - / "nightly-cranelift-project" - / "Cargo.toml" + manifest_path.write_text( + "[package]\nname = 'demo'\nversion = '0.1.0'\n\n" + "[profile.dev] # comment\ncodegen-backend = 'cranelift'\n", + encoding="utf-8", ) - captured_env = _run_cucumber_coverage_and_capture_env( - run_rust_module, - monkeypatch, - tmp_path, - _CucumberEnvScenario( - manifest_path=manifest_path, - extra_env=run_rust_module.get_cargo_coverage_env(manifest_path), - ), + assert run_rust_module.get_cargo_coverage_env(manifest_path) == { + "CARGO_PROFILE_DEV_CODEGEN_BACKEND": "llvm", + "CARGO_PROFILE_TEST_CODEGEN_BACKEND": "llvm", + } + + manifest_path.write_text( + "[package]\nname = 'demo'\nversion = '0.1.0'\n\n" + "[profile.dev]\ncodegen-backend = 'llvm'\n", + encoding="utf-8", ) - assert captured_env == _LLVM_CODEGEN_ENV + assert run_rust_module.get_cargo_coverage_env(manifest_path) == {} -def test_run_cucumber_rs_coverage_passes_extra_env_for_non_cranelift( - run_rust_module: ModuleType, - monkeypatch: pytest.MonkeyPatch, - tmp_path: Path, +def test_get_cargo_coverage_env_workspace_member_manifest_cranelift( + tmp_path: Path, run_rust_module: ModuleType ) -> None: - """cucumber.rs coverage forwards explicit non-Cranelift env unchanged.""" - manifest_path = tmp_path / "Cargo.toml" - manifest_path.write_text( - "[package]\nname='demo'\nversion='0.1.0'\n", + """Workspace-root profile-only Cranelift is not detected for members.""" + workspace_manifest = tmp_path / "Cargo.toml" + workspace_manifest.write_text( + '[workspace]\nmembers = ["crates/foo"]\n\n' + "[profile.dev]\ncodegen-backend = 'cranelift'\n", encoding="utf-8", ) - extra_env = {"FOO": "BAR", "BAZ": "QUX"} - captured_env = _run_cucumber_coverage_and_capture_env( - run_rust_module, - monkeypatch, - tmp_path, - _CucumberEnvScenario(manifest_path=manifest_path, extra_env=extra_env), + member_dir = tmp_path / "crates" / "foo" + member_dir.mkdir(parents=True) + member_manifest = member_dir / "Cargo.toml" + member_manifest.write_text( + '[package]\nname = "foo"\nversion = "0.1.0"\n', + encoding="utf-8", ) - assert captured_env == extra_env + + # This is a documented limitation for now: manifest scanning only inspects + # the selected member manifest, not workspace-root profile sections. + assert run_rust_module.get_cargo_coverage_env(member_manifest) == {} def test_nextest_config_is_temporary( @@ -684,14 +744,18 @@ def test_run_cargo_windows_closes_streams( """``_run_cargo`` closes captured streams on success.""" mod = _load_module(monkeypatch, "run_rust") monkeypatch.setattr(mod.os, "name", "nt") - monkeypatch.setattr(mod.typer, "echo", lambda *a, **k: None) + monkeypatch.setattr(mod.typer, "echo", lambda *_args, **_kwargs: None) class TrackingStream(io.StringIO): + """String stream that counts close calls.""" + def __init__(self, value: str) -> None: + """Initialise the stream with the provided text.""" super().__init__(value) self.close_calls = 0 def close(self) -> None: + """Close the stream and record the close call.""" self.close_calls += 1 super().close() @@ -709,6 +773,242 @@ def close(self) -> None: assert stderr.close_calls >= 1 +def test_run_cargo_passes_env_overrides( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """``_run_cargo`` merges env overrides into the spawned cargo process.""" + mod = _load_module(monkeypatch, "run_rust") + monkeypatch.setattr(mod.os, "name", "nt") + monkeypatch.setattr(mod.typer, "echo", lambda *_a, **_k: None) + monkeypatch.setenv("RUN_RUST_INHERITED", "present") + monkeypatch.setenv("CARGO_PROFILE_DEV_CODEGEN_BACKEND", "cranelift") + + fake_cargo = _make_fake_cargo("out-line\n", "err-line\n") + monkeypatch.setattr(mod, "cargo", fake_cargo) + + result = mod._run_cargo( + ["llvm-cov"], + env_unsets=("CARGO_PROFILE_DEV_CODEGEN_BACKEND",), + env_overrides={"CARGO_PROFILE_DEV_CODEGEN_BACKEND": "llvm"}, + ) + + assert result == "out-line" + assert fake_cargo.last_popen_kwargs is not None + env = typ.cast("dict[str, str]", fake_cargo.last_popen_kwargs["env"]) + assert env["RUN_RUST_INHERITED"] == "present" + assert env["CARGO_PROFILE_DEV_CODEGEN_BACKEND"] == "llvm" + + +def test_run_cargo_unix_pump_timeout( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """``_run_cargo`` aborts when the pump loop outlives the wait timeout.""" + mod = _load_module(monkeypatch, "run_rust") + monkeypatch.setattr(mod.os, "name", "posix") + messages: list[tuple[str, bool]] = [] + + def fake_echo(message: str, *, err: bool = False, nl: bool = True) -> None: + _ = nl + messages.append((message, err)) + + monkeypatch.setattr(mod.typer, "echo", fake_echo) + monkeypatch.setenv("RUN_RUST_CARGO_WAIT_TIMEOUT", "1") + + class FakeSelector: + """Selector stub that never yields readable events.""" + + def __init__(self) -> None: + """Initialise the selector with a non-empty registration map.""" + self._map = {"stdout": object()} + + def register(self, fileobj: object, event: object, data: str) -> None: + """Accept a selector registration without storing it.""" + _ = (fileobj, event, data) + + def get_map(self) -> dict[str, object]: + """Return the current selector registration map.""" + return self._map + + def select(self, timeout: float | None = None) -> list[tuple[object, object]]: + """Return no ready events for the given timeout.""" + _ = timeout + return [] + + def close(self) -> None: + """Close the selector stub.""" + + class FakeProc: + """Minimal subprocess test double for timeout handling.""" + + def __init__(self) -> None: + """Initialise the stub with empty captured streams.""" + self.stdout = io.StringIO("") + self.stderr = io.StringIO("") + self.killed = False + self.waited = False + + def poll(self) -> None: + """Report that the process is still running.""" + + def kill(self) -> None: + """Record that the process was killed.""" + self.killed = True + + def wait(self, timeout: float | None = None) -> int: + """Record the wait and return success.""" + _ = timeout + self.waited = True + return 0 + + fake_proc = FakeProc() + + class FakeRunner: + """Runner stub that always returns the fake process.""" + + def popen(self, **_kw: object) -> FakeProc: + """Return the pre-created fake process.""" + return fake_proc + + class FakeCargoCommand: + """Cargo command stub that returns the fake runner.""" + + def __getitem__(self, _args: list[str]) -> FakeRunner: + """Return a runner for the requested cargo arguments.""" + return FakeRunner() + + monkeypatch.setattr(mod, "cargo", FakeCargoCommand()) + monkeypatch.setattr(mod.selectors, "DefaultSelector", FakeSelector) + time_values = iter([0.0, 0.0, 0.5, 1.0]) + monkeypatch.setattr(mod.time, "monotonic", lambda: next(time_values)) + + with pytest.raises(mod.typer.Exit) as excinfo: + mod._run_cargo(["llvm-cov"]) + + assert _exit_code(excinfo.value) == 1 + assert fake_proc.killed + assert fake_proc.waited + assert ("::error::cargo did not exit within 1.0s; killing", True) in messages + + +def test_run_cargo_invalid_timeout_does_not_spawn( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Invalid wait-timeout values fail before cargo is spawned.""" + mod = _load_module(monkeypatch, "run_rust") + messages: list[tuple[str, bool]] = [] + + def fake_echo(message: str, *, err: bool = False, nl: bool = True) -> None: + _ = nl + messages.append((message, err)) + + monkeypatch.setattr(mod.os, "name", "nt") + monkeypatch.setattr(mod.typer, "echo", fake_echo) + monkeypatch.setenv("RUN_RUST_CARGO_WAIT_TIMEOUT", "not-a-float") + + fake_cargo = _make_fake_cargo("out-line\n", "err-line\n") + monkeypatch.setattr(mod, "cargo", fake_cargo) + + with pytest.raises(mod.typer.Exit) as excinfo: + mod._run_cargo(["llvm-cov"]) + + assert _exit_code(excinfo.value) == 1 + assert fake_cargo.last_proc is None + assert ("::error::RUN_RUST_CARGO_WAIT_TIMEOUT must be a number", True) in messages + + +def _make_cucumber_spy( + calls: list[dict[str, object]], +) -> typ.Callable[..., None]: + """Return a ``run_cucumber_rs_coverage`` spy that records its call arguments.""" + + def _spy( + out: Path, + fmt: str, + features: str, + *, + manifest_path: Path, + cargo_env: typ.Mapping[str, str], + with_default: bool, + use_nextest: bool, + cucumber_rs_features: str, + cucumber_rs_args: str, + ) -> None: + calls.append( + { + "out": out, + "fmt": fmt, + "features": features, + "manifest_path": manifest_path, + "cargo_env": dict(cargo_env), + "with_default": with_default, + "use_nextest": use_nextest, + "cucumber_rs_features": cucumber_rs_features, + "cucumber_rs_args": cucumber_rs_args, + } + ) + + return _spy + + +def test_main_reuses_cargo_env_for_cucumber( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, + run_rust_module: ModuleType, +) -> None: + """``main`` computes cargo env once and threads it into cucumber coverage.""" + monkeypatch.chdir(tmp_path) + output = tmp_path / "cov.lcov" + output.write_text("LF:10\nLH:10\n", encoding="utf-8") + github_output = tmp_path / "gh.txt" + cargo_env = {"CARGO_PROFILE_DEV_CODEGEN_BACKEND": "llvm"} + env_calls: list[Path] = [] + run_cargo_env_calls: list[typ.Mapping[str, str] | None] = [] + cucumber_calls: list[dict[str, object]] = [] + + def fake_get_cargo_coverage_env(manifest_path: Path) -> dict[str, str]: + env_calls.append(manifest_path) + return cargo_env + + def fake_run_cargo( + args: list[str], + *, + env_overrides: typ.Mapping[str, str] | None = None, + env_unsets: typ.Iterable[str] = (), + ) -> str: + _ = (args, env_unsets) + run_cargo_env_calls.append(env_overrides) + return "Coverage: 100%" + + cucumber_spy = _make_cucumber_spy(cucumber_calls) + + monkeypatch.setattr( + run_rust_module, "get_cargo_coverage_env", fake_get_cargo_coverage_env + ) + monkeypatch.setattr(run_rust_module, "_run_cargo", fake_run_cargo) + monkeypatch.setattr(run_rust_module, "run_cucumber_rs_coverage", cucumber_spy) + + manifest_path = Path("rust-toy-app/Cargo.toml") + run_rust_module.main( + output, + "", + with_default=True, + use_nextest=False, + lang="rust", + fmt="lcov", + manifest_path=manifest_path, + github_output=github_output, + cucumber_rs_features="tests/features", + cucumber_rs_args="--tag fast", + with_cucumber_rs=True, + baseline_file=None, + ) + + assert env_calls == [manifest_path] + assert run_cargo_env_calls == [cargo_env] + assert len(cucumber_calls) == 1 + assert cucumber_calls[0]["cargo_env"] == cargo_env + + def test_run_cargo_windows_nonzero_exit( monkeypatch: pytest.MonkeyPatch, ) -> None: @@ -717,7 +1017,7 @@ def test_run_cargo_windows_nonzero_exit( mod = _load_module(monkeypatch, "run_rust") monkeypatch.setattr(mod.os, "name", "nt") - monkeypatch.setattr(mod.typer, "echo", lambda *a, **k: None) + monkeypatch.setattr(mod.typer, "echo", lambda *_args, **_kwargs: None) monkeypatch.setattr(mod.typer, "Exit", real_typer.Exit) monkeypatch.setattr( @@ -729,16 +1029,72 @@ def test_run_cargo_windows_nonzero_exit( assert _exit_code(excinfo.value) == 1 +class _WindowsPumpFakeProc: + """Minimal subprocess test double for Windows timeout handling.""" + + def __init__(self) -> None: + """Initialise the stub with captured output streams.""" + self.stdout = io.StringIO("out-line\n") + self.stderr = io.StringIO("err-line\n") + self.killed = False + self.waited = False + + def poll(self) -> None: + """Report that the process is still running.""" + + def kill(self) -> None: + """Record that the process was killed.""" + self.killed = True + + def wait(self, timeout: float | None = None) -> int: + """Record the wait and return success.""" + _ = timeout + self.waited = True + return 0 + + +def test_run_cargo_windows_pump_timeout( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """``_run_cargo`` enforces the wait timeout on Windows pump threads.""" + mod = _load_module(monkeypatch, "run_rust") + monkeypatch.setattr(mod.os, "name", "nt") + messages: list[tuple[str, bool]] = [] + + def fake_echo(message: str, *, err: bool = False, nl: bool = True) -> None: + _ = nl + messages.append((message, err)) + + monkeypatch.setattr(mod.typer, "echo", fake_echo) + monkeypatch.setenv("RUN_RUST_CARGO_WAIT_TIMEOUT", "1") + monkeypatch.setattr(mod.threading, "Thread", _WindowsPumpTimeoutFakeThread) + time_values = iter([0.0, 0.0, 1.0]) + monkeypatch.setattr(mod.time, "monotonic", lambda: next(time_values)) + fake_proc = _WindowsPumpFakeProc() + monkeypatch.setattr(mod, "cargo", _WindowsPumpTimeoutFakeCargoCommand(fake_proc)) + + with pytest.raises(mod.typer.Exit) as excinfo: + mod._run_cargo(["llvm-cov"]) + + assert _exit_code(excinfo.value) == 1 + assert fake_proc.killed + assert fake_proc.waited + assert ("::error::cargo did not exit within 1.0s; killing", True) in messages + + def test_run_cargo_windows_pump_exception( monkeypatch: pytest.MonkeyPatch, ) -> None: """``_run_cargo`` re-raises exceptions from pump threads on Windows.""" mod = _load_module(monkeypatch, "run_rust") monkeypatch.setattr(mod.os, "name", "nt") - monkeypatch.setattr(mod.typer, "echo", lambda *a, **k: None) + monkeypatch.setattr(mod.typer, "echo", lambda *_args, **_kwargs: None) class BoomIO(io.StringIO): + """Stream stub whose ``readline`` always raises.""" + def readline(self) -> str: + """Raise a runtime error when the pump reads a line.""" message = "boom in pump" raise RuntimeError(message) @@ -758,7 +1114,7 @@ def test_run_cargo_windows_none_stdout( """``_run_cargo`` fails when stdout is missing on Windows.""" mod = _load_module(monkeypatch, "run_rust") monkeypatch.setattr(mod.os, "name", "nt") - monkeypatch.setattr(mod.typer, "echo", lambda *a, **k: None) + monkeypatch.setattr(mod.typer, "echo", lambda *_args, **_kwargs: None) fake_cargo = _make_fake_cargo(None, "err-line\n") monkeypatch.setattr(mod, "cargo", fake_cargo) @@ -776,7 +1132,7 @@ def test_run_cargo_windows_none_stderr( """``_run_cargo`` fails when stderr is missing on Windows.""" mod = _load_module(monkeypatch, "run_rust") monkeypatch.setattr(mod.os, "name", "nt") - monkeypatch.setattr(mod.typer, "echo", lambda *a, **k: None) + monkeypatch.setattr(mod.typer, "echo", lambda *_args, **_kwargs: None) fake_cargo = _make_fake_cargo("out-line\n", None) monkeypatch.setattr(mod, "cargo", fake_cargo) @@ -794,14 +1150,18 @@ def test_run_cargo_stream_close_error_suppressed( """Errors closing streams are suppressed during cleanup.""" mod = _load_module(monkeypatch, "run_rust") monkeypatch.setattr(mod.os, "name", "nt") - monkeypatch.setattr(mod.typer, "echo", lambda *a, **k: None) + monkeypatch.setattr(mod.typer, "echo", lambda *_args, **_kwargs: None) class ExplodingStream(io.StringIO): + """String stream that raises after being closed.""" + def __init__(self, value: str) -> None: + """Initialise the stream with the provided text.""" super().__init__(value) self.close_calls = 0 def close(self) -> None: + """Close the stream, then raise a cleanup error.""" self.close_calls += 1 super().close() message = "close failure" @@ -1331,22 +1691,22 @@ def test_merge_cobertura(tmp_path: Path, shell_stubs: StubManager) -> None: @pytest.mark.parametrize( - ("filename", "content"), + ("content", "label"), [ - pytest.param("zero.lcov", "LF:0\nLH:0\n", id="zero-lines"), - pytest.param("empty.lcov", "", id="empty-file"), - pytest.param("missing.lcov", "LF:100\n", id="missing-lh-tag"), - pytest.param("bad.lcov", "LF:abc\nLH:xyz\n", id="malformed"), + ("LF:0\nLH:0\n", "zero_lines"), + ("", "empty_file"), + ("LF:100\n", "missing_lh_tag"), + ("LF:abc\nLH:xyz\n", "malformed_values"), ], ) -def test_lcov_zero_coverage_variants( +def test_lcov_returns_zero_for_degenerate_files( tmp_path: Path, run_rust_module: ModuleType, - filename: str, content: str, + label: str, ) -> None: """``get_line_coverage_percent_from_lcov`` returns 0.00 for degenerate inputs.""" - lcov = tmp_path / filename + lcov = tmp_path / f"{label}.lcov" lcov.write_text(content) assert run_rust_module.get_line_coverage_percent_from_lcov(lcov) == "0.00" diff --git a/docs/generate-coverage-design.md b/docs/generate-coverage-design.md index 0b2fbf1b..8a2b9362 100644 --- a/docs/generate-coverage-design.md +++ b/docs/generate-coverage-design.md @@ -11,8 +11,161 @@ action and the evolution of its supporting scripts. `plumbum`-driven Python subprocess and exposes the composed name to the workflow. The script migrated to `cyclopts` for CLI parsing so additional inputs can be mapped declaratively from the GitHub Actions environment. +- *2026-04-16* — Rust coverage runs now force LLVM via subprocess environment + overrides instead of outer `cargo --config ...` flags when a repository + configures the Cranelift backend. This keeps the action compatible with + `cargo-llvm-cov`, which spawns nested Cargo commands that inherit environment + variables but do not inherit the wrapper process's ad hoc `--config` flags. + +## Rust Coverage Environment Overrides + +### Problem Statement + +Some Rust repositories set: + +```toml +[profile.dev] +codegen-backend = "cranelift" +``` + +to speed up normal local builds. That is a valid repository-level choice, but +it breaks source-based coverage because `-Cinstrument-coverage` only works with +LLVM. Earlier versions of `run_rust.py` tried to work around this by launching +the outer coverage command with: + +```text +cargo --config 'profile.dev.codegen-backend="llvm"' \ + --config 'profile.test.codegen-backend="llvm"' \ + llvm-cov ... +``` + +That looked correct at first glance, but it only affected the wrapper Cargo +process. `cargo-llvm-cov` then spawned nested `cargo test` or `cargo nextest` +processes, and those child processes still read the repository's +`.cargo/config.toml` with `codegen-backend = "cranelift"`. The result was the +same failure the action was supposed to prevent: + +```text +error: `-Cinstrument-coverage` is LLVM specific and not supported by Cranelift +``` + +### Current Design + +`run_rust.py` now splits the behaviour into two explicit pieces: + +1. `get_cargo_coverage_env(manifest_path)` decides whether coverage-specific + Cargo environment overrides are needed. +2. `_run_cargo(args, env_overrides=...)` merges those overrides into the + subprocess environment before invoking Cargo. + +When the target repository does not use Cranelift, `get_cargo_coverage_env()` +returns an empty mapping and the action behaves as before. + +When Cranelift is detected, the helper returns: + +```text +CARGO_PROFILE_DEV_CODEGEN_BACKEND=llvm +CARGO_PROFILE_TEST_CODEGEN_BACKEND=llvm +``` + +These variables are passed to the `cargo llvm-cov` subprocess and therefore +propagate into the nested Cargo commands that perform the actual compilation. + +### Why Environment Variables Instead of `--config` + +The environment-variable approach is intentional rather than incidental: + +- Cargo child processes inherit environment variables by default. +- `cargo-llvm-cov` launches nested Cargo commands internally. +- Those nested commands do not inherit wrapper-only `--config` CLI arguments. +- Coverage therefore needs a transport mechanism that survives process + boundaries. + +Using `CARGO_PROFILE_DEV_CODEGEN_BACKEND` and +`CARGO_PROFILE_TEST_CODEGEN_BACKEND` keeps the override tightly scoped to the +coverage subprocess. The repository's checked-in configuration stays unchanged, +and non-coverage flows continue to use Cranelift if the repository asked for +it. + +### Where the Overrides Apply + +The overrides are applied in both Rust coverage entry points: + +- the main `cargo llvm-cov` run +- the optional cucumber.rs follow-up run when `with-cucumber-rs` is enabled + +Both paths call the same `get_cargo_coverage_env()` helper so the logic does +not drift between the primary and follow-up coverage invocations. + +### Behaviour of `env_overrides` + +`_run_cargo(..., env_overrides=..., env_unsets=...)` accepts two optional +inputs: + +- `env_overrides=None` means "do not add replacement values" +- `env_unsets=()` means "do not remove any inherited keys" +- when `env_overrides` is a mapping, apply these overrides +- when `env_unsets` is an iterable of key names, remove these inherited + variables before applying overrides + +The helper still starts from `os.environ` so it preserves PATH, toolchain +configuration, and the rest of the GitHub Actions runtime context, but it now +explicitly removes inherited `CARGO_PROFILE_DEV_CODEGEN_BACKEND` and +`CARGO_PROFILE_TEST_CODEGEN_BACKEND` before applying coverage overrides. That +prevents a caller or runner host from +leaking an unrelated Cranelift preference into coverage runs. + +### Cranelift Detection Strategy + +Cranelift detection is intentionally lightweight, but it checks two +sources before deciding coverage needs LLVM overrides: + +- `_uses_cranelift_backend(manifest_path)` walks upward from the selected Cargo + manifest directory and scans `.cargo/config.toml` plus `.cargo/config`. +- `_manifest_uses_cranelift_backend(manifest_path)` reads the selected + `Cargo.toml`. +- The selected manifest is scanned for profile sections containing + `codegen-backend = "cranelift"` or the single-quoted equivalent. + +The action therefore catches repository-level Cargo config overrides and +per-manifest profile settings using two lightweight text scans: `.cargo/config*` +detection stays regex-based, while `_manifest_uses_cranelift_backend()` walks +the selected `Cargo.toml` line by line and checks only `[profile]` sections. + +### Known Limitations + +The current detection path is intentionally simple and has limits developers +should understand: + +- `.cargo/config*` detection is text/regex-based rather than + TOML-structure-aware, so any matching `codegen-backend = "cranelift"` + assignment in those files triggers the override. Manifest profile detection + is likewise text-based: `_manifest_uses_cranelift_backend()` scans the + selected `Cargo.toml` for `[profile]` and `[profile.*]` sections before + matching `codegen-backend = "cranelift"` assignments inside them. +- It only inspects `.cargo/config.toml`, `.cargo/config`, and the selected + `Cargo.toml`. It does not model configuration injected via CLI `--config`, + environment-backed Cargo config beyond the explicit dev-profile unset, or + other runtime indirection. +- It always applies the `dev` and `test` profile overrides once Cranelift is + detected. The action currently does not try to mirror per-profile granularity + from the repository config. +- Files that cannot be read as UTF-8 are ignored rather than failing the run, + because the action prefers a conservative fallback over blocking coverage for + an unrelated config parse issue. +- When `cargo-manifest` points to a workspace member, + `_manifest_uses_cranelift_backend` only inspects that member's `Cargo.toml`. + Profile overrides in the workspace root `Cargo.toml` are not scanned via this + path. Use `.cargo/config.toml` at the workspace root to ensure detection + works regardless of which member manifest is selected. + +If the repository ever needs finer-grained handling, the next step would be a +real TOML parser plus table-aware resolution. The current design intentionally +stops short of that complexity. ## Roadmap - [x] Extend artefact naming to include platform metadata and support custom suffixes. +- [x] Document the Rust coverage environment-override design for + Cranelift-configured repositories.