From 6ed0d38122de74e7cbd67dc3db76fd15a270acc2 Mon Sep 17 00:00:00 2001 From: Amaury Chamayou Date: Wed, 9 Sep 2026 08:11:10 +0100 Subject: [PATCH] Instrument first-party libraries in coverage reports Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .github/skills/testing/SKILL.md | 10 ++ .github/workflows/coverage.yml | 11 +- cmake/ccf_app.cmake | 1 + cmake/crypto.cmake | 1 + cmake/tools.cmake | 19 ++-- scripts/coverage.sh | 13 ++- scripts/coverage_report.py | 157 ++++++++++++++++++++++++++ scripts/tests/coverage_report_test.py | 131 +++++++++++++++++++++ 8 files changed, 332 insertions(+), 11 deletions(-) create mode 100644 scripts/coverage_report.py create mode 100644 scripts/tests/coverage_report_test.py diff --git a/.github/skills/testing/SKILL.md b/.github/skills/testing/SKILL.md index c891945da26b..ec593a16e215 100644 --- a/.github/skills/testing/SKILL.md +++ b/.github/skills/testing/SKILL.md @@ -54,10 +54,20 @@ Configure with `-DCOVERAGE=ON`, build instrumented targets, and run the selected ```bash ../scripts/coverage.sh # Print summary ../scripts/coverage.sh --html report/ # Generate HTML report +../scripts/coverage.sh --html report/ --json report/coverage.json +python3 ../scripts/coverage_report.py report/coverage.json ``` These paths assume `build` is immediately under the repository root. For another layout, use the actual path to `scripts/coverage.sh` while retaining the build working directory. The script consumes `.profraw` files and the generated `coverage_binaries.txt`; building alone does not produce coverage. +The JSON export contains LLVM's per-file line and branch counts with the same exclusions as the HTML report. `coverage_report.py` separates framework (`src/`, `include/`), sample (`samples/`) and other paths without dropping any paths from the combined total. For reports produced elsewhere, pass `--source-dir` with that report's source root. + +After the coverage workflow's unit and e2e selection, use `coverage_report.py --check-instrumentation report/coverage.json` to require nonzero line coverage in representative implementation files and headers. This guard is intentionally opt-in for smaller local test selections. Its reporting tests use the standard library: `python3 scripts/tests/coverage_report_test.py` from the source root. + +First-party C++ static libraries are instrumented, but only linked binaries are report inputs; archives are not counted a second time. Their coverage runtime link requirement also applies to consumers of installed instrumented libraries. Rust is not instrumented by the C++ coverage flags. + +For a baseline, use a fresh coverage build/profile directory and record the source revision, build options and exact test selection. Targeted tests do not establish a full-suite baseline. Reports predating library instrumentation have a different denominator and are not directly comparable; including previously invisible code can lower the headline percentage. + ## End-to-end test infrastructure E2e tests use the infrastructure in `tests/infra/`. The key classes are: diff --git a/.github/workflows/coverage.yml b/.github/workflows/coverage.yml index b7afb0467899..7d6dedb2add9 100644 --- a/.github/workflows/coverage.yml +++ b/.github/workflows/coverage.yml @@ -77,7 +77,11 @@ jobs: run: | set -exo pipefail cd build - ../scripts/coverage.sh --html coverage_html | tee coverage_report.txt + python3 ../scripts/tests/coverage_report_test.py + ../scripts/coverage.sh --html coverage_html --json coverage_html/coverage.json \ + | tee coverage_report.txt + python3 ../scripts/coverage_report.py coverage_html/coverage.json \ + --check-instrumentation | tee coverage_html/summary.md shell: bash - name: "Download previous coverage summaries" @@ -114,6 +118,9 @@ jobs: run: | set -ex cd build + if [[ -f coverage_html/summary.md ]]; then + cat coverage_html/summary.md >> "$GITHUB_STEP_SUMMARY" + fi if [[ -f coverage_report.txt ]]; then python3 ../scripts/coverage_summary.py coverage_report.txt coverage_history \ >> "$GITHUB_STEP_SUMMARY" || true @@ -127,7 +134,7 @@ jobs: fi shell: bash - - name: "Upload HTML coverage report" + - name: "Upload HTML and JSON coverage report" uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 with: name: coverage-report-html diff --git a/cmake/ccf_app.cmake b/cmake/ccf_app.cmake index 2f119beae2bf..36aa7647eadf 100644 --- a/cmake/ccf_app.cmake +++ b/cmake/ccf_app.cmake @@ -65,6 +65,7 @@ function(add_ccf_static_library name) add_hardening(${name}) add_tidy(${name}) add_warning_checks(${name}) + enable_coverage(${name}) install(TARGETS ${name} EXPORT ccf DESTINATION lib) diff --git a/cmake/crypto.cmake b/cmake/crypto.cmake index 0efcee0742dd..dba4e35dd5ee 100644 --- a/cmake/crypto.cmake +++ b/cmake/crypto.cmake @@ -42,6 +42,7 @@ target_compile_options( add_san(ccfcrypto) add_hardening(ccfcrypto) add_tidy(ccfcrypto) +enable_coverage(ccfcrypto) target_link_libraries(ccfcrypto PUBLIC crypto ssl evercbor ccf_threading) target_link_libraries( diff --git a/cmake/tools.cmake b/cmake/tools.cmake index 0585dc33a64f..e67d21e59ce1 100644 --- a/cmake/tools.cmake +++ b/cmake/tools.cmake @@ -119,16 +119,21 @@ separate_arguments( UNIX_COMMAND "-fprofile-instr-generate -fcoverage-mapping" ) -separate_arguments( - COVERAGE_LINK - UNIX_COMMAND - "-fprofile-instr-generate -fcoverage-mapping" -) +separate_arguments(COVERAGE_LINK UNIX_COMMAND "-fprofile-instr-generate") function(enable_coverage name) if(COVERAGE) target_compile_options(${name} PRIVATE ${COVERAGE_FLAGS}) - target_link_libraries(${name} PRIVATE ${COVERAGE_LINK}) - set_property(GLOBAL APPEND PROPERTY CCF_COVERAGE_TARGETS ${name}) + get_target_property(target_type ${name} TYPE) + if(target_type STREQUAL "STATIC_LIBRARY") + # Consumers need the runtime even if their own sources are not + # instrumented. This also applies to installed instrumented archives. + target_link_options(${name} INTERFACE ${COVERAGE_LINK}) + else() + target_link_options(${name} PRIVATE ${COVERAGE_LINK}) + # Report linked objects through their binaries, not again as archives + # containing implementation code that may never have been linked. + set_property(GLOBAL APPEND PROPERTY CCF_COVERAGE_TARGETS ${name}) + endif() endif() endfunction() diff --git a/scripts/coverage.sh b/scripts/coverage.sh index 15d54172a8a4..c6f7c27de4fb 100755 --- a/scripts/coverage.sh +++ b/scripts/coverage.sh @@ -20,6 +20,7 @@ Options: -d Directory to search for .profraw files (default: .) -o Output merged profile file (default: /coverage.profdata) --html Generate an HTML coverage report in + --json Export LLVM per-file coverage counts as JSON --show-uncovered Print files and line numbers with zero coverage -h, --help Show this help @@ -33,8 +34,8 @@ Examples: ctest -L unit ../scripts/coverage.sh - # Generate an HTML report: - ../scripts/coverage.sh --html ./coverage_html + # Generate HTML and machine-readable per-file counts: + ../scripts/coverage.sh --html ./coverage_html --json ./coverage_html/coverage.json # Show which specific lines are uncovered: ../scripts/coverage.sh --show-uncovered @@ -56,6 +57,7 @@ EOF PROFRAW_DIR="." OUTPUT_FILE="" HTML_DIR="" +JSON_FILE="" SHOW_UNCOVERED=0 BINARIES=() @@ -64,6 +66,7 @@ while [[ $# -gt 0 ]]; do -d) PROFRAW_DIR="$2"; shift 2 ;; -o) OUTPUT_FILE="$2"; shift 2 ;; --html) HTML_DIR="$2"; shift 2 ;; + --json) JSON_FILE="$2"; shift 2 ;; --show-uncovered) SHOW_UNCOVERED=1; shift ;; -h|--help) usage ;; --) shift; BINARIES+=("$@"); break ;; @@ -226,3 +229,9 @@ if [[ -n "${HTML_DIR}" ]]; then "${LLVM_COV}" show "${COV_ARGS[@]}" --format=html --output-dir="${HTML_DIR}" echo "HTML report written to '${HTML_DIR}/index.html'" fi + +if [[ -n "${JSON_FILE}" ]]; then + mkdir -p "$(dirname "${JSON_FILE}")" + "${LLVM_COV}" export "${COV_ARGS[@]}" --summary-only > "${JSON_FILE}" + echo "Per-file coverage counts written to '${JSON_FILE}'" +fi diff --git a/scripts/coverage_report.py b/scripts/coverage_report.py new file mode 100644 index 000000000000..9357535547bd --- /dev/null +++ b/scripts/coverage_report.py @@ -0,0 +1,157 @@ +# Copyright (c) Microsoft Corporation. All rights reserved. +# Licensed under the Apache 2.0 License. + +"""Summarise and check a summary-only llvm-cov JSON export.""" + +import argparse +import json +import sys +from pathlib import Path + +METRICS = ("lines", "branches") +SCOPES = ("Framework", "Samples", "Other", "All reported C++") + +# One implementation file per first-party library, plus formerly unhit headers. +INSTRUMENTATION_SENTINELS = ( + "src/host/run.cpp", + "src/enclave/main.cpp", + "src/crypto/base64.cpp", + "src/js/core/context.cpp", + "src/endpoints/authentication/jwt_auth.cpp", + "src/kv/tx.cpp", + "src/tasks/task_system.cpp", + "src/pal/attestation.cpp", + "src/threading/thread_ids.cpp", + "src/node/node_state.h", + "src/host/tcp.h", + "src/http/http2_parser.h", +) + + +def coverage_files(document: dict, source_dir: Path) -> dict: + if document["type"] != "llvm.coverage.json.export" or not document["data"]: + raise ValueError("Expected a non-empty llvm-cov JSON export") + + source_dir = source_dir.resolve() + files = {} + for data in document["data"]: + for entry in data["files"]: + path = Path(entry["filename"]).resolve() + filename = ( + path.relative_to(source_dir).as_posix() + if path.is_relative_to(source_dir) + else path.as_posix() + ) + if filename in files: + raise ValueError(f"Duplicate coverage file: {filename}") + files[filename] = entry["summary"] + if not files: + raise ValueError("Coverage export contains no files") + return files + + +def scoped_totals(files: dict) -> dict: + totals = { + scope: {metric: {"count": 0, "covered": 0} for metric in METRICS} + for scope in SCOPES + } + for filename, summary in files.items(): + if filename.startswith(("src/", "include/")): + scope = "Framework" + elif filename.startswith("samples/"): + scope = "Samples" + else: + scope = "Other" + for group in (scope, "All reported C++"): + for metric in METRICS: + for field in ("count", "covered"): + totals[group][metric][field] += summary[metric][field] + return totals + + +def format_count(counts: dict) -> str: + count = counts["count"] + covered = counts["covered"] + percent = f"{100 * covered / count:.2f}%" if count else "n/a" + return f"{covered}/{count} ({percent})" + + +def render_report(files: dict) -> str: + lines = [ + "## Native C++ coverage scope", + "", + "| Scope | Covered lines | Covered branches |", + "| --- | --- | --- |", + ] + for scope, totals in scoped_totals(files).items(): + lines.append( + f"| {scope} | {format_count(totals['lines'])} " + f"| {format_count(totals['branches'])} |" + ) + lines += [ + "", + "Framework is `src/` and `include/`; samples are `samples/`. Other " + "reported paths are retained in the combined total. All scopes use " + "the same third-party, test and performance-code exclusions as HTML.", + "", + "These totals include separately compiled first-party C++ libraries. " + "The denominator is not comparable with older reports that only " + "instrumented application and test translation units; adding previously " + "invisible code can reduce the headline percentage. Rust is not " + "instrumented by these C++ flags.", + "", + ] + return "\n".join(lines) + + +def instrumentation_failures(files: dict) -> list[str]: + failures = [] + for filename in INSTRUMENTATION_SENTINELS: + summary = files.get(filename) + if summary is None: + failures.append(f"{filename}: missing from coverage report") + elif summary["lines"]["count"] == 0 or summary["lines"]["covered"] == 0: + failures.append(f"{filename}: no covered executable lines") + return failures + + +def main() -> int: + parser = argparse.ArgumentParser(description=__doc__) + parser.add_argument("report", type=Path, help="llvm-cov JSON export") + parser.add_argument( + "--source-dir", + type=Path, + default=Path(__file__).resolve().parents[1], + help="Source directory used for this report (default: repository root)", + ) + parser.add_argument( + "--check-instrumentation", + action="store_true", + help="Require implementation sentinels to have hits after unit and e2e tests", + ) + args = parser.parse_args() + files = coverage_files( + json.loads(args.report.read_text(encoding="utf-8")), args.source_dir + ) + print(render_report(files)) + + if args.check_instrumentation: + print("## Instrumentation sentinels\n") + print("| Source | Covered lines |") + print("| --- | --- |") + for filename in INSTRUMENTATION_SENTINELS: + summary = files.get(filename) + coverage = format_count(summary["lines"]) if summary else "Missing" + print(f"| `{filename}` | {coverage} |") + failures = instrumentation_failures(files) + if failures: + for failure in failures: + print( + f"Coverage instrumentation check failed: {failure}", file=sys.stderr + ) + return 1 + return 0 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/scripts/tests/coverage_report_test.py b/scripts/tests/coverage_report_test.py new file mode 100644 index 000000000000..f108b5bc891f --- /dev/null +++ b/scripts/tests/coverage_report_test.py @@ -0,0 +1,131 @@ +# Copyright (c) Microsoft Corporation. All rights reserved. +# Licensed under the Apache 2.0 License. + +import importlib.util +import json +import subprocess +import sys +import tempfile +import unittest +from pathlib import Path + +SCRIPT = Path(__file__).parents[1] / "coverage_report.py" +SPEC = importlib.util.spec_from_file_location("coverage_report", SCRIPT) +if SPEC is None or SPEC.loader is None: + raise RuntimeError(f"Could not load {SCRIPT}") +REPORT = importlib.util.module_from_spec(SPEC) +SPEC.loader.exec_module(REPORT) + + +def summary(lines=10, covered_lines=4, branches=2, covered_branches=1): + return { + "lines": {"count": lines, "covered": covered_lines}, + "branches": {"count": branches, "covered": covered_branches}, + } + + +def export(files, root): + return { + "type": "llvm.coverage.json.export", + "data": [ + { + "files": [ + {"filename": str(root / name), "summary": counts} + for name, counts in files.items() + ] + } + ], + } + + +class CoverageReportTest(unittest.TestCase): + def test_scopes_preserve_combined_counts(self): + files = { + "src/crypto/base64.cpp": summary(), + "include/ccf/crypto/base64.h": summary(3, 1, 0, 0), + "samples/apps/logging/logging.cpp": summary(7, 2), + "/external/src/example.h": summary(2, 1, 2, 0), + } + with tempfile.TemporaryDirectory() as directory: + root = Path(directory) + normalised = REPORT.coverage_files(export(files, root), root) + self.assertEqual(normalised, files) + totals = REPORT.scoped_totals(normalised) + self.assertEqual(totals["Framework"]["lines"], {"count": 13, "covered": 5}) + self.assertEqual(totals["Samples"]["lines"], {"count": 7, "covered": 2}) + self.assertEqual(totals["Other"]["lines"], {"count": 2, "covered": 1}) + self.assertEqual( + totals["All reported C++"]["lines"], {"count": 22, "covered": 8} + ) + self.assertEqual( + totals["All reported C++"]["branches"], {"count": 6, "covered": 2} + ) + + def test_rejects_duplicate_files(self): + with tempfile.TemporaryDirectory() as directory: + root = Path(directory) + document = export({"src/crypto/base64.cpp": summary()}, root) + document["data"].append(document["data"][0]) + with self.assertRaisesRegex(ValueError, "Duplicate coverage file"): + REPORT.coverage_files(document, root) + + def test_rejects_invalid_or_empty_exports(self): + for document in ( + {"type": "unknown", "data": [{}]}, + {"type": "llvm.coverage.json.export", "data": []}, + {"type": "llvm.coverage.json.export", "data": [{"files": []}]}, + ): + with self.subTest(document=document): + with self.assertRaises(ValueError): + REPORT.coverage_files(document, Path.cwd()) + + def test_checks_every_sentinel(self): + files = {name: summary() for name in REPORT.INSTRUMENTATION_SENTINELS} + self.assertEqual(REPORT.instrumentation_failures(files), []) + for filename in REPORT.INSTRUMENTATION_SENTINELS: + with self.subTest(filename=filename): + for replacement in (None, summary(covered_lines=0), summary(0, 0)): + changed = files.copy() + if replacement is None: + del changed[filename] + else: + changed[filename] = replacement + failures = REPORT.instrumentation_failures(changed) + self.assertEqual(len(failures), 1) + self.assertIn(filename, failures[0]) + + def test_labels_denominator_change_and_empty_scopes(self): + text = REPORT.render_report({"src/crypto/base64.cpp": summary()}) + self.assertIn("| Framework | 4/10 (40.00%) | 1/2 (50.00%) |", text) + self.assertIn("| Samples | 0/0 (n/a) | 0/0 (n/a) |", text) + self.assertIn("denominator is not comparable", text) + self.assertIn("Rust is not instrumented", text) + + def test_cli_only_checks_sentinels_when_requested(self): + with tempfile.TemporaryDirectory() as directory: + root = Path(directory) + report = root / "coverage.json" + report.write_text( + json.dumps(export({"src/crypto/base64.cpp": summary()}, root)), + encoding="utf-8", + ) + command = [ + sys.executable, + str(SCRIPT), + str(report), + "--source-dir", + str(root), + ] + result = subprocess.run(command, capture_output=True, text=True, check=True) + self.assertIn("Native C++ coverage scope", result.stdout) + checked = subprocess.run( + command + ["--check-instrumentation"], capture_output=True, text=True + ) + self.assertEqual(checked.returncode, 1) + self.assertIn( + "src/host/run.cpp: missing from coverage report", checked.stderr + ) + + +if __name__ == "__main__": + unittest.main()