From b87420af0b37eec54b8c2c3dff64444720856e33 Mon Sep 17 00:00:00 2001 From: Karthik Nadig Date: Mon, 28 Sep 2026 12:22:51 -0700 Subject: [PATCH 1/4] test: prove subprocess and production coverage (Fixes #534) Verify exact idle/info child profiles and real handler/transport/writer counter increases. Add conservative production/changed-line diagnostics without altering existing raw coverage gates, and measure native macOS against the exact base on one runner. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .github/workflows/coverage-baseline.yml | 26 +- .github/workflows/coverage-macos.yml | 92 ++++++ .github/workflows/coverage.yml | 21 ++ crates/pet/tests/jsonrpc_server_test.rs | 90 ++++++ docs/QUALITY_SNAPSHOTS.md | 47 ++++ scripts/coverage_detail.py | 354 ++++++++++++++++++++++++ scripts/tests/test_coverage_detail.py | 206 ++++++++++++++ scripts/tests/test_quality_workflows.py | 28 ++ 8 files changed, 863 insertions(+), 1 deletion(-) create mode 100644 .github/workflows/coverage-macos.yml create mode 100644 scripts/coverage_detail.py create mode 100644 scripts/tests/test_coverage_detail.py diff --git a/.github/workflows/coverage-baseline.yml b/.github/workflows/coverage-baseline.yml index 9aeb54ef..372cbd86 100644 --- a/.github/workflows/coverage-baseline.yml +++ b/.github/workflows/coverage-baseline.yml @@ -27,6 +27,9 @@ jobs: steps: - name: Checkout uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + fetch-depth: 0 + persist-credentials: false - name: Set Python to PATH uses: actions/setup-python@5fda3b95a4ea91299a34e894583c3862153e4b97 # v7.0.0 @@ -181,6 +184,21 @@ jobs: env: RUST_BACKTRACE: 1 RUST_LOG: trace + PET_SUBPROCESS_COVERAGE_PROOF: ${{ github.workspace }}/subprocess-coverage.json + shell: bash + + - name: Verify Isolated Server Coverage + if: always() + run: python scripts/coverage_detail.py proof --manifest subprocess-coverage.json --output subprocess-coverage + shell: bash + + - name: Report Production and Changed Coverage + if: always() + run: >- + python scripts/coverage_detail.py report + --lcov lcov.info + --base "HEAD" + --output production-coverage shell: bash - name: Validate Coverage Baseline @@ -194,8 +212,14 @@ jobs: shell: bash - name: Upload Coverage Artifact + if: always() uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 with: name: coverage-baseline-${{ matrix.os }} - path: lcov.info + path: | + lcov.info + coverage-baseline-report.md + production-coverage/ + subprocess-coverage/ + subprocess-coverage.json retention-days: 90 diff --git a/.github/workflows/coverage-macos.yml b/.github/workflows/coverage-macos.yml new file mode 100644 index 00000000..48c784d1 --- /dev/null +++ b/.github/workflows/coverage-macos.yml @@ -0,0 +1,92 @@ +name: Native macOS Coverage + +on: + pull_request: + branches: [main, 'release*', 'release/*', 'release-*'] + push: + branches: [main, 'release*', 'release/*', 'release-*'] + workflow_dispatch: + +permissions: + contents: read + +jobs: + coverage: + runs-on: macos-14 + timeout-minutes: 35 + steps: + - name: Checkout + uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + fetch-depth: 0 + persist-credentials: false + + - name: Set Python to PATH + uses: actions/setup-python@5fda3b95a4ea91299a34e894583c3862153e4b97 # v7.0.0 + with: + python-version: '3.12' + + - name: Rust Tool Chain setup + uses: dtolnay/rust-toolchain@stable + with: + toolchain: stable + components: llvm-tools-preview + + - name: Install cargo-llvm-cov + uses: taiki-e/install-action@cargo-llvm-cov + + - name: Validate Coverage Reporting + run: python -m unittest discover -s scripts/tests -p 'test_*.py' -v + + - name: Collect Native macOS Coverage + run: cargo llvm-cov --workspace --lcov --output-path lcov.info -- --test-threads=1 + env: + PET_SUBPROCESS_COVERAGE_PROOF: ${{ github.workspace }}/subprocess-coverage.json + + - name: Verify Isolated Server Coverage + if: always() + run: python scripts/coverage_detail.py proof --manifest subprocess-coverage.json --output subprocess-coverage + + - name: Report Production and Changed Coverage + if: always() + env: + BASE: ${{ github.event.pull_request.base.sha || github.sha }} + run: python scripts/coverage_detail.py report --lcov lcov.info --base "$BASE" --output production-coverage + + - name: Measure Exact PR Base on the Same Runner + if: github.event_name == 'pull_request' + env: + BASE: ${{ github.event.pull_request.base.sha }} + CARGO_TARGET_DIR: ${{ runner.temp }}/coverage-base-target + run: | + git worktree add --detach "$RUNNER_TEMP/coverage-base" "$BASE" + cd "$RUNNER_TEMP/coverage-base" + cargo llvm-cov --workspace --lcov --output-path "$GITHUB_WORKSPACE/baseline-lcov.info" -- --test-threads=1 + + - name: Compare Exact Base Coverage + if: always() && github.event_name == 'pull_request' + run: >- + python scripts/quality_snapshot.py coverage + --current lcov.info --baseline baseline-lcov.info --platform macOS + --report coverage-report.md --summary "$GITHUB_STEP_SUMMARY" + + - name: Validate Main Coverage + if: github.event_name != 'pull_request' + run: >- + python scripts/quality_snapshot.py coverage + --current lcov.info --baseline lcov.info --platform macOS + --report coverage-report.md --summary "$GITHUB_STEP_SUMMARY" + + - name: Upload Native macOS Coverage + if: always() + uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 + with: + name: coverage-macos-native + path: | + lcov.info + baseline-lcov.info + coverage-report.md + production-coverage/ + subprocess-coverage/ + subprocess-coverage.json + retention-days: 30 diff --git a/.github/workflows/coverage.yml b/.github/workflows/coverage.yml index 1141ca7d..0811878a 100644 --- a/.github/workflows/coverage.yml +++ b/.github/workflows/coverage.yml @@ -32,6 +32,9 @@ jobs: steps: - name: Checkout uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + fetch-depth: 0 + persist-credentials: false - name: Post Coverage Started Comment if: github.event.pull_request.head.repo.full_name == github.repository @@ -200,6 +203,21 @@ jobs: env: RUST_BACKTRACE: 1 RUST_LOG: trace + PET_SUBPROCESS_COVERAGE_PROOF: ${{ github.workspace }}/subprocess-coverage.json + shell: bash + + - name: Verify Isolated Server Coverage + if: always() + run: python scripts/coverage_detail.py proof --manifest subprocess-coverage.json --output subprocess-coverage + shell: bash + + - name: Report Production and Changed Coverage + if: always() + run: >- + python scripts/coverage_detail.py report + --lcov lcov.info + --base "${{ github.event.pull_request.base.sha }}" + --output production-coverage shell: bash - name: Wait for Exact PR Base Coverage @@ -248,6 +266,9 @@ jobs: path: | lcov.info coverage-report.md + production-coverage/ + subprocess-coverage/ + subprocess-coverage.json if-no-files-found: ignore - name: Post Coverage Comment diff --git a/crates/pet/tests/jsonrpc_server_test.rs b/crates/pet/tests/jsonrpc_server_test.rs index 7123928c..49262fa1 100644 --- a/crates/pet/tests/jsonrpc_server_test.rs +++ b/crates/pet/tests/jsonrpc_server_test.rs @@ -38,6 +38,10 @@ struct RawRpcClient { impl RawRpcClient { fn spawn() -> Self { + Self::spawn_with_profile(None) + } + + fn spawn_with_profile(profile: Option<&Path>) -> Self { let mut command = Command::new(env!("CARGO_BIN_EXE_pet")); command .arg("server") @@ -45,6 +49,9 @@ impl RawRpcClient { .stdout(Stdio::piped()) .stderr(Stdio::inherit()); jsonrpc_client::configure_isolated_pet_environment(&mut command); + if let Some(profile) = profile { + command.env("LLVM_PROFILE_FILE", profile); + } let mut child = command.spawn().expect("raw fixture must spawn PET"); let stdout = child.stdout.take().expect("PET stdout must be piped"); let (sender, responses) = mpsc::channel(); @@ -819,6 +826,89 @@ fn invalid_and_oversize_framing_terminates_with_bounded_diagnostics() { } } +#[test] +fn normal_shutdown_records_pid_unique_server_profiles() { + let Some(proof_path) = std::env::var_os("PET_SUBPROCESS_COVERAGE_PROOF") else { + return; + }; + let inherited = PathBuf::from( + std::env::var_os("LLVM_PROFILE_FILE") + .expect("coverage proof requires cargo-llvm-cov instrumentation"), + ); + let directory = inherited.parent().expect("profile must have a directory"); + assert!( + directory.is_absolute(), + "profile directory must be absolute" + ); + assert!( + inherited + .file_name() + .unwrap() + .to_string_lossy() + .contains("%p"), + "cargo-llvm-cov must use PID-unique profiles" + ); + let mut profiles = serde_json::Map::new(); + for (name, request) in [("idle", false), ("info", true)] { + let prefix = format!("pet-proof-{}-{name}-", std::process::id()); + let pattern = directory.join(format!("{prefix}%p-%m.profraw")); + let mut client = RawRpcClient::spawn_with_profile(Some(&pattern)); + let prefix = format!("{prefix}{}-", client.child.id()); + assert!( + fs::read_dir(directory).unwrap().all(|entry| !entry + .unwrap() + .file_name() + .to_string_lossy() + .starts_with(&prefix)), + "child profile must not predate this process exit" + ); + if request { + client.send(json!({"jsonrpc": "2.0", "id": "profile-proof", "method": "info"})); + let reply = client.receive(); + assert_eq!(reply["id"], "profile-proof"); + assert_eq!(reply["result"]["petVersion"], env!("CARGO_PKG_VERSION")); + } + client.child.stdin.take(); + let status = jsonrpc_client::wait_for_exit(&mut client.child, Duration::from_secs(4)) + .expect("profile proof requires graceful exit, not kill fallback"); + assert!( + status.success(), + "profile probe exited unsuccessfully: {status}" + ); + jsonrpc_client::join_reader(client.reader.take().unwrap(), Duration::from_secs(4)).unwrap(); + let raw: Vec = fs::read_dir(directory) + .unwrap() + .map(|entry| entry.unwrap().path()) + .filter(|path| { + path.file_name() + .unwrap() + .to_string_lossy() + .starts_with(&prefix) + }) + .collect(); + assert!( + !raw.is_empty(), + "normal server exit did not flush its own profile" + ); + for path in &raw { + assert!( + fs::metadata(path).unwrap().len() > 0, + "empty server profile" + ); + } + profiles.insert(name.into(), json!(raw)); + } + fs::write( + proof_path, + serde_json::to_vec_pretty(&json!({ + "binary": env!("CARGO_BIN_EXE_pet"), + "profiles": profiles, + })) + .unwrap(), + ) + .expect("write subprocess coverage evidence"); +} + #[test] fn stdin_eof_after_exchange_exits_cleanly_within_one_second() { let client = PetJsonRpcClient::spawn().unwrap(); diff --git a/docs/QUALITY_SNAPSHOTS.md b/docs/QUALITY_SNAPSHOTS.md index 1c8a1c8a..62726e41 100644 --- a/docs/QUALITY_SNAPSHOTS.md +++ b/docs/QUALITY_SNAPSHOTS.md @@ -113,6 +113,53 @@ for the new client clocks. Linux and Windows line and function coverage are compared with the exact base commit. A decrease greater than 0.01 percentage points blocks the pull request. Coverage artifacts and comments remain available for inspection even when the comparison fails. +### Production-focused and subprocess evidence + +The raw workspace percentages still include inline tests and retain the same exact-base +0.01 percentage-point line/function gate. Supplemental `production-coverage/report.md` and +schema-1 `details.json` separate executable production/test lines, list uncovered production +lines, and intersect added/modified Rust lines with executable production lines. Changed files +without instrumentation are listed explicitly, never assumed covered. These diagnostics do not +introduce a fabricated baseline, change the raw denominator, or replace regression protection. + +Classification excludes integration-test/benchmark directories and Rust items explicitly marked +`#[cfg(test)]` or `#[test]`, including inline modules and test-only helper functions. It masks +strings, raw/byte strings, characters, and nested comments before matching item boundaries. +Helpers outside those boundaries and complex conditional attributes remain conservatively in the +production category; this is a source-focused diagnostic, not full Rust conditional-compilation +analysis. Invalid/missing LCOV, missing source, inconsistent hit summaries, and source-line +mismatches fail the reporting step. LLVM summaries can include more entries in `LF`/`LH` +than the unique `DA` source lines (observed in real Windows exports). That deficit is reported +per file (including unmatched summary hits) and conservatively retained as uncovered production, +never dropped from the denominator or silently assigned coverage. +Changed lines without `DA` records are listed separately in JSON, including non-executable syntax; +they are not silently considered covered. + +Every coverage job opts into `normal_shutdown_records_pid_unique_server_profiles` through +`PET_SUBPROCESS_COVERAGE_PROOF`. The test requires cargo-llvm-cov's absolute, PID-unique output +pattern, launches idle and known-`info` PET subprocesses, closes stdin, and requires successful +bounded exit and nonempty profiles for those exact child PIDs. The raw profiles stay in the normal +cargo-llvm-cov collection directory and are included in the workspace report. The verifier also +merges each child's profiles separately with the matching Rust LLVM tools and proves zero idle +versus positive `info` execution at the real handler, transport dispatch, and response writer. +The uploaded `subprocess-coverage/proof.json` and isolated LCOV exports retain that evidence; +a killed child, missing profile, or absent execution witness fails rather than appearing covered. + +Native ARM64 macOS coverage runs workspace default-feature and native process/transport tests, +including Darwin-specific process ownership paths. It intentionally does not compare this workload +with Linux/Windows's installed-manager `ci` workload. For each macOS PR, the exact base revision is +built and measured separately on the same runner with the same compiler and feature selection; +the unchanged line/function comparator gates those comparable artifacts. This works on the first +PR without silently accepting an absent macOS baseline. Main/manual runs publish the native +measurement and proof for inspection. Existing functional macOS installed-manager jobs remain. + +Stable Rust line instrumentation does not provide condition/branch outcomes. Reports show LCOV +`BRDA` totals when supplied, otherwise explicitly report branch data as unavailable (not 100%). +Native malformed-frame/envelope, EOF, broken-output, saturation, and descendant tests provide +behavioral failure-path evidence, but line coverage cannot prove both sides of every condition. +Nightly `cargo llvm-cov --branch` can be used as a separate experiment; its unstable toolchain +and differing denominator are not substituted into the stable cross-platform gate. + ## Running locally The comparator requires Python 3.10 or newer. diff --git a/scripts/coverage_detail.py b/scripts/coverage_detail.py new file mode 100644 index 00000000..82bad008 --- /dev/null +++ b/scripts/coverage_detail.py @@ -0,0 +1,354 @@ +#!/usr/bin/env python3 +# Copyright (c) Microsoft Corporation. +# Licensed under the MIT License. +"""Supplement raw LCOV gates with source-classified lines and subprocess proof.""" + +from __future__ import annotations + +import argparse +import json +import re +import subprocess +import sys +from dataclasses import dataclass +from pathlib import Path + +from quality_snapshot import SnapshotError, parse_lcov + + +def run(*args: str, cwd: Path) -> str: + return subprocess.check_output(args, cwd=cwd, text=True, encoding='utf-8') + + +def relative_source(value: str) -> str: + parts = value.replace('\\', '/').split('/') + try: + index = len(parts) - 1 - parts[::-1].index('crates') + except ValueError as error: + raise SnapshotError(f'Non-workspace LCOV source: {value}') from error + parts = parts[index:] + if any(part in {'', '.', '..'} for part in parts) or not parts[-1].endswith('.rs'): + raise SnapshotError(f'Invalid workspace LCOV source: {value}') + return '/'.join(parts) + + +@dataclass +class SourceCoverage: + lines: dict[int, int] + unmapped: int = 0 + unmapped_hits: int = 0 + + +def line_records(path: Path) -> dict[str, SourceCoverage]: + parse_lcov(path) + records: dict[str, SourceCoverage] = {} + name = None + summaries: dict[str, int] = {} + for text in path.read_text(encoding='utf-8').splitlines(): + if text.startswith('SF:'): + if name is not None: + raise SnapshotError('LCOV record missing terminator') + name = relative_source(text[3:]) + if name in records: + raise SnapshotError(f'Duplicate LCOV source: {name}') + records[name] = SourceCoverage({}) + summaries = {} + elif text.startswith('DA:'): + if name is None: + raise SnapshotError('LCOV line has no source') + fields = text[3:].split(',') + if len(fields) not in (2, 3): + raise SnapshotError('Malformed LCOV line') + number, hits = map(int, fields[:2]) + if number <= 0 or hits < 0 or number in records[name].lines: + raise SnapshotError('Invalid or duplicate LCOV line') + records[name].lines[number] = hits + elif text.startswith(('LF:', 'LH:')): + key, value = text.split(':', 1) + if name is None or key in summaries or int(value) < 0: + raise SnapshotError('Missing source or invalid/duplicate line summary') + summaries[key] = int(value) + elif text == 'end_of_record': + if name is None or summaries.keys() != {'LF', 'LH'}: + raise SnapshotError('LCOV source missing line summaries') + row = records[name] + row.unmapped = summaries['LF'] - len(row.lines) + row.unmapped_hits = summaries['LH'] - sum(count > 0 for count in row.lines.values()) + if row.unmapped < 0 or not 0 <= row.unmapped_hits <= row.unmapped: + raise SnapshotError(f'LCOV line records disagree with summaries: {name}') + # LLVM summaries can count more entries than its unique DA source lines. + # Unlocated entries remain conservatively uncovered in the source-focused report. + name = None + if name is not None or not records or not any(row.lines or row.unmapped for row in records.values()): + raise SnapshotError('LCOV source/line records are incomplete') + return records + + +def code_mask(source: str) -> str: + # Mask Rust literals/comments without moving line or character positions. + chars = list(source) + i = 0 + while i < len(source): + end = i + if source.startswith('//', i): + end = source.find('\n', i) + if end == -1: + end = len(source) + elif source.startswith('/*', i): + depth, end = 1, i + 2 + while depth and end < len(source): + if source.startswith('/*', end): + depth += 1 + end += 2 + elif source.startswith('*/', end): + depth -= 1 + end += 2 + else: + end += 1 + if depth: + raise SnapshotError('Unterminated Rust block comment') + else: + raw = re.match(r'(?:br|cr|r)(#*)"', source[i:]) + if raw: + close = '"' + raw[1] + end = source.find(close, i + raw.end()) + if end == -1: + raise SnapshotError('Unterminated Rust raw string') + end += len(close) + elif source[i] == '"': + end = i + 1 + while end < len(source): + if source[end] == chr(92): + end += 2 + elif source[end] == '"': + end += 1 + break + else: + end += 1 + else: + raise SnapshotError('Unterminated Rust string') + elif source[i] == "'": + char = re.match(r"'(?:[^'\\\n]|\\(?:u\{[0-9a-fA-F_]+\}|x[0-9a-fA-F]{2}|.))'", source[i:]) + if char: + end = i + char.end() + if end > i: + for n in range(i, end): + if chars[n] != '\n': + chars[n] = ' ' + i = end + else: + i += 1 + return ''.join(chars) + + +def test_lines(source: str) -> set[int]: + masked = code_mask(source) + excluded: set[int] = set() + for match in re.finditer(r'#\s*\[\s*(?:cfg\s*\(\s*test\s*\)|test)\s*\]', masked): + start = match.start() + end = match.end() + # Other attributes belong to this same item, not its body. + while True: + attr = re.match(r'\s*#\s*\[', masked[end:]) + if not attr: + break + end += attr.end() + depth = 1 + while depth and end < len(masked): + depth += (masked[end] == '[') - (masked[end] == ']') + end += 1 + if depth: + raise SnapshotError('Unterminated Rust attribute') + delimiters: list[str] = [] + closing = {')': '(', ']': '[', '>': '<', '}': '{'} + initializer = False + while end < len(masked): + char = masked[end] + end += 1 + if char == '>' and end >= 2 and masked[end - 2] == '-': + continue + if not delimiters: + if char == '=': + initializer = True + if char == ';' or (char == '{' and not initializer): + break + type_context = not initializer and not any(c in '[{' for c in delimiters) + if char in '([{' or (char == '<' and type_context): + delimiters.append(char) + elif char in closing and delimiters and delimiters[-1] == closing[char]: + delimiters.pop() + else: + raise SnapshotError('Test attribute has no item') + if masked[end - 1] == '{': + depth = 1 + while depth and end < len(masked): + depth += (masked[end] == '{') - (masked[end] == '}') + end += 1 + if depth: + raise SnapshotError('Unterminated test item') + excluded.update(range(source.count('\n', 0, start) + 1, source.count('\n', 0, end) + 2)) + return excluded + + +def changed_lines(root: Path, base: str) -> dict[str, set[int]]: + text = run('git', 'diff', '--no-ext-diff', '--no-renames', '--unified=0', base, + '--', '*.rs', cwd=root) + changed: dict[str, set[int]] = {} + name = None + for line in text.splitlines(): + if line.startswith('+++ b/'): + name = line[6:] + changed.setdefault(name, set()) + elif line.startswith('+++ '): + name = None + elif line.startswith('@@ '): + hunk = re.match(r'@@ -[0-9]+(?:,[0-9]+)? \+([0-9]+)(?:,([0-9]+))? @@', line) + if not hunk or name is None: + continue + start, length = int(hunk[1]), int(hunk[2] or '1') + changed[name].update(range(start, start + length)) + return changed + + +def summarize(root: Path, records: dict[str, SourceCoverage], changed: dict[str, set[int]]) -> dict: + files = [] + for name, record in sorted(records.items()): + lines = record.lines + source = (root / name).read_text(encoding='utf-8') + if any(n > len(source.splitlines()) for n in lines): + raise SnapshotError(f'LCOV source revision mismatch: {name}') + parts = Path(name).parts + excluded = set(lines) if 'tests' in parts or 'benches' in parts else test_lines(source) + production = {n: count for n, count in lines.items() if n not in excluded} + tests = {n: count for n, count in lines.items() if n in excluded} + edits = production.keys() & changed.get(name, set()) + files.append({ + 'path': name, 'production_found': len(production) + record.unmapped, + 'unmapped_summary_lines': record.unmapped, + 'unmapped_summary_hits': record.unmapped_hits, + 'changed_lines_without_line_records': sorted(changed.get(name, set()) - lines.keys() - excluded), + 'production_hit': sum(n > 0 for n in production.values()), + 'test_found': len(tests), 'test_hit': sum(n > 0 for n in tests.values()), + 'uncovered_production': sorted(n for n, hits in production.items() if not hits), + 'changed_production_found': len(edits), + 'uncovered_changed_production': sorted(n for n in edits if not production[n]), + }) + return {'schema_version': 1, 'files': files, + 'changed_files_without_instrumentation': sorted(name for name, lines in changed.items() + if lines and name not in records)} + + +def branch_counts(path: Path) -> tuple[int, int]: + found = hit = 0 + for line in path.read_text(encoding='utf-8').splitlines(): + if not line.startswith('BRDA:'): + continue + fields = line[5:].split(',') + if len(fields) != 4: + raise SnapshotError('Malformed LCOV branch record') + count = 0 if fields[3] == '-' else int(fields[3]) + if count < 0: + raise SnapshotError('Negative branch count') + found += 1 + hit += count > 0 + return hit, found + + +def details_report(data: dict) -> str: + files = data['files'] + totals = {key: sum(f[key] for f in files) for key in + ('production_hit', 'production_found', 'test_hit', 'test_found', 'changed_production_found')} + uncovered = sum(len(f['uncovered_changed_production']) for f in files) + lines = ['## Production-focused coverage', '', + 'Supplemental schema 1; the whole-workspace exact-base line/function gate is unchanged.', '', + f"Production lines: {totals['production_hit']}/{totals['production_found']}; " + f"test lines: {totals['test_hit']}/{totals['test_found']}.", + f"Changed executable production lines: {totals['changed_production_found'] - uncovered}/" + f"{totals['changed_production_found']} covered.", '', + '| Source | Production hit/total | Test hit/total | Unmapped summary lines | Uncovered changed production lines |', + '| --- | ---: | ---: | ---: | --- |'] + for f in files: + missing = ', '.join(map(str, f['uncovered_changed_production'])) or '-' + lines.append(f"| {f['path']} | {f['production_hit']}/{f['production_found']} | " + f"{f['test_hit']}/{f['test_found']} | {f['unmapped_summary_lines']} | {missing} |") + lines += ['', 'Unmapped LF summary entries are conservatively counted as uncovered production.', + 'Changed lines lacking DA records (including non-executable syntax) are retained in details.json;', + 'they are not assumed covered.', '', + 'Changed Rust files without instrumentation (not assumed covered):'] + lines += [f'- {name}' for name in data['changed_files_without_instrumentation']] or ['- None'] + hit, found = data['branches'] + lines += ['', f'LCOV branches (whole workspace): {hit}/{found}.' if found else + 'Branch outcomes: unavailable from this stable Rust instrumentation; not reported as 100%.', + 'Line hits do not prove both outcomes of a condition. See QUALITY_SNAPSHOTS.md for limits.', ''] + return '\n'.join(lines) + + +def verify_proof(root: Path, manifest: Path, output: Path) -> None: + proof = json.loads(manifest.read_text(encoding='utf-8')) + sysroot = Path(run('rustc', '--print', 'sysroot', cwd=root).strip()) + host = next(line[6:] for line in run('rustc', '-vV', cwd=root).splitlines() if line.startswith('host: ')) + tools = sysroot / 'lib' / 'rustlib' / host / 'bin' + suffix = '.exe' if sys.platform == 'win32' else '' + output.mkdir(parents=True, exist_ok=True) + witnesses = ( + ('handler', 'crates/pet/src/jsonrpc.rs', 'pub fn handle_info('), + ('transport', 'crates/pet-jsonrpc/src/server.rs', 'if handle_payload(handlers, &payload).is_err() {'), + ('writer', 'crates/pet-jsonrpc/src/output.rs', 'if let Err(error) = writer.write_all(&frame) {'), + ) + exports = {} + for label in ('idle', 'info'): + paths = proof['profiles'][label] + if not paths or any(not Path(p).is_file() or Path(p).stat().st_size == 0 for p in paths): + raise SnapshotError(f'Missing or empty {label} subprocess profiles') + merged = output / f'{label}.profdata' + run(str(tools / f'llvm-profdata{suffix}'), 'merge', '-sparse', *paths, '-o', str(merged), cwd=root) + text = run(str(tools / f'llvm-cov{suffix}'), 'export', proof['binary'], + *(str(root / name) for _, name, _ in witnesses), + f'-instr-profile={merged}', '-format=lcov', cwd=root) + lcov = output / f'{label}.info' + lcov.write_text(text, encoding='utf-8') + exports[label] = line_records(lcov) + evidence = {} + for label, name, marker in witnesses: + locations = [i for i, line in enumerate((root / name).read_text(encoding='utf-8').splitlines(), 1) + if marker in line] + if len(locations) != 1: + raise SnapshotError(f'Coverage witness changed: {name}: {marker}') + line = locations[0] + before, after = [exports[k].get(name, SourceCoverage({})).lines.get(line) for k in ('idle', 'info')] + if before != 0 or after is None or after <= 0: + raise SnapshotError(f'{label} not proved by isolated info request: idle={before}, info={after}') + evidence[label] = {'source': name, 'line': line, 'idle_hits': before, 'info_hits': after} + (output / 'proof.json').write_text(json.dumps(evidence, indent=2) + '\n', encoding='utf-8') + + +def main() -> int: + parser = argparse.ArgumentParser(description=__doc__) + parser.add_argument('--root', type=Path, default=Path.cwd()) + sub = parser.add_subparsers(dest='command', required=True) + report = sub.add_parser('report') + report.add_argument('--lcov', type=Path, required=True) + report.add_argument('--base', required=True) + report.add_argument('--output', type=Path, required=True) + proof = sub.add_parser('proof') + proof.add_argument('--manifest', type=Path, required=True) + proof.add_argument('--output', type=Path, required=True) + args = parser.parse_args() + try: + if args.command == 'proof': + verify_proof(args.root, args.manifest, args.output) + else: + base = run('git', 'rev-parse', '--verify', f'{args.base}^{{commit}}', cwd=args.root).strip() + data = summarize(args.root, line_records(args.lcov), changed_lines(args.root, base)) + data.update(base_commit=base, branches=branch_counts(args.lcov)) + args.output.mkdir(parents=True, exist_ok=True) + (args.output / 'details.json').write_text(json.dumps(data, indent=2) + '\n', encoding='utf-8') + (args.output / 'report.md').write_text(details_report(data), encoding='utf-8') + except (OSError, ValueError, KeyError, StopIteration, subprocess.CalledProcessError) as error: + print(f'Coverage evidence failed: {error}', file=sys.stderr) + return 1 + return 0 + + +if __name__ == '__main__': + sys.exit(main()) diff --git a/scripts/tests/test_coverage_detail.py b/scripts/tests/test_coverage_detail.py new file mode 100644 index 00000000..86371805 --- /dev/null +++ b/scripts/tests/test_coverage_detail.py @@ -0,0 +1,206 @@ +# Copyright (c) Microsoft Corporation. +# Licensed under the MIT License. + +import json +import subprocess +import sys +import tempfile +import unittest +from pathlib import Path +from unittest.mock import patch + +sys.path.insert(0, str(Path(__file__).resolve().parents[1])) +import coverage_detail as detail + + +class DetailCoverageTests(unittest.TestCase): + def setUp(self): + self.temp = tempfile.TemporaryDirectory() + self.addCleanup(self.temp.cleanup) + self.root = Path(self.temp.name) + + def lcov(self, text): + path = self.root / 'lcov.info' + path.write_text(text, encoding='utf-8') + return path + + def record(self, name='crates/pet/src/lib.rs', lines='DA:1,1\nDA:2,0\n'): + entries = [line for line in lines.splitlines() if line.startswith('DA:')] + hits = sum(int(line.split(',')[1]) > 0 for line in entries) + return (f'SF:C:\\checkout\\{name}\n{lines}LF:{len(entries)}\nLH:{hits}\n' + 'FNF:1\nFNH:1\nend_of_record\n') + + def test_absolute_platform_paths_share_relative_identity(self): + for name in ['C:\\checkout\\crates\\pet\\src\\lib.rs', '/tmp/base/crates/pet/src/lib.rs', '/tmp/crates/checkout/crates/pet/src/lib.rs']: + self.assertEqual(detail.relative_source(name), 'crates/pet/src/lib.rs') + for name in ['/tmp/other.rs', '/tmp/crates/pet/../secret.rs']: + with self.assertRaises(detail.SnapshotError): + detail.relative_source(name) + + def test_lcov_rejects_missing_empty_malformed_and_duplicate_records(self): + for text in ['', self.record(lines='DA:0,1\n'), self.record(lines='DA:1,-1\n'), + self.record(lines='DA:1,1\nDA:1,2\n'), self.record() * 2, + self.record().replace('end_of_record\n', '')]: + with self.subTest(text=text), self.assertRaises((ValueError, detail.SnapshotError)): + detail.line_records(self.lcov(text)) + self.assertEqual(detail.line_records(self.lcov(self.record())), + {'crates/pet/src/lib.rs': detail.SourceCoverage({1: 1, 2: 0})}) + + def test_lcov_line_summaries_cannot_hide_truncated_or_overcounted_data(self): + valid = self.record() + for text in [valid.replace('LH:1', 'LH:0'), + valid.replace('LF:2', 'LF:1'), + valid.replace('LF:2\n', ''), + valid.replace('LH:1\n', ''), + valid.replace('LF:2', 'LF:2\nLF:2')]: + with self.subTest(text=text), self.assertRaises(detail.SnapshotError): + detail.line_records(self.lcov(text)) + + def test_test_item_types_do_not_terminate_at_nested_semicolons_or_braces(self): + for signature in ['fn helper() -> [u8; 4]', + 'fn helper() -> [u8; { 2 + 2 }]', + 'fn helper() -> Buffer<{ 2 + 2 }>', + 'fn helper(arg: [u8; 4])']: + source = '#[cfg(test)]\n' + signature + ' {\n let value = 4;\n}\nfn real() {}\n' + with self.subTest(signature=signature): + self.assertEqual(detail.test_lines(source), {1, 2, 3, 4}) + + def test_unmapped_llvm_summary_lines_remain_uncovered_production(self): + name = 'crates/pet/src/lib.rs' + path = self.root / name + path.parent.mkdir(parents=True) + path.write_text('fn real() {}\nfn missing() {}\n') + records = detail.line_records(self.lcov(self.record().replace('DA:2,0\n', ''))) + self.assertEqual(records[name].unmapped, 1) + result = detail.summarize(self.root, records, {name: {2}})['files'][0] + self.assertEqual(result['production_found'], 2) + self.assertEqual(result['production_hit'], 1) + self.assertEqual(result['unmapped_summary_lines'], 1) + self.assertEqual(result['changed_lines_without_line_records'], [2]) + records = detail.line_records(self.lcov(self.record().replace('DA:1,1\n', ''))) + self.assertEqual(records[name].unmapped_hits, 1) + result = detail.summarize(self.root, records, {name: {1}})['files'][0] + self.assertEqual(result['production_found'], 2) + self.assertEqual(result['production_hit'], 0) + self.assertEqual(result['changed_lines_without_line_records'], [1]) + + def test_comparison_and_shift_operators_are_not_generic_type_delimiters(self): + for declaration in ['const LESS: bool = 1 < 2;', + 'static SHIFT: usize = 1 << 2;', + 'const BLOCK: bool = { 1 < 2 };', + 'type Array = [bool; { (1 < 2) as usize }];', + 'fn helper() -> Buffer<{ (1 < 2) as usize }> {}', + 'fn helper bool>() {}']: + source = '#[cfg(test)]\n' + declaration + '\nfn real() {}\n' + with self.subTest(declaration=declaration): + self.assertEqual(detail.test_lines(source), {1, 2}) + + def test_inline_tests_do_not_hide_later_production_items(self): + source = 'fn before() {}\n#[cfg(test)]\nmod tests {\n fn check() {}\n}\nfn after() {}\n' + self.assertEqual(detail.test_lines(source), {2, 3, 4, 5}) + + def test_literals_nested_comments_and_attributes_do_not_move_boundaries(self): + source = ('// #[cfg(test)] fake {\n' + 'const TEXT: &str = r###"#[cfg(test)] }"###;\n' + '#[cfg(test)]\n#[allow(dead_code)]\nmod tests {\n' + ' /* outer { /* nested } */ } */\n' + ' let ch = \'{\'; let escaped = "}\\"{";\n' + ' let raw = br##"{{}}"##;\n}\nfn after() {}\n') + self.assertEqual(detail.test_lines(source), set(range(3, 10))) + self.assertEqual(detail.code_mask(source).count('\n'), source.count('\n')) + + def test_cfg_test_function_and_external_module_are_test_code(self): + source = '#[cfg(test)]\nfn helper() {}\n#[cfg(test)]\nmod fixtures;\nfn real() {}\n' + self.assertEqual(detail.test_lines(source), {1, 2, 3, 4}) + self.assertEqual(detail.test_lines("fn real<'a>(x: &'a str) { let c = '\\u{7b}'; }\n"), set()) + + def test_unterminated_source_is_an_error_not_a_smaller_denominator(self): + for source in ['/* never closed', 'let raw = r#"never closed', + '#[cfg(test)] mod tests {', '#[cfg(test)]']: + with self.subTest(source=source), self.assertRaises(detail.SnapshotError): + detail.test_lines(source) + + def test_reports_uncovered_changed_production_and_all_test_lines(self): + name = 'crates/pet/src/lib.rs' + path = self.root / name + path.parent.mkdir(parents=True) + path.write_text('fn real() {}\n#[cfg(test)]\nmod tests {\n fn test() {}\n}\nfn later() {}\n') + integration = 'crates/pet/tests/native.rs' + target = self.root / integration + target.parent.mkdir() + target.write_text('fn native() {}\n') + data = detail.summarize(self.root, {name: detail.SourceCoverage({1: 2, 4: 1, 6: 0}), + integration: detail.SourceCoverage({1: 1})}, + {name: {1, 4, 6}, 'crates/pet/src/unmeasured.rs': {1}}) + row = next(f for f in data['files'] if f['path'] == name) + self.assertEqual((row['production_hit'], row['production_found']), (1, 2)) + self.assertEqual((row['test_hit'], row['test_found']), (1, 1)) + self.assertEqual(row['uncovered_changed_production'], [6]) + self.assertEqual(row['changed_production_found'], 2) + self.assertEqual(data['changed_files_without_instrumentation'], ['crates/pet/src/unmeasured.rs']) + data['branches'] = (0, 0) + report = detail.details_report(data) + self.assertIn('not reported as 100%', report) + self.assertIn('1/2 covered', report) + with self.assertRaises(detail.SnapshotError): + detail.summarize(self.root, {name: detail.SourceCoverage({100: 1})}, {}) + + def test_changed_line_hunks_handle_add_delete_and_rename_without_count_substitution(self): + diff = ('+++ b/crates/pet/src/lib.rs\n@@ -1,0 +2,2 @@\n+x\n+y\n' + '@@ -8 +10 @@\n-x\n+y\n@@ -15,2 +17,0 @@\n-x\n-y\n' + '+++ /dev/null\n@@ -1,4 +0,0 @@\n') + with patch.object(detail, 'run', return_value=diff) as command: + self.assertEqual(detail.changed_lines(self.root, 'exact-base'), + {'crates/pet/src/lib.rs': {2, 3, 10}}) + self.assertIn('--no-renames', command.call_args.args) + self.assertIn('exact-base', command.call_args.args) + + def test_branch_records_are_optional_but_invalid_values_fail(self): + self.assertEqual(detail.branch_counts(self.lcov(self.record())), (0, 0)) + self.assertEqual(detail.branch_counts(self.lcov('BRDA:1,0,0,2\nBRDA:1,0,1,-\n')), (1, 2)) + with self.assertRaises(detail.SnapshotError): + detail.branch_counts(self.lcov('BRDA:1,0,0,-2\n')) + + def test_missing_exact_child_profiles_fail_before_export(self): + manifest = self.root / 'proof.json' + manifest.write_text(json.dumps({'binary': 'pet', 'profiles': {'idle': ['absent']}})) + with patch.object(detail, 'run', side_effect=['/toolchain', 'host: unit-test']), self.assertRaises(detail.SnapshotError): + detail.verify_proof(self.root, manifest, self.root / 'proof') + + def test_info_proof_requires_all_three_positive_differential_witnesses(self): + witnesses = [('crates/pet/src/jsonrpc.rs', 'pub fn handle_info('), + ('crates/pet-jsonrpc/src/server.rs', 'if handle_payload(handlers, &payload).is_err() {'), + ('crates/pet-jsonrpc/src/output.rs', 'if let Err(error) = writer.write_all(&frame) {')] + for name, marker in witnesses: + path = self.root / name + path.parent.mkdir(parents=True, exist_ok=True) + path.write_text(marker + '\n') + raw = self.root / 'child.profraw' + raw.write_bytes(b'nonempty') + manifest = self.root / 'manifest.json' + manifest.write_text(json.dumps({'binary': 'pet', 'profiles': {'idle': [str(raw)], 'info': [str(raw)]}})) + for counts in [(0, 1), (1, 1), (0, 0)]: + with self.subTest(counts=counts): + before = ''.join(self.record(n, f'DA:1,{counts[0]}\n') for n, _ in witnesses) + after = ''.join(self.record(n, f'DA:1,{counts[1]}\n') for n, _ in witnesses) + with patch.object(detail, 'run', side_effect=['/toolchain', 'host: unit-test', '', before, '', after]): + if counts == (0, 1): + detail.verify_proof(self.root, manifest, self.root / 'proof') + evidence = json.loads((self.root / 'proof/proof.json').read_text()) + self.assertEqual(set(evidence), {'handler', 'transport', 'writer'}) + else: + with self.assertRaises(detail.SnapshotError): + detail.verify_proof(self.root, manifest, self.root / 'proof') + + def test_cli_failure_does_not_create_success_report(self): + script = Path(detail.__file__) + result = subprocess.run([sys.executable, str(script), '--root', str(self.root), 'report', + '--lcov', 'missing', '--base', 'absent', '--output', str(self.root / 'out')], + capture_output=True, text=True) + self.assertNotEqual(result.returncode, 0) + self.assertIn('Coverage evidence failed', result.stderr) + self.assertFalse((self.root / 'out/details.json').exists()) + + +if __name__ == '__main__': + unittest.main() diff --git a/scripts/tests/test_quality_workflows.py b/scripts/tests/test_quality_workflows.py index ce85d32b..60b35cd0 100644 --- a/scripts/tests/test_quality_workflows.py +++ b/scripts/tests/test_quality_workflows.py @@ -71,6 +71,34 @@ def test_comment_steps_are_fork_safe_and_non_gating(self) -> None: ), ) + def test_coverage_proof_and_source_reports_are_required_and_uploaded(self) -> None: + for workflow in ("coverage.yml", "coverage-baseline.yml", "coverage-macos.yml"): + with self.subTest(workflow=workflow): + path = ".github/workflows/" + workflow + text = (ROOT / path).read_text(encoding="utf-8") + steps = workflow_steps(path) + self.assertIn("PET_SUBPROCESS_COVERAGE_PROOF:", text) + self.assertIn("fetch-depth: 0", text) + for name in ("Verify Isolated Server Coverage", "Report Production and Changed Coverage"): + self.assertEqual(step_property(steps[name], "if"), "always()") + self.assertIsNone(step_property(steps[name], "continue-on-error")) + upload = next(value for value in steps.values() if "actions/upload-artifact@" in value) + for artifact in ("lcov.info", "production-coverage/", "subprocess-coverage/", "subprocess-coverage.json"): + self.assertIn(artifact, upload) + + def test_macos_coverage_measures_the_exact_base_without_a_schema_bypass(self) -> None: + steps = workflow_steps(".github/workflows/coverage-macos.yml") + measure = steps["Measure Exact PR Base on the Same Runner"] + self.assertIn("github.event.pull_request.base.sha", measure) + self.assertIn("git worktree add --detach", measure) + self.assertIn("cargo llvm-cov --workspace", measure) + self.assertNotIn("--features", measure) + self.assertIn("cargo llvm-cov --workspace", steps["Collect Native macOS Coverage"]) + compare = steps["Compare Exact Base Coverage"] + self.assertIn("--baseline baseline-lcov.info", compare) + self.assertIn("always()", step_property(compare, "if")) + self.assertIsNone(step_property(compare, "continue-on-error")) + def test_report_artifacts_are_uploaded_after_comparison(self) -> None: cases = ( ( From 31ab1171750f4cb663b1f1cf10170510dc1d8e6b Mon Sep 17 00:00:00 2001 From: Karthik Nadig Date: Mon, 28 Sep 2026 13:16:01 -0700 Subject: [PATCH 2/4] fix: retain conservative native LLVM coverage bounds (Refs #534) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .github/skills/rust-coding-skill/SKILL.md | 2 ++ .github/workflows/coverage-macos.yml | 1 + docs/QUALITY_SNAPSHOTS.md | 5 ++++ scripts/coverage_detail.py | 24 ++++++++++++----- scripts/tests/test_coverage_detail.py | 32 +++++++++++++++++++++-- scripts/tests/test_quality_workflows.py | 2 ++ 6 files changed, 57 insertions(+), 9 deletions(-) diff --git a/.github/skills/rust-coding-skill/SKILL.md b/.github/skills/rust-coding-skill/SKILL.md index e89b0e15..fda01626 100644 --- a/.github/skills/rust-coding-skill/SKILL.md +++ b/.github/skills/rust-coding-skill/SKILL.md @@ -102,3 +102,5 @@ Do not execute freshly written scripts as concurrent Unix subprocess fixtures: s For real-pipe EOF/EPIPE tests, create the pipe inside an isolated test subprocess when other test threads spawn children. Unix `CLOEXEC` closes descriptors at exec, not fork: a concurrent child can temporarily retain a reader, allowing the only write to succeed before the final reader disappears. A readiness handshake alone does not prevent this race. Keep the operation's measured deadline separate from setup, and make an outer fixture deadline cover readiness, waits both before and after forced termination, reader joins, and fallback `Drop` cleanup. Use a per-worktree Cargo target directory when validating stacked changes so native fixtures cannot execute another worktree's stale binary. On WSL, run timing-sensitive Linux binaries from the native Linux filesystem rather than a Windows mount, where page faults can stall in filesystem RPC. When launching instrumented PET with `env_clear()`, retain `LLVM_PROFILE_FILE` exactly so child coverage reaches the collector instead of an uncollected default profile. + +LLVM LCOV summaries are not necessarily counts of unique `DA` source entries: a native export can have `LF=191`, `LH=173`, 181 mapped lines, and 177 positive mapped lines. Do not reject valid exports using summary/source equality or silently drop unmapped entries; preserve the raw exact-base gate and follow the conservative supplemental accounting documented in [Quality snapshots](../../../docs/QUALITY_SNAPSHOTS.md#production-focused-and-subprocess-evidence). Prove subprocess collection with exact-PID profiles and isolated idle/request counter differences, not a whole-workspace percentage increase. diff --git a/.github/workflows/coverage-macos.yml b/.github/workflows/coverage-macos.yml index 48c784d1..76ce93cc 100644 --- a/.github/workflows/coverage-macos.yml +++ b/.github/workflows/coverage-macos.yml @@ -58,6 +58,7 @@ jobs: env: BASE: ${{ github.event.pull_request.base.sha }} CARGO_TARGET_DIR: ${{ runner.temp }}/coverage-base-target + PET_SUBPROCESS_COVERAGE_PROOF: ${{ runner.temp }}/base-subprocess-coverage.json run: | git worktree add --detach "$RUNNER_TEMP/coverage-base" "$BASE" cd "$RUNNER_TEMP/coverage-base" diff --git a/docs/QUALITY_SNAPSHOTS.md b/docs/QUALITY_SNAPSHOTS.md index 62726e41..95d6b285 100644 --- a/docs/QUALITY_SNAPSHOTS.md +++ b/docs/QUALITY_SNAPSHOTS.md @@ -134,6 +134,11 @@ per file (including unmatched summary hits) and conservatively retained as uncov never dropped from the denominator or silently assigned coverage. Changed lines without `DA` records are listed separately in JSON, including non-executable syntax; they are not silently considered covered. +Native macOS also demonstrates `LH` below the number of positive unique `DA` entries. Reports +retain this deficit and deduct `max(positive DA + unmapped LF - LH, 0)` from each covered +production/test/changed subtotal (clamped at zero). This accounts for hits that could belong to +unmapped entries instead of a mapped subset. These subtotals are lower bounds; the report does +not pretend to locate the discrepancy on a particular source line. Raw LCOV and the exact-base gate remain intact. Every coverage job opts into `normal_shutdown_records_pid_unique_server_profiles` through `PET_SUBPROCESS_COVERAGE_PROOF`. The test requires cargo-llvm-cov's absolute, PID-unique output diff --git a/scripts/coverage_detail.py b/scripts/coverage_detail.py index 82bad008..dbbdc4c8 100644 --- a/scripts/coverage_detail.py +++ b/scripts/coverage_detail.py @@ -37,6 +37,7 @@ class SourceCoverage: lines: dict[int, int] unmapped: int = 0 unmapped_hits: int = 0 + summary_hit_shortfall: int = 0 def line_records(path: Path) -> dict[str, SourceCoverage]: @@ -73,9 +74,12 @@ def line_records(path: Path) -> dict[str, SourceCoverage]: raise SnapshotError('LCOV source missing line summaries') row = records[name] row.unmapped = summaries['LF'] - len(row.lines) - row.unmapped_hits = summaries['LH'] - sum(count > 0 for count in row.lines.values()) - if row.unmapped < 0 or not 0 <= row.unmapped_hits <= row.unmapped: + hit_difference = summaries['LH'] - sum(count > 0 for count in row.lines.values()) + if (row.unmapped < 0 or summaries['LH'] > summaries['LF'] or + hit_difference > row.unmapped): raise SnapshotError(f'LCOV line records disagree with summaries: {name}') + row.unmapped_hits = max(hit_difference, 0) + row.summary_hit_shortfall = max(-hit_difference, 0) # LLVM summaries can count more entries than its unique DA source lines. # Unlocated entries remain conservatively uncovered in the source-focused report. name = None @@ -222,15 +226,20 @@ def summarize(root: Path, records: dict[str, SourceCoverage], changed: dict[str, production = {n: count for n, count in lines.items() if n not in excluded} tests = {n: count for n, count in lines.items() if n in excluded} edits = production.keys() & changed.get(name, set()) + uncertainty = record.unmapped + record.summary_hit_shortfall - record.unmapped_hits files.append({ 'path': name, 'production_found': len(production) + record.unmapped, 'unmapped_summary_lines': record.unmapped, 'unmapped_summary_hits': record.unmapped_hits, + 'summary_hit_shortfall': record.summary_hit_shortfall, + 'mapped_hit_uncertainty': uncertainty, 'changed_lines_without_line_records': sorted(changed.get(name, set()) - lines.keys() - excluded), - 'production_hit': sum(n > 0 for n in production.values()), - 'test_found': len(tests), 'test_hit': sum(n > 0 for n in tests.values()), + 'production_hit': max(0, sum(n > 0 for n in production.values()) - uncertainty), + 'test_found': len(tests), + 'test_hit': max(0, sum(n > 0 for n in tests.values()) - uncertainty), 'uncovered_production': sorted(n for n, hits in production.items() if not hits), 'changed_production_found': len(edits), + 'changed_production_hit': max(0, sum(production[n] > 0 for n in edits) - uncertainty), 'uncovered_changed_production': sorted(n for n in edits if not production[n]), }) return {'schema_version': 1, 'files': files, @@ -257,13 +266,12 @@ def branch_counts(path: Path) -> tuple[int, int]: def details_report(data: dict) -> str: files = data['files'] totals = {key: sum(f[key] for f in files) for key in - ('production_hit', 'production_found', 'test_hit', 'test_found', 'changed_production_found')} - uncovered = sum(len(f['uncovered_changed_production']) for f in files) + ('production_hit', 'production_found', 'test_hit', 'test_found', 'changed_production_found', 'changed_production_hit')} lines = ['## Production-focused coverage', '', 'Supplemental schema 1; the whole-workspace exact-base line/function gate is unchanged.', '', f"Production lines: {totals['production_hit']}/{totals['production_found']}; " f"test lines: {totals['test_hit']}/{totals['test_found']}.", - f"Changed executable production lines: {totals['changed_production_found'] - uncovered}/" + f"Changed executable production lines: {totals['changed_production_hit']}/" f"{totals['changed_production_found']} covered.", '', '| Source | Production hit/total | Test hit/total | Unmapped summary lines | Uncovered changed production lines |', '| --- | ---: | ---: | ---: | --- |'] @@ -272,6 +280,8 @@ def details_report(data: dict) -> str: lines.append(f"| {f['path']} | {f['production_hit']}/{f['production_found']} | " f"{f['test_hit']}/{f['test_found']} | {f['unmapped_summary_lines']} | {missing} |") lines += ['', 'Unmapped LF summary entries are conservatively counted as uncovered production.', + 'Each covered subtotal deducts max(positive DA + unmapped LF - LH, 0) as unmapped-hit uncertainty;', + 'these are conservative lower bounds, not a claim of exact attribution. Details retain the deficit.', 'Changed lines lacking DA records (including non-executable syntax) are retained in details.json;', 'they are not assumed covered.', '', 'Changed Rust files without instrumentation (not assumed covered):'] diff --git a/scripts/tests/test_coverage_detail.py b/scripts/tests/test_coverage_detail.py index 86371805..52d99a2c 100644 --- a/scripts/tests/test_coverage_detail.py +++ b/scripts/tests/test_coverage_detail.py @@ -48,7 +48,7 @@ def test_lcov_rejects_missing_empty_malformed_and_duplicate_records(self): def test_lcov_line_summaries_cannot_hide_truncated_or_overcounted_data(self): valid = self.record() - for text in [valid.replace('LH:1', 'LH:0'), + for text in [valid.replace('LH:1', 'LH:-1'), valid.replace('LF:2', 'LF:1'), valid.replace('LF:2\n', ''), valid.replace('LH:1\n', ''), @@ -74,7 +74,7 @@ def test_unmapped_llvm_summary_lines_remain_uncovered_production(self): self.assertEqual(records[name].unmapped, 1) result = detail.summarize(self.root, records, {name: {2}})['files'][0] self.assertEqual(result['production_found'], 2) - self.assertEqual(result['production_hit'], 1) + self.assertEqual(result['production_hit'], 0) self.assertEqual(result['unmapped_summary_lines'], 1) self.assertEqual(result['changed_lines_without_line_records'], [2]) records = detail.line_records(self.lcov(self.record().replace('DA:1,1\n', ''))) @@ -95,6 +95,34 @@ def test_comparison_and_shift_operators_are_not_generic_type_delimiters(self): with self.subTest(declaration=declaration): self.assertEqual(detail.test_lines(source), {1, 2}) + def test_macos_summary_hit_shortfall_is_visible_and_cannot_inflate_subtotals(self): + name = 'crates/pet/src/lib.rs' + path = self.root / name + path.parent.mkdir(parents=True) + path.write_text('fn first() {}\nfn second() {}\n') + text = self.record(lines='DA:1,1\nDA:2,1\n').replace('LH:2', 'LH:1') + records = detail.line_records(self.lcov(text)) + self.assertEqual(records[name].summary_hit_shortfall, 1) + result = detail.summarize(self.root, records, {name: {1, 2}})['files'][0] + self.assertEqual(result['production_hit'], 1) + self.assertEqual(result['changed_production_hit'], 1) + self.assertEqual(result['summary_hit_shortfall'], 1) + + def test_unmapped_hits_cannot_inflate_any_subset_lower_bound(self): + name = 'crates/pet/src/lib.rs' + path = self.root / name + path.parent.mkdir(parents=True) + path.write_text('fn real() {}\n#[cfg(test)]\nmod tests {\n fn test1() {}\n fn test2() {}\n}\n') + text = self.record(lines='DA:1,1\nDA:4,1\nDA:5,1\n').replace('LF:3', 'LF:4').replace('LH:3', 'LH:2') + records = detail.line_records(self.lcov(text)) + row = detail.summarize(self.root, records, {name: {1}})['files'][0] + self.assertEqual(row['unmapped_summary_lines'], 1) + self.assertEqual(row['summary_hit_shortfall'], 1) + self.assertEqual(row['mapped_hit_uncertainty'], 2) + self.assertEqual(row['production_hit'], 0) + self.assertEqual(row['test_hit'], 0) + self.assertEqual(row['changed_production_hit'], 0) + def test_inline_tests_do_not_hide_later_production_items(self): source = 'fn before() {}\n#[cfg(test)]\nmod tests {\n fn check() {}\n}\nfn after() {}\n' self.assertEqual(detail.test_lines(source), {2, 3, 4, 5}) diff --git a/scripts/tests/test_quality_workflows.py b/scripts/tests/test_quality_workflows.py index 60b35cd0..c467f5eb 100644 --- a/scripts/tests/test_quality_workflows.py +++ b/scripts/tests/test_quality_workflows.py @@ -91,6 +91,8 @@ def test_macos_coverage_measures_the_exact_base_without_a_schema_bypass(self) -> measure = steps["Measure Exact PR Base on the Same Runner"] self.assertIn("github.event.pull_request.base.sha", measure) self.assertIn("git worktree add --detach", measure) + self.assertIn("PET_SUBPROCESS_COVERAGE_PROOF: ${{ runner.temp }}/base-subprocess-coverage.json", measure) + self.assertNotIn("${{ github.workspace }}/subprocess-coverage.json", measure) self.assertIn("cargo llvm-cov --workspace", measure) self.assertNotIn("--features", measure) self.assertIn("cargo llvm-cov --workspace", steps["Collect Native macOS Coverage"]) From 543e80f6a4b487851b9543d95ac879bdae8d3fc0 Mon Sep 17 00:00:00 2001 From: Karthik Nadig Date: Wed, 30 Sep 2026 09:48:51 -0700 Subject: [PATCH 3/4] fix: avoid coverage scanner suffix copies (Refs #534) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- scripts/coverage_detail.py | 16 ++++++++++------ scripts/tests/test_coverage_detail.py | 16 ++++++++++++++++ 2 files changed, 26 insertions(+), 6 deletions(-) diff --git a/scripts/coverage_detail.py b/scripts/coverage_detail.py index dbbdc4c8..22c67f8d 100644 --- a/scripts/coverage_detail.py +++ b/scripts/coverage_detail.py @@ -15,6 +15,10 @@ from quality_snapshot import SnapshotError, parse_lcov +RAW_STRING_START = re.compile(r'(?:br|cr|r)(#*)"') +CHARACTER_LITERAL = re.compile(r"'(?:[^'\\\n]|\\(?:u\{[0-9a-fA-F_]+\}|x[0-9a-fA-F]{2}|.))'") +ITEM_ATTRIBUTE = re.compile(r'\s*#\s*\[') + def run(*args: str, cwd: Path) -> str: return subprocess.check_output(args, cwd=cwd, text=True, encoding='utf-8') @@ -112,10 +116,10 @@ def code_mask(source: str) -> str: if depth: raise SnapshotError('Unterminated Rust block comment') else: - raw = re.match(r'(?:br|cr|r)(#*)"', source[i:]) + raw = RAW_STRING_START.match(source, i) if raw: close = '"' + raw[1] - end = source.find(close, i + raw.end()) + end = source.find(close, raw.end()) if end == -1: raise SnapshotError('Unterminated Rust raw string') end += len(close) @@ -132,9 +136,9 @@ def code_mask(source: str) -> str: else: raise SnapshotError('Unterminated Rust string') elif source[i] == "'": - char = re.match(r"'(?:[^'\\\n]|\\(?:u\{[0-9a-fA-F_]+\}|x[0-9a-fA-F]{2}|.))'", source[i:]) + char = CHARACTER_LITERAL.match(source, i) if char: - end = i + char.end() + end = char.end() if end > i: for n in range(i, end): if chars[n] != '\n': @@ -153,10 +157,10 @@ def test_lines(source: str) -> set[int]: end = match.end() # Other attributes belong to this same item, not its body. while True: - attr = re.match(r'\s*#\s*\[', masked[end:]) + attr = ITEM_ATTRIBUTE.match(masked, end) if not attr: break - end += attr.end() + end = attr.end() depth = 1 while depth and end < len(masked): depth += (masked[end] == '[') - (masked[end] == ']') diff --git a/scripts/tests/test_coverage_detail.py b/scripts/tests/test_coverage_detail.py index 52d99a2c..7dfc33e7 100644 --- a/scripts/tests/test_coverage_detail.py +++ b/scripts/tests/test_coverage_detail.py @@ -137,6 +137,22 @@ def test_literals_nested_comments_and_attributes_do_not_move_boundaries(self): self.assertEqual(detail.test_lines(source), set(range(3, 10))) self.assertEqual(detail.code_mask(source).count('\n'), source.count('\n')) + def test_literal_scanning_matches_at_offsets_without_copying_source_suffixes(self): + class NoSlices(str): + def __getitem__(self, key): + if isinstance(key, slice): + raise AssertionError('scanner copied a source suffix') + return super().__getitem__(key) + + prefix = "fn real<'a>(x: &'a str) { " + literals = ['r"text"', 'br##"}\\n{"##', 'cr#"text"#', + "'{'", r"'\u{7b}'", r"'\x7b'", r"'\''"] + source = prefix + '; '.join(literals) + '; }\n' + expected = prefix + '; '.join(' ' * len(value) for value in literals) + '; }\n' + for copies in [1, 1000]: + with self.subTest(copies=copies): + self.assertEqual(detail.code_mask(NoSlices(source * copies)), expected * copies) + def test_cfg_test_function_and_external_module_are_test_code(self): source = '#[cfg(test)]\nfn helper() {}\n#[cfg(test)]\nmod fixtures;\nfn real() {}\n' self.assertEqual(detail.test_lines(source), {1, 2, 3, 4}) From e79fcacbd240f5bff4832cdb2ada15c917db7792 Mon Sep 17 00:00:00 2001 From: Karthik Nadig Date: Wed, 30 Sep 2026 10:05:28 -0700 Subject: [PATCH 4/4] fix: classify conditional test coverage correctly (Refs #534) Exclude all/any predicates that require test without excluding optional test branches. Keep unmapped integration-test and benchmark entries in the test denominator while preserving conservative bounds and raw coverage gates. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- docs/QUALITY_SNAPSHOTS.md | 11 +++++--- scripts/coverage_detail.py | 36 ++++++++++++++++++++++++--- scripts/tests/test_coverage_detail.py | 31 +++++++++++++++++++++++ 3 files changed, 70 insertions(+), 8 deletions(-) diff --git a/docs/QUALITY_SNAPSHOTS.md b/docs/QUALITY_SNAPSHOTS.md index 95d6b285..df35bed8 100644 --- a/docs/QUALITY_SNAPSHOTS.md +++ b/docs/QUALITY_SNAPSHOTS.md @@ -123,15 +123,18 @@ without instrumentation are listed explicitly, never assumed covered. These diag introduce a fabricated baseline, change the raw denominator, or replace regression protection. Classification excludes integration-test/benchmark directories and Rust items explicitly marked -`#[cfg(test)]` or `#[test]`, including inline modules and test-only helper functions. It masks +`#[cfg(test)]` or `#[test]`, including inline modules and test-only helper functions. Nested +`all`/`any` predicates are also excluded when they require `test`: `all(test, unix)` is test-only, +but `any(test, unix)` is not. It masks strings, raw/byte strings, characters, and nested comments before matching item boundaries. -Helpers outside those boundaries and complex conditional attributes remain conservatively in the +Helpers outside those boundaries and unsupported conditional predicates remain conservatively in the production category; this is a source-focused diagnostic, not full Rust conditional-compilation analysis. Invalid/missing LCOV, missing source, inconsistent hit summaries, and source-line mismatches fail the reporting step. LLVM summaries can include more entries in `LF`/`LH` than the unique `DA` source lines (observed in real Windows exports). That deficit is reported -per file (including unmatched summary hits) and conservatively retained as uncovered production, -never dropped from the denominator or silently assigned coverage. +per file (including unmatched summary hits) and conservatively retained as uncovered production +in mixed source files, or uncovered tests in integration-test/benchmark files. It is never dropped +from the denominator or silently assigned coverage. Changed lines without `DA` records are listed separately in JSON, including non-executable syntax; they are not silently considered covered. Native macOS also demonstrates `LH` below the number of positive unique `DA` entries. Reports diff --git a/scripts/coverage_detail.py b/scripts/coverage_detail.py index 22c67f8d..e6190073 100644 --- a/scripts/coverage_detail.py +++ b/scripts/coverage_detail.py @@ -149,10 +149,37 @@ def code_mask(source: str) -> str: return ''.join(chars) +def cfg_requires_test(expression: str) -> bool: + expression = expression.strip() + if expression == 'test': + return True + group = re.fullmatch(r'(all|any)\s*\((.*)\)', expression, re.DOTALL) + if not group: + return False + arguments = [] + depth = start = 0 + for index, char in enumerate(group[2]): + depth += (char == '(') - (char == ')') + if depth < 0: + raise SnapshotError('Unbalanced Rust cfg predicate') + if char == ',' and depth == 0: + arguments.append(group[2][start:index]) + start = index + 1 + if depth: + raise SnapshotError('Unbalanced Rust cfg predicate') + final = group[2][start:] + if final.strip(): + arguments.append(final) + required = [cfg_requires_test(argument) for argument in arguments] + return any(required) if group[1] == 'all' else bool(required) and all(required) + + def test_lines(source: str) -> set[int]: masked = code_mask(source) excluded: set[int] = set() - for match in re.finditer(r'#\s*\[\s*(?:cfg\s*\(\s*test\s*\)|test)\s*\]', masked): + for match in re.finditer(r'#\s*\[\s*(?:cfg\s*\((?P[^]]*)\)|test)\s*\]', masked): + if match['cfg'] is not None and not cfg_requires_test(match['cfg']): + continue start = match.start() end = match.end() # Other attributes belong to this same item, not its body. @@ -226,20 +253,21 @@ def summarize(root: Path, records: dict[str, SourceCoverage], changed: dict[str, if any(n > len(source.splitlines()) for n in lines): raise SnapshotError(f'LCOV source revision mismatch: {name}') parts = Path(name).parts - excluded = set(lines) if 'tests' in parts or 'benches' in parts else test_lines(source) + test_only = 'tests' in parts or 'benches' in parts + excluded = set(lines) if test_only else test_lines(source) production = {n: count for n, count in lines.items() if n not in excluded} tests = {n: count for n, count in lines.items() if n in excluded} edits = production.keys() & changed.get(name, set()) uncertainty = record.unmapped + record.summary_hit_shortfall - record.unmapped_hits files.append({ - 'path': name, 'production_found': len(production) + record.unmapped, + 'path': name, 'production_found': len(production) + (0 if test_only else record.unmapped), 'unmapped_summary_lines': record.unmapped, 'unmapped_summary_hits': record.unmapped_hits, 'summary_hit_shortfall': record.summary_hit_shortfall, 'mapped_hit_uncertainty': uncertainty, 'changed_lines_without_line_records': sorted(changed.get(name, set()) - lines.keys() - excluded), 'production_hit': max(0, sum(n > 0 for n in production.values()) - uncertainty), - 'test_found': len(tests), + 'test_found': len(tests) + (record.unmapped if test_only else 0), 'test_hit': max(0, sum(n > 0 for n in tests.values()) - uncertainty), 'uncovered_production': sorted(n for n, hits in production.items() if not hits), 'changed_production_found': len(edits), diff --git a/scripts/tests/test_coverage_detail.py b/scripts/tests/test_coverage_detail.py index 7dfc33e7..0c1f4bd4 100644 --- a/scripts/tests/test_coverage_detail.py +++ b/scripts/tests/test_coverage_detail.py @@ -123,6 +123,37 @@ def test_unmapped_hits_cannot_inflate_any_subset_lower_bound(self): self.assertEqual(row['test_hit'], 0) self.assertEqual(row['changed_production_hit'], 0) + def test_conditional_attributes_exclude_only_items_requiring_test(self): + required = [ + 'all(test, unix)', 'all(windows, test,)', 'all(test, feature = "flag")', + 'all(any(unix, windows), all(feature = "flag", test))', + 'any(all(test, unix), all(windows, test))', + ] + optional = [ + 'any(test, unix)', 'any(windows, test)', 'not(test)', 'all(unix, feature = "test")', + 'any(all(test, unix), windows)', 'all(any(test, unix), windows)', 'any()', 'all()', + ] + for condition in required + optional: + source = f'#[cfg({condition})]\nmod scoped {{\n fn helper() {{}}\n}}\nfn real() {{}}\n' + with self.subTest(condition=condition): + self.assertEqual(detail.test_lines(source), {1, 2, 3, 4} if condition in required else set()) + + def test_unmapped_test_file_entries_stay_in_the_test_denominator(self): + for folder in ['tests', 'benches']: + name = f'crates/pet/{folder}/fixture.rs' + path = self.root / name + path.parent.mkdir(parents=True) + path.write_text('fn helper() {}\nfn check() {}\n') + records = detail.line_records(self.lcov(self.record(name).replace('LF:2', 'LF:3'))) + row = detail.summarize(self.root, records, {name: {1, 2}})['files'][0] + with self.subTest(folder=folder): + self.assertEqual(row['production_found'], 0) + self.assertEqual(row['production_hit'], 0) + self.assertEqual(row['changed_production_found'], 0) + self.assertEqual(row['test_found'], 3) + self.assertEqual(row['test_hit'], 0) + self.assertEqual(row['unmapped_summary_lines'], 1) + def test_inline_tests_do_not_hide_later_production_items(self): source = 'fn before() {}\n#[cfg(test)]\nmod tests {\n fn check() {}\n}\nfn after() {}\n' self.assertEqual(detail.test_lines(source), {2, 3, 4, 5})