From bb3f7b56702e1fc5f0271a260aeeb62402a17755 Mon Sep 17 00:00:00 2001 From: Dan Calavrezo <195309321+dcalavrezo-qorix@users.noreply.github.com> Date: Tue, 15 Sep 2026 09:48:21 +0300 Subject: [PATCH 1/6] Fix scope leak via forwarded CcInfo and broken HTML links (#5) Two defects reported in issue #5, both visible in the baselibs report: 1. A workspace rule that only forwards the CcInfo of a third-party library (baselibs third_party/openssl applies a transition that way) made that library's headers the workspace target's direct_public_headers, and the scope aspect took them as declared headers: 72 external/openssl+ files in the report. 2. The reporter let llvm-cov read sources through the workspace directory. Generated headers (bazel-out/.../_virtual_includes/) exist there only after an earlier local build and external repositories never do, so those pages were silently not rendered while the index still linked them (94 dead links). Scope aspect: - Only files a workspace target declares itself (srcs/hdrs, including headers vendored from an external repository) enter the allowlist. Forwarded headers do not. - A header exposed through strip_include_prefix / include_prefix is recorded in a new path map (_path_map.txt): the generated _virtual_includes/ path the compiler names, and the declared header it stands for. Only paths the target generated itself are mapped. - The source files are exported (source_files output group) and the reporter wrapper adds them to the reporter's runfiles. Reporter: - Stages every in-scope source from its runfiles under the raw covmap layout and points llvm-cov at that directory, so every page can be rendered regardless of the workspace directory's state. - Files the HTML pages under coverage/.html, rewrites the index links and the pages' asset links, and names files canonically in LCOV, text summary and page titles: headers behind include prefixes appear under their declared path, never under _virtual_includes/ or a machine-specific directory. A row whose source could not be read keeps its numbers but loses the dead link. - Exclusion filters match one compiled file exactly (anchored on the recorded compilation directory and the staging root); an excluded foo/bar.h no longer suppresses an in-scope src/foo/bar.h. - A file compiled under two paths is reported once (declared path preferred), with a warning. Tests: 5 new analysis tests / fixtures (forwarded external library, include_prefix mapping, path map and source_files outputs), 19 new reporter unit tests (selection, exclusion regex, staging, page relocation, path map, summary naming), and the integration workspace gained a forwarded third-party library plus a vendored external header with hand-derived ground truth and a link-integrity check over the generated index. Validated on baselibs (//score/hash/..., //score/flatbuffers/..., //score/static_reflection_with_serialization/...): 440 index links, all resolving; no external/openssl+ entries; no bazel-out/ or absolute paths; the 15 flatbuffers headers under external/flatbuffers+/include/flatbuffers/, each once. Docs: tool requirements (scope_transitive, scope_excludes, report_allowlist, report_relative_paths), architecture, known problems, release notes 0.2.0, verification report inventory. Fixes #5 Signed-off-by: Dan Calavrezo <195309321+dcalavrezo-qorix@users.noreply.github.com> --- docs/architecture/index.rst | 21 +- docs/manual/known_problems.rst | 21 +- docs/release/release_notes.rst | 28 +- docs/requirements/tool_requirements.rst | 56 ++- docs/verification/verification_report.rst | 15 +- integration_tests/MODULE.bazel | 6 + integration_tests/expected_lcov.dat | 41 +- integration_tests/external_lib/BUILD | 24 + integration_tests/external_lib/MODULE.bazel | 18 + integration_tests/external_lib/extlib.cpp | 21 + .../external_lib/include/extlib/extlib.h | 24 + .../external_lib/include/vext/vext.h | 28 ++ integration_tests/run_integration_test.sh | 40 ++ integration_tests/src/BUILD | 2 + integration_tests/src/coverable_test.cpp | 11 + integration_tests/third_party/BUILD | 32 ++ integration_tests/third_party/forward.bzl | 30 ++ integration_tests/tools/coverage/BUILD | 5 + score_coverage/coverage_scope.bzl | 135 ++++-- score_coverage/reporter.py | 456 ++++++++++++++---- score_coverage/reporter_wrapper.bzl | 13 +- score_coverage/tests/reporter_test.py | 387 ++++++++++++++- .../tests/starlark/coverage_scope_tests.bzl | 75 ++- .../tests/starlark/external_fixture/BUILD | 10 + .../starlark/external_fixture/MODULE.bazel | 2 + score_coverage/tests/starlark/fixtures/BUILD | 23 + .../tests/starlark/fixtures/forward.bzl | 30 ++ 27 files changed, 1354 insertions(+), 200 deletions(-) create mode 100644 integration_tests/external_lib/BUILD create mode 100644 integration_tests/external_lib/MODULE.bazel create mode 100644 integration_tests/external_lib/extlib.cpp create mode 100644 integration_tests/external_lib/include/extlib/extlib.h create mode 100644 integration_tests/external_lib/include/vext/vext.h create mode 100644 integration_tests/third_party/BUILD create mode 100644 integration_tests/third_party/forward.bzl create mode 100644 score_coverage/tests/starlark/fixtures/forward.bzl diff --git a/docs/architecture/index.rst b/docs/architecture/index.rst index 1353b6a..12927af 100644 --- a/docs/architecture/index.rst +++ b/docs/architecture/index.rst @@ -36,9 +36,9 @@ justification and gating layer on top. It has two phases. [test binaries] --> (profraw per test) (profraw per test) --> [merger.py\n--coverage_output_generator] [merger.py\n--coverage_output_generator] --> (coverage.dat zip\nprofdata + meta.json) - [score_coverage_scope\naspect] --> (allowlist.txt\nobjects.txt) + [score_coverage_scope\naspect] --> (allowlist.txt\npath_map.txt\nobjects.txt\nsource files) (coverage.dat zip\nprofdata + meta.json) --> [reporter.py\n--coverage_report_generator] - (allowlist.txt\nobjects.txt) --> [reporter.py\n--coverage_report_generator] + (allowlist.txt\npath_map.txt\nobjects.txt\nsource files) --> [reporter.py\n--coverage_report_generator] [reporter.py\n--coverage_report_generator] --> (_coverage_report.dat zip\nhtml_report, lcov_report, text_report) } package "Phase 2: bazel run //:generate_coverage_html" { @@ -79,9 +79,20 @@ workspace, does four things: **Scope.** Covmap instruments everything. Filtering happens at report time through the allowlist written by ``score_coverage_scope``: an aspect walks the -dependency graph from the listed production targets and collects every -in-workspace source file they own. Everything else, test sources, googletest, -external dependencies, is excluded. +dependency graph from the listed production targets and collects every source +file a workspace target declares (including headers it vendors from an +external repository). Everything else, test sources, googletest, external +dependencies, headers a wrapper rule only forwards, is excluded. For headers +exposed through ``strip_include_prefix`` / ``include_prefix`` the aspect also +writes a path map from the generated ``_virtual_includes/`` path the compiler +records to the declared header, and it exports the source files themselves. + +**Sources.** The reporter does not read sources through the workspace +directory: generated headers and external repositories are not there at +report time. It links every in-scope file from its runfiles into a staging +directory laid out like the coverage mapping expects, points llvm-cov at that +directory, and afterwards files the HTML pages under canonical paths so the +archive is machine-independent and every index link resolves. **Baseline.** A file that no test executes produces no profile data. The scope aspect therefore also collects the compiled archives and executables, and the diff --git a/docs/manual/known_problems.rst b/docs/manual/known_problems.rst index daa3d0e..ed8421e 100644 --- a/docs/manual/known_problems.rst +++ b/docs/manual/known_problems.rst @@ -62,12 +62,21 @@ stay listed with their upstream references. - ``no coverage data found`` on a Rust archive. - Handled since the pipeline expands rlibs into their object members; if seen, the installed version predates the fix. - * - **Vendored headers appear under their virtual-includes path.** A header - compiled through ``strip_include_prefix`` is reported as - ``/_virtual_includes//``, not under the label it was - declared with, because that is the identity the compiler records. - - Report rows named ``_virtual_includes``. - - Expected; justifications for such lines must use the reported path. + * - **A header compiled under two different paths is reported once.** When + a translation unit includes a header through its ``_virtual_includes/`` + path and another through the declared path, the compiler produces two + coverage entries for one file; the reporter keeps the declared-path + variant and drops the other. + - ``WARNING: is compiled under several paths`` in the reporter log. + - Expected; hits recorded only through the dropped variant are not + counted. Include the header consistently. + * - **A source could not be staged for llvm-cov.** The reporter reads the + sources from the scope's exported files; a file that is neither there + nor in the workspace directory gets no HTML page (its numbers stay in + the index and the LCOV). + - ``WARNING: N in-scope sources were not found`` in the reporter log, an + index row without a link. + - Report it; every declared source is expected to be exported. * - **Instrumentation filter appears ignored.** - ``--instrumentation_filter`` has no visible effect. - Expected: ``--experimental_use_llvm_covmap`` instruments everything; diff --git a/docs/release/release_notes.rst b/docs/release/release_notes.rst index d276354..441f3a1 100644 --- a/docs/release/release_notes.rst +++ b/docs/release/release_notes.rst @@ -23,7 +23,33 @@ Release notes :security: NO :realizes: wp__module_sw_release_note -0.1.0 (unreleased) +0.2.0 (unreleased) +------------------ + +- Fixed (eclipse-score/coverage_tool#5): a workspace rule that forwards the + ``CcInfo`` of a third-party library (e.g. a transition wrapper around + OpenSSL) no longer puts that library's headers into the scope; only headers + a workspace target declares itself count. +- Fixed (eclipse-score/coverage_tool#5): every index link of the HTML report + points at a generated page. The reporter stages the in-scope sources from + the scope's exported files instead of reading them through the workspace + directory, where generated headers and external repositories are not + present at report time. +- Changed: headers compiled through ``strip_include_prefix`` / + ``include_prefix`` are reported under their declared path (e.g. + ``score/flatbuffers/include/flatbuffers/base.h`` or + ``external/flatbuffers+/include/flatbuffers/base.h``), no longer under the + generated ``_virtual_includes/`` path. Justifications written against a + ``_virtual_includes/`` path must be updated. +- Changed: HTML pages live at ``coverage/.html``; the archive + contains no directory of the producing machine any more. +- Fixed: the exclusion filter matches each out-of-scope compiled file exactly; + an excluded ``foo/bar.h`` no longer suppresses an in-scope ``src/foo/bar.h``. +- ``score_coverage_scope`` gained the ``path_map`` and ``source_files`` output + groups; ``score_coverage_reporter`` passes them to the reporter + (``--path_map``). Consumers only instantiate the macros; no change needed. + +0.1.0 (2026-09-11) ------------------ First release as a standalone module, extracted from ``@score_tooling//coverage`` diff --git a/docs/requirements/tool_requirements.rst b/docs/requirements/tool_requirements.rst index f7e830e..a55ea15 100644 --- a/docs/requirements/tool_requirements.rst +++ b/docs/requirements/tool_requirements.rst @@ -41,12 +41,16 @@ Scope ``implementation``, the checked-in source and header files of ``cc_library`` and ``rust_library`` targets (``srcs``, ``hdrs``) and the ``CrateInfo`` sources of ``rust_binary`` targets, and shall write them sorted and - deduplicated, one workspace-relative path per line, to the allowlist file. - For workspace targets it shall additionally list the post-processing - identity of the public headers: the generated ``_virtual_includes/`` path a - header gets through ``strip_include_prefix`` or ``include_prefix`` (the path - the coverage mapping records), and headers a workspace target vendors from - an external repository. + deduplicated, one canonical path per line, to the allowlist file: the + workspace-relative path of a main-repository file, or + ``external//`` for a header a workspace target declares from an + external repository. For a header a workspace target exposes through + ``strip_include_prefix`` or ``include_prefix`` it shall additionally record, + in the path map, the generated ``/_virtual_includes//...`` path + the coverage mapping names together with the declared header it stands for, + and it shall export the listed source files themselves (``source_files`` + output group) so the reporter can read them independently of the workspace + directory. .. tool_req:: External and generated sources are excluded from the scope :id: tool_req__coverage_scope_excludes @@ -56,11 +60,14 @@ Scope :safety: QM :satisfies: stkh_req__coverage__uc_scope_completeness - ``score_coverage_scope`` shall not traverse external targets and shall not - list their files, and shall not list generated files, with one exception - each: headers a workspace target declares from an external repository, and - the ``_virtual_includes/`` identities of a workspace target's public headers - (see :need:`tool_req__coverage_scope_transitive`). + ``score_coverage_scope`` shall not list files of external targets and shall + not list generated files, with one exception: headers a workspace target + declares in its own ``hdrs`` from an external repository (see + :need:`tool_req__coverage_scope_transitive`). In particular, the public + headers a workspace target merely inherits by forwarding another target's + ``CcInfo`` (a wrapper rule around a third-party library) shall not enter + the scope, and a ``_virtual_includes/`` path shall be mapped only when the + target itself generated it. .. tool_req:: Baseline objects accompany the scope :id: tool_req__coverage_scope_baseline_objects @@ -143,8 +150,10 @@ Report :satisfies: stkh_req__coverage__uc_scope_completeness The reporter shall exclude every file with coverage data that is not in the - allowlist from all three report formats. An empty allowlist shall be an - error (exit non-zero), not an empty report. + allowlist from all three report formats, matching each excluded compiled + file exactly (an excluded ``foo/bar.h`` shall not suppress an in-scope + ``src/foo/bar.h``). An empty allowlist shall be an error (exit non-zero), + not an empty report. .. tool_req:: Untested in-scope files appear at exact 0 % :id: tool_req__coverage_report_baseline_zero @@ -192,13 +201,20 @@ Report :safety: QM :satisfies: stkh_req__coverage__uc_archive - The reporter shall rewrite the absolute workspace root and the compiler's - ``/proc/self/cwd/`` prefix in LCOV ``SF:`` records and in HTML page titles to - workspace-relative paths, and shall drop the configuration-specific - ``bazel-out//bin/`` prefix of generated headers, so that the archived - report is portable and file identity depends neither on the machine nor on - the build configuration. A generated header covered by a test binary shall - appear once, not additionally as a 0 % entry from the baseline archive. + The reporter shall name every file by its canonical path in all three + report formats: LCOV ``SF:`` records, the text summary, HTML page titles, + the HTML page location below ``coverage/`` and the index links. The + canonical path is the allowlist path; for a header compiled through a + ``_virtual_includes/`` tree it is the declared header from the scope's + path map. No absolute directory of the producing machine, no + ``/proc/self/cwd/`` prefix and no configuration-specific + ``bazel-out//bin/`` prefix shall remain, so that the archived report + is portable and file identity depends neither on the machine nor on the + build configuration. The reporter shall read the sources it renders from + the scope's exported files, so that every index link points at a generated + page; a file compiled under several paths (a declared header covered by a + test binary and again by the baseline archive, or under two include paths) + shall appear once. .. tool_req:: Report contents :id: tool_req__coverage_report_outputs diff --git a/docs/verification/verification_report.rst b/docs/verification/verification_report.rst index 54ea9c1..7daef34 100644 --- a/docs/verification/verification_report.rst +++ b/docs/verification/verification_report.rst @@ -48,7 +48,7 @@ Test inventory - 20 - merge_profraw, merge_no_data, merge_tool_error * - ``//score_coverage/tests:reporter_test`` - - 39 + - 58 - report_merged_profile, report_allowlist, report_baseline_zero, report_rlib_expansion, report_missing_baseline, report_relative_paths, report_outputs, scope_transitive @@ -67,13 +67,14 @@ Test inventory * - ``//score_coverage/tests:coverage_summary_test`` - 17 - summary_first - * - ``//score_coverage/tests/starlark:coverage_scope_tests`` (11 analysis tests) - - 11 + * - ``//score_coverage/tests/starlark:coverage_scope_tests`` (13 analysis tests) + - 13 - scope_transitive, scope_excludes, scope_baseline_objects - * - ``integration_tests/run_integration_test.sh`` (15 end-to-end checks) - - 15 - - validation_ground_truth, report_baseline_zero, gate_exit_codes, - gate_no_verdict, just_unknown_id, artifacts, summary_first + * - ``integration_tests/run_integration_test.sh`` (16 end-to-end checks) + - 16 + - validation_ground_truth, report_baseline_zero, report_relative_paths, + report_allowlist, gate_exit_codes, gate_no_verdict, just_unknown_id, + artifacts, summary_first Requirement coverage -------------------- diff --git a/integration_tests/MODULE.bazel b/integration_tests/MODULE.bazel index 5d2f65a..f7e0483 100644 --- a/integration_tests/MODULE.bazel +++ b/integration_tests/MODULE.bazel @@ -23,6 +23,12 @@ local_path_override( path = "..", ) +bazel_dep(name = "itest_external", version = "0.0.0") +local_path_override( + module_name = "itest_external", + path = "external_lib", +) + bazel_dep(name = "platforms", version = "1.0.0") bazel_dep(name = "rules_cc", version = "0.2.17") bazel_dep(name = "rules_python", version = "1.8.5") diff --git a/integration_tests/expected_lcov.dat b/integration_tests/expected_lcov.dat index 4766a68..0609f97 100644 --- a/integration_tests/expected_lcov.dat +++ b/integration_tests/expected_lcov.dat @@ -27,14 +27,25 @@ # baseline; no branches. # src/uncovered.cpp no test links against it => all lines 0, both directions # of the branch on line 18 never executed ('-'). -# src/_virtual_includes/vendored_math/vendored/inline_math.h -# header-only library behind strip_include_prefix, reported -# under the generated virtual-includes path the compiler -# records (baselibs#558). twice() (lines 18-20) is called once -# by coverable_test; never_inlined() (22-24) is never called -# and clang still emits its mapping => 3 of 6 lines, no -# branches. Exactly one record: the baseline variant of the -# same header is suppressed. +# src/vendored/include/vendored/inline_math.h +# header-only library behind strip_include_prefix. The +# compiler records the generated _virtual_includes/ path; the +# report names the declared header (baselibs#558, +# coverage_tool#5). twice() (lines 18-20) is called once by +# coverable_test; never_inlined() (22-24) is never called and +# clang still emits its mapping => 3 of 6 lines, no branches. +# Exactly one record: the baseline variant of the same header +# is suppressed. +# external/itest_external+/include/vext/vext.h +# header of the external module vendored by +# //third_party:vendored_ext behind strip_include_prefix: +# same shape, thrice() (18-20) called once, never_used() +# (22-24) at 0 => 3 of 6 lines. Reported under the file's own +# path in the external repository. +# (absent) external/itest_external+/extlib.cpp and extlib.h: compiled +# with coverage and executed by coverable_test, but reached +# only through the forwarding target //third_party:extlib, so +# not in scope and not in this file (coverage_tool#5). SF:rust/lib.rs DA:16,2 DA:17,2 @@ -103,7 +114,19 @@ BRH:0 LF:6 LH:0 end_of_record -SF:src/_virtual_includes/vendored_math/vendored/inline_math.h +SF:src/vendored/include/vendored/inline_math.h +DA:18,1 +DA:19,1 +DA:20,1 +DA:22,0 +DA:23,0 +DA:24,0 +BRF:0 +BRH:0 +LF:6 +LH:3 +end_of_record +SF:external/itest_external+/include/vext/vext.h DA:18,1 DA:19,1 DA:20,1 diff --git a/integration_tests/external_lib/BUILD b/integration_tests/external_lib/BUILD new file mode 100644 index 0000000..355bc7c --- /dev/null +++ b/integration_tests/external_lib/BUILD @@ -0,0 +1,24 @@ +# ******************************************************************************* +# Copyright (c) 2026 Contributors to the Eclipse Foundation +# +# See the NOTICE file(s) distributed with this work for additional +# information regarding copyright ownership. +# +# This program and the accompanying materials are made available under the +# terms of the Apache License Version 2.0 which is available at +# https://www.apache.org/licenses/LICENSE-2.0 +# +# SPDX-License-Identifier: Apache-2.0 +# ******************************************************************************* +load("@rules_cc//cc:cc_library.bzl", "cc_library") + +package(default_visibility = ["//visibility:public"]) + +exports_files(["include/vext/vext.h"]) + +cc_library( + name = "extlib", + srcs = ["extlib.cpp"], + hdrs = ["include/extlib/extlib.h"], + strip_include_prefix = "include", +) diff --git a/integration_tests/external_lib/MODULE.bazel b/integration_tests/external_lib/MODULE.bazel new file mode 100644 index 0000000..6583134 --- /dev/null +++ b/integration_tests/external_lib/MODULE.bazel @@ -0,0 +1,18 @@ +# ******************************************************************************* +# Copyright (c) 2026 Contributors to the Eclipse Foundation +# +# See the NOTICE file(s) distributed with this work for additional +# information regarding copyright ownership. +# +# This program and the accompanying materials are made available under the +# terms of the Apache License Version 2.0 which is available at +# https://www.apache.org/licenses/LICENSE-2.0 +# +# SPDX-License-Identifier: Apache-2.0 +# ******************************************************************************* +# Stand-in for a third-party module: a compiled library that a workspace rule +# forwards (must stay OUT of the coverage scope) and a header a workspace +# target vendors behind strip_include_prefix (IN scope, baselibs#558). +module(name = "itest_external") + +bazel_dep(name = "rules_cc", version = "0.2.17") diff --git a/integration_tests/external_lib/extlib.cpp b/integration_tests/external_lib/extlib.cpp new file mode 100644 index 0000000..5fa8b3f --- /dev/null +++ b/integration_tests/external_lib/extlib.cpp @@ -0,0 +1,21 @@ +/******************************************************************************** + * Copyright (c) 2026 Contributors to the Eclipse Foundation + * + * See the NOTICE file(s) distributed with this work for additional + * information regarding copyright ownership. + * + * This program and the accompanying materials are made available under the + * terms of the Apache License Version 2.0 which is available at + * https://www.apache.org/licenses/LICENSE-2.0 + * + * SPDX-License-Identifier: Apache-2.0 + ********************************************************************************/ +#include "extlib/extlib.h" + +namespace extlib { + +int add(int a, int b) { + return a + b; +} + +} // namespace extlib diff --git a/integration_tests/external_lib/include/extlib/extlib.h b/integration_tests/external_lib/include/extlib/extlib.h new file mode 100644 index 0000000..33f1daf --- /dev/null +++ b/integration_tests/external_lib/include/extlib/extlib.h @@ -0,0 +1,24 @@ +/******************************************************************************** + * Copyright (c) 2026 Contributors to the Eclipse Foundation + * + * See the NOTICE file(s) distributed with this work for additional + * information regarding copyright ownership. + * + * This program and the accompanying materials are made available under the + * terms of the Apache License Version 2.0 which is available at + * https://www.apache.org/licenses/LICENSE-2.0 + * + * SPDX-License-Identifier: Apache-2.0 + ********************************************************************************/ +#ifndef ITEST_EXTERNAL_EXTLIB_H +#define ITEST_EXTERNAL_EXTLIB_H + +namespace extlib { + +// Third-party code: instrumented like everything else under +// --experimental_use_llvm_covmap, but not part of the workspace's scope. +int add(int a, int b); + +} // namespace extlib + +#endif // ITEST_EXTERNAL_EXTLIB_H diff --git a/integration_tests/external_lib/include/vext/vext.h b/integration_tests/external_lib/include/vext/vext.h new file mode 100644 index 0000000..687085b --- /dev/null +++ b/integration_tests/external_lib/include/vext/vext.h @@ -0,0 +1,28 @@ +/******************************************************************************** + * Copyright (c) 2026 Contributors to the Eclipse Foundation + * + * See the NOTICE file(s) distributed with this work for additional + * information regarding copyright ownership. + * + * This program and the accompanying materials are made available under the + * terms of the Apache License Version 2.0 which is available at + * https://www.apache.org/licenses/LICENSE-2.0 + * + * SPDX-License-Identifier: Apache-2.0 + ********************************************************************************/ +#ifndef ITEST_EXTERNAL_VEXT_H +#define ITEST_EXTERNAL_VEXT_H + +namespace vext { + +inline int thrice(int value) { + return value * 3; +} + +inline int never_used(int value) { + return value + 1; +} + +} // namespace vext + +#endif // ITEST_EXTERNAL_VEXT_H diff --git a/integration_tests/run_integration_test.sh b/integration_tests/run_integration_test.sh index 73e0693..2808705 100755 --- a/integration_tests/run_integration_test.sh +++ b/integration_tests/run_integration_test.sh @@ -150,6 +150,46 @@ fi rm -f actual_normalised.dat expected_normalised.dat echo "OK: LCOV matches the ground truth" +echo "=== Every index link must point at an existing page; no machine or config paths ===" +rm -rf link_check && mkdir link_check +unzip -q coverage_artifacts.zip -d link_check +HTML_DIR="link_check/artifacts/coverage_linux" +[[ -f "${HTML_DIR}/index.html" ]] || { echo "ERROR: ${HTML_DIR}/index.html missing" >&2; exit 1; } +LINKS="$(grep -oE "href='coverage/[^']+\.html'" "${HTML_DIR}/index.html" | sed -E "s/^href='//; s/'$//")" +[[ -n "${LINKS}" ]] || { echo "ERROR: no source links in index.html" >&2; exit 1; } +while IFS= read -r link; do + if [[ ! -f "${HTML_DIR}/${link}" ]]; then + echo "ERROR: index.html links to ${link}, which was not generated" >&2 + exit 1 + fi + case "${link}" in + coverage/bazel-out/*|coverage/home/*|coverage/tmp/*|*/_virtual_includes/*) + echo "ERROR: index.html link is not a canonical workspace path: ${link}" >&2 + exit 1 ;; + esac + # The page's stylesheet link must resolve from the page's location. + page_dir="$(dirname "${HTML_DIR}/${link}")" + css="$(grep -oE "href='(\.\./)*style\.css'" "${HTML_DIR}/${link}" | head -1 | sed -E "s/^href='//; s/'$//")" + if [[ -z "${css}" || ! -f "${page_dir}/${css}" ]]; then + echo "ERROR: ${link}: stylesheet link '${css}' does not resolve" >&2 + exit 1 + fi +done <<< "${LINKS}" +for page in "coverage/src/vendored/include/vendored/inline_math.h.html" \ + "coverage/external/itest_external+/include/vext/vext.h.html"; do + grep -qF "href='${page}'" "${HTML_DIR}/index.html" || { echo "ERROR: ${page} not linked from index.html" >&2; exit 1; } +done +if grep -q "itest_external+/extlib" "${HTML_DIR}/index.html"; then + echo "ERROR: forwarded third-party library leaked into the HTML report" >&2 + exit 1 +fi +if grep -q "extlib" lcov.dat; then + echo "ERROR: forwarded third-party library leaked into the LCOV" >&2 + exit 1 +fi +rm -rf link_check +echo "OK: $(echo "${LINKS}" | wc -l) index links resolve, canonical paths only, third-party code excluded" + echo "=== Covered files must be present with hits ===" grep -q "SF:.*src/coverable.cpp" lcov.dat || { echo "ERROR: coverable.cpp missing" >&2; exit 1; } grep -q "SF:.*rust/lib.rs" lcov.dat || { echo "ERROR: lib.rs missing" >&2; exit 1; } diff --git a/integration_tests/src/BUILD b/integration_tests/src/BUILD index 71e2aee..5425fc7 100644 --- a/integration_tests/src/BUILD +++ b/integration_tests/src/BUILD @@ -46,5 +46,7 @@ cc_test( deps = [ ":coverable", ":vendored_math", + "//third_party:extlib", + "//third_party:vendored_ext", ], ) diff --git a/integration_tests/src/coverable_test.cpp b/integration_tests/src/coverable_test.cpp index 7dbe368..0755758 100644 --- a/integration_tests/src/coverable_test.cpp +++ b/integration_tests/src/coverable_test.cpp @@ -12,8 +12,10 @@ ********************************************************************************/ #include +#include "extlib/extlib.h" #include "src/coverable.h" #include "vendored/inline_math.h" +#include "vext/vext.h" // Deliberately exercises only the negative and zero branches; the positive // branch stays uncovered (and justified via the COV_JUSTIFIED marker). @@ -28,5 +30,14 @@ int main() { if (coverage_integration::twice(21) != 42) { return 1; } + // Third-party code reached through a forwarding workspace target: executed, + // instrumented, and expected to stay out of the report. + if (extlib::add(1, 2) != 3) { + return 1; + } + // Header vendored from the external module: in scope, called once. + if (vext::thrice(2) != 6) { + return 1; + } return 0; } diff --git a/integration_tests/third_party/BUILD b/integration_tests/third_party/BUILD new file mode 100644 index 0000000..42418fb --- /dev/null +++ b/integration_tests/third_party/BUILD @@ -0,0 +1,32 @@ +# ******************************************************************************* +# Copyright (c) 2026 Contributors to the Eclipse Foundation +# +# See the NOTICE file(s) distributed with this work for additional +# information regarding copyright ownership. +# +# This program and the accompanying materials are made available under the +# terms of the Apache License Version 2.0 which is available at +# https://www.apache.org/licenses/LICENSE-2.0 +# +# SPDX-License-Identifier: Apache-2.0 +# ******************************************************************************* +load("@rules_cc//cc:cc_library.bzl", "cc_library") +load(":forward.bzl", "forward_cc") + +package(default_visibility = ["//visibility:public"]) + +# Forwarded third-party library: compiled with coverage, linked into the +# test binary, and NOT part of the report (coverage_tool#5). +forward_cc( + name = "extlib", + dep = "@itest_external//:extlib", +) + +# Header vendored from the external module behind strip_include_prefix: part +# of this workspace target and therefore IN the report, under its source path +# (baselibs#558). +cc_library( + name = "vendored_ext", + hdrs = ["@itest_external//:include/vext/vext.h"], + strip_include_prefix = "/include", +) diff --git a/integration_tests/third_party/forward.bzl b/integration_tests/third_party/forward.bzl new file mode 100644 index 0000000..a74ca79 --- /dev/null +++ b/integration_tests/third_party/forward.bzl @@ -0,0 +1,30 @@ +# ******************************************************************************* +# Copyright (c) 2026 Contributors to the Eclipse Foundation +# +# See the NOTICE file(s) distributed with this work for additional +# information regarding copyright ownership. +# +# This program and the accompanying materials are made available under the +# terms of the Apache License Version 2.0 which is available at +# https://www.apache.org/licenses/LICENSE-2.0 +# +# SPDX-License-Identifier: Apache-2.0 +# ******************************************************************************* +"""Workspace rule that forwards an external library's CcInfo. + +Mirrors how consumers wrap third-party libraries to apply a transition +(eclipse-score/baselibs third_party/openssl). The forwarded headers become +this target's direct_public_headers; they must not enter the coverage scope +(eclipse-score/coverage_tool#5). +""" + +def _forward_cc_impl(ctx): + dep = ctx.attr.dep + return [dep[DefaultInfo], dep[CcInfo]] + +forward_cc = rule( + implementation = _forward_cc_impl, + attrs = { + "dep": attr.label(providers = [CcInfo], mandatory = True), + }, +) diff --git a/integration_tests/tools/coverage/BUILD b/integration_tests/tools/coverage/BUILD index 1e828b7..75a1f40 100644 --- a/integration_tests/tools/coverage/BUILD +++ b/integration_tests/tools/coverage/BUILD @@ -26,6 +26,11 @@ score_coverage_scope( "//src:coverable", "//src:uncovered", "//src:vendored_math", + # Wrapper of a third-party library: visited by the aspect, must + # contribute nothing (coverage_tool#5). + "//third_party:extlib", + # Header vendored from an external module (baselibs#558). + "//third_party:vendored_ext", ], ) diff --git a/score_coverage/coverage_scope.bzl b/score_coverage/coverage_scope.bzl index 11a52ba..a20f968 100644 --- a/score_coverage/coverage_scope.bzl +++ b/score_coverage/coverage_scope.bzl @@ -25,9 +25,20 @@ transitive deps. At each node it collects the actual source files. - rust_binary (no CcInfo): source files come from CrateInfo.srcs; the coverage-built executable itself serves as the baseline object. -The resulting allowlist contains one source file path per line (relative to the -workspace root). The coverage reporter uses this to restrict reports to exactly -the files that are part of the covered implementation. +The rule writes three text files the reporter consumes: + +- ``_allowlist.txt``: one canonical source file path per line + (``/`` for the main repository, ``external//`` for a + header a workspace target vendors from an external repository). +- ``_path_map.txt``: ``\t`` per header a + workspace target exposes through ``strip_include_prefix`` / ``include_prefix``. + The compiler records the generated ``/_virtual_includes//...`` + path; the reporter maps it back to the declared header. +- ``_objects.txt``: archives / executables for the zero-coverage baseline. + +The source files themselves are exported in the ``source_files`` output group +so the reporter can stage them for llvm-cov from its runfiles, independent of +what happens to exist in the workspace directory at report time. """ load("@rules_rust//rust:rust_common.bzl", "CrateInfo") @@ -39,7 +50,9 @@ load("@rules_rust//rust:rust_common.bzl", "CrateInfo") _CoverageScopeInfo = provider( doc = "Carries source file paths and object files collected by the coverage scope aspect.", fields = { - "source_files": "Depset of source file path strings (workspace-relative).", + "source_files": "Depset of canonical source file path strings.", + "source_file_objects": "Depset of the File objects behind source_files (staged by the reporter).", + "path_map": "Depset of '\\t' strings for headers behind include prefixes.", "object_files": "Depset of compiled archive/executable File objects for baseline coverage.", }, ) @@ -63,38 +76,74 @@ def _workspace_relative(f): return "external/" + f.short_path[3:] return f.short_path +def _match_declared(tail, declared_paths): + """The declared header a virtual-includes path ends with, at a path-component boundary.""" + for path in declared_paths: + if path == tail or path.endswith("/" + tail): + return path + return None + +def _virtual_include_map(target, ctx, declared_hdrs): + """'\\t' for the headers THIS target exposes via a _virtual_includes/ tree. + + Only headers the target generated itself are considered (``owner``), so a + workspace rule that merely forwards the CcInfo of an external library does + not contribute that library's headers. + """ + marker = "/_virtual_includes/" + target.label.name + "/" + include_prefix = getattr(ctx.rule.attr, "include_prefix", "") or "" + include_prefix = include_prefix.strip("/") + entries = [] + for f in target[CcInfo].compilation_context.direct_public_headers: + if f.owner != target.label or marker not in f.short_path: + continue + tail = f.short_path.split(marker, 1)[1] + if include_prefix and tail.startswith(include_prefix + "/"): + tail = tail[len(include_prefix) + 1:] + canonical = _match_declared(tail, declared_hdrs) + if canonical: + entries.append(f.short_path + "\t" + canonical) + return entries + def _coverage_scope_aspect_impl(target, ctx): """Collects source file paths and archive files from the build graph.""" direct_files = [] + direct_file_objects = [] + direct_map = [] direct_archives = [] transitive = [] + transitive_file_objects = [] + transitive_map = [] transitive_archives = [] # At cc_library / rust_library targets (rust_library provides CcInfo with # its rlib exposed as a .a symlink): collect srcs, hdrs, and static archive if CcInfo in target: in_workspace = _is_workspace_target(target) - for attr_name in ["srcs", "hdrs"]: - if hasattr(ctx.rule.attr, attr_name): - for src in getattr(ctx.rule.attr, attr_name): - for f in src.files.to_list(): - if not f.path.startswith("external/") and f.is_source: - direct_files.append(f.short_path) - - if in_workspace: - # Post-processing identity of the public headers. With - # strip_include_prefix / include_prefix, Bazel compiles against a - # generated _virtual_includes/ symlink tree and the coverage mapping - # records THAT path, never the declared header label. Headers a - # workspace target vendors from an external repository are part of - # that target and therefore in scope as well - # (eclipse-score/baselibs#558). Other generated headers stay out. - for f in target[CcInfo].compilation_context.direct_public_headers: - if "/_virtual_includes/" in f.short_path or f.is_source: - direct_files.append(_workspace_relative(f)) - - # Only collect workspace-internal labels and archives + declared_hdrs = [] if in_workspace: + # The checked-in files the target declares. A header a workspace + # target vendors from an external repository is part of that + # target and therefore in scope (eclipse-score/baselibs#558); + # files of external TARGETS never are (the aspect still visits + # them, but in_workspace is False there). + for attr_name in ["srcs", "hdrs"]: + if hasattr(ctx.rule.attr, attr_name): + for src in getattr(ctx.rule.attr, attr_name): + for f in src.files.to_list(): + if f.is_source: + path = _workspace_relative(f) + direct_files.append(path) + direct_file_objects.append(f) + if attr_name == "hdrs": + declared_hdrs.append(path) + + # With strip_include_prefix / include_prefix, Bazel compiles + # against a generated _virtual_includes/ symlink tree and the + # coverage mapping records THAT path, never the declared header. + # Record the mapping so the reporter can report the declared file. + direct_map.extend(_virtual_include_map(target, ctx, declared_hdrs)) + # Collect .a archive files for baseline coverage. for linker_input in target[CcInfo].linking_context.linker_inputs.to_list(): for lib in linker_input.libraries: @@ -108,6 +157,7 @@ def _coverage_scope_aspect_impl(target, ctx): for f in target[CrateInfo].srcs.to_list(): if not f.path.startswith("external/") and f.is_source: direct_files.append(f.short_path) + direct_file_objects.append(f) out = target[CrateInfo].output if out and "/external/" not in out.path and not out.path.startswith("external/"): direct_archives.append(out) @@ -118,10 +168,14 @@ def _coverage_scope_aspect_impl(target, ctx): for dep in getattr(ctx.rule.attr, attr_name): if _CoverageScopeInfo in dep: transitive.append(dep[_CoverageScopeInfo].source_files) + transitive_file_objects.append(dep[_CoverageScopeInfo].source_file_objects) + transitive_map.append(dep[_CoverageScopeInfo].path_map) transitive_archives.append(dep[_CoverageScopeInfo].object_files) return [_CoverageScopeInfo( source_files = depset(direct_files, transitive = transitive), + source_file_objects = depset(direct_file_objects, transitive = transitive_file_objects), + path_map = depset(direct_map, transitive = transitive_map), object_files = depset(direct_archives, transitive = transitive_archives), )] @@ -136,19 +190,26 @@ _coverage_scope_aspect = aspect( # ============================================================================= def _coverage_scope_impl(ctx): - """Aggregates source file paths from all deps and writes allowlist + baseline objects.""" + """Aggregates aspect results into the allowlist, path map and baseline objects manifest.""" all_files = {} + all_map = {} all_objects = [] + all_sources = [] for dep in ctx.attr.deps: if _CoverageScopeInfo in dep: for path in dep[_CoverageScopeInfo].source_files.to_list(): if path: all_files[path] = True + for entry in dep[_CoverageScopeInfo].path_map.to_list(): + all_map[entry] = True all_objects.append(dep[_CoverageScopeInfo].object_files) + all_sources.append(dep[_CoverageScopeInfo].source_file_objects) sorted_files = sorted(all_files.keys()) + sorted_map = sorted(all_map.keys()) object_depset = depset(transitive = all_objects) + source_depset = depset(transitive = all_sources) # Write the allowlist file output = ctx.actions.declare_file(ctx.attr.name + "_allowlist.txt") @@ -157,6 +218,13 @@ def _coverage_scope_impl(ctx): content = "\n".join(sorted_files) + "\n" if sorted_files else "", ) + # Write the virtual-includes -> declared header map + map_output = ctx.actions.declare_file(ctx.attr.name + "_path_map.txt") + ctx.actions.write( + output = map_output, + content = "\n".join(sorted_map) + "\n" if sorted_map else "", + ) + # Write archive file paths for baseline coverage (reporter uses these as --object args) archive_paths = sorted(set([f.short_path for f in object_depset.to_list()])) objects_output = ctx.actions.declare_file(ctx.attr.name + "_objects.txt") @@ -166,11 +234,13 @@ def _coverage_scope_impl(ctx): ) return [ - DefaultInfo(files = depset([output, objects_output], transitive = [object_depset])), + DefaultInfo(files = depset([output, map_output, objects_output], transitive = [object_depset])), OutputGroupInfo( allowlist = depset([output]), + path_map = depset([map_output]), objects = depset([objects_output]), object_files = object_depset, + source_files = source_depset, ), ] @@ -213,11 +283,14 @@ coverage_scope = rule( Uses an aspect to traverse the listed targets (cc_library, rust_library, rust_binary) and their transitive deps, collecting all source files - (srcs + hdrs / CrateInfo.srcs). Outputs a text file with one - workspace-relative file path per line. - - This allowlist is consumed by the coverage reporter to restrict coverage - reporting to exactly the source files that are part of the implementation. + (srcs + hdrs / CrateInfo.srcs). Outputs the allowlist (one canonical file + path per line), the virtual-includes path map and the baseline objects + manifest, and exports the source files themselves in the ``source_files`` + output group. + + The coverage reporter restricts reporting to exactly the allowlisted + files, reports headers behind include prefixes under their declared path, + and stages the sources so every HTML page can be rendered. """, attrs = { "deps": attr.label_list( diff --git a/score_coverage/reporter.py b/score_coverage/reporter.py index d3e0b14..4fa1d3c 100644 --- a/score_coverage/reporter.py +++ b/score_coverage/reporter.py @@ -26,9 +26,11 @@ import json import os import re +import shutil import subprocess import sys import zipfile +from dataclasses import dataclass, field from pathlib import Path from typing import Protocol @@ -102,65 +104,15 @@ def main(argv: list[str] | None = None) -> None: # Expand such archives into their object members. baseline_objects = expand_rlib_archives(baseline_objects, Path.cwd() / "rlib_baseline_objects") - # Determine filter regexes: prefer allowlist-based filtering, fall back to manual regexes. - workspace_root = args.workspace_root - allowlist_files = [] - filter_regexes = [] - baseline_only_archives = [] - baseline_only_files = set() + selection, path_map, root, regexes = prepare_sources( + r, args, llvm_bin_path, sorted_objects, str(merged_profdata), baseline_objects + ) + # All valid baseline archives are passed when baseline-only files exist; + # _filter_lcov keeps only those files' records. + baseline_only_archives = list(baseline_objects) if selection.baseline_only else [] - if args.coverage_allowlist: - allowlist_files = load_coverage_allowlist(r, args.coverage_allowlist) - if allowlist_files: - print(f"INFO: Using coverage allowlist with {len(allowlist_files)} source files.", file=sys.stderr) - allowlist_set = set(allowlist_files) - - # Get files covered by test binaries (raw covmap path -> normalized name). - test_covered = get_covered_files(llvm_bin_path, sorted_objects, str(merged_profdata), workspace_root) - test_covered_files = set(test_covered.values()) - print(f"INFO: Test binaries cover {len(test_covered_files)} files.", file=sys.stderr) - - # Get files from baseline archives via a SEPARATE llvm-cov run. - # Combining archives with test binaries in a single llvm-cov invocation - # causes some files to vanish (suspected llvm-cov deduplication issue). - # Some archives may have oversized coverage mappings ("malformed coverage - # data"), so we iteratively remove bad ones. - baseline_covered: dict[str, str] = {} - baseline_files = set() - if baseline_objects: - baseline_covered = get_covered_files(llvm_bin_path, baseline_objects, None, workspace_root) - baseline_files = set(baseline_covered.values()) - print(f"INFO: Baseline archives contain {len(baseline_files)} files.", file=sys.stderr) - - # Files only in baseline archives (not in any test binary). - baseline_only_files = (baseline_files & allowlist_set) - test_covered_files - baseline_only_archives = [] - if baseline_only_files: - print( - f"INFO: {len(baseline_only_files)} allowlisted files only in baseline " - f"(e.g., {sorted(baseline_only_files)[:5]})", - file=sys.stderr, - ) - # Use all valid baseline archives for LCOV generation. - # The _filter_lcov function will filter to only baseline-only files. - baseline_only_archives = list(baseline_objects) - - # Union of test + baseline for exclude-set calculation. A generated - # header (virtual includes) covered by a test binary also appears in - # the baseline archive under another configuration prefix; that raw - # variant would show up as a second, 0% row and is excluded here. - all_covered_files = test_covered_files | baseline_files - files_to_exclude = (all_covered_files - allowlist_set) | redundant_baseline_variants( - test_covered, baseline_covered - ) - filter_regexes = [re.escape(f) + "$" for f in sorted(files_to_exclude)] - print(f"INFO: Excluding {len(filter_regexes)} files not in allowlist.", file=sys.stderr) - else: - print("ERROR: Coverage allowlist is empty, falling back to filter_regexes.txt.", file=sys.stderr) - sys.exit(-1) cxxfilt = find_cxxfilt(llvm_bin_path, r, args.llvm_cxxfilt) profile = str(merged_profdata) - regexes = sorted(filter_regexes) def show_html(objects: list[str]) -> None: run_llvm_cov_show( @@ -168,7 +120,7 @@ def show_html(objects: list[str]) -> None: objects, profile, regexes, - workspace_root, + root, output_format="html", html_report_dir=html_report_dir, cxxfilt=cxxfilt, @@ -189,29 +141,31 @@ def show_html(objects: list[str]) -> None: else: show_html(sorted_objects) - # Rewrite absolute workspace paths in the HTML pages so unpacked report - # archives remain browsable outside the machine that produced them. - _make_html_paths_relative(html_report_dir, workspace_root) + # File the pages under canonical, machine-independent paths so the index + # links resolve wherever the archive is unpacked, and name the sources + # the same way in the page titles. + relocate_html_pages(html_report_dir, root, path_map) + _make_html_paths_relative(html_report_dir, root, path_map) # Generate LCOV report from test binaries. lcov_report_dir = Path.cwd() / "lcov_report" lcov_report_dir.mkdir(exist_ok=True) - lcov_content = run_llvm_cov_export(llvm_bin_path, sorted_objects, profile, regexes, workspace_root).stdout + lcov_content = _make_lcov_paths_relative( + run_llvm_cov_export(llvm_bin_path, sorted_objects, profile, regexes, root).stdout, root, path_map + ) # If there are baseline-only files, generate a separate baseline LCOV and merge. if baseline_only_archives: # No filtering: only the needed archives are passed. - baseline_lcov = run_llvm_cov_export(llvm_bin_path, baseline_only_archives, None, [], workspace_root) + baseline_lcov = run_llvm_cov_export(llvm_bin_path, baseline_only_archives, None, [], root) if baseline_lcov.stdout: # Filter baseline LCOV to only include baseline-only files. - filtered_baseline = _filter_lcov(baseline_lcov.stdout, baseline_only_files) + filtered_baseline = _filter_lcov( + _make_lcov_paths_relative(baseline_lcov.stdout, root, path_map), selection.baseline_only + ) if filtered_baseline: lcov_content += filtered_baseline - print(f"INFO: Merged baseline LCOV for {len(baseline_only_files)} files.", file=sys.stderr) - - # Strip the absolute workspace root from SF: records so the LCOV file is - # portable (IDE gutters, SonarQube, reports produced inside containers). - lcov_content = _make_lcov_paths_relative(lcov_content, workspace_root) + print(f"INFO: Merged baseline LCOV for {len(selection.baseline_only)} files.", file=sys.stderr) with open(lcov_report_dir / "lcov.dat", "w", encoding="utf-8") as f: f.write(lcov_content) @@ -219,10 +173,11 @@ def show_html(objects: list[str]) -> None: # Generate text summary. text_report_dir = Path.cwd() / "text_report" text_report_dir.mkdir(exist_ok=True) - summary = run_llvm_cov_report(llvm_bin_path, sorted_objects, profile, regexes, workspace_root) + summary = run_llvm_cov_report(llvm_bin_path, sorted_objects, profile, regexes, root) + summary_text = _canonicalize_summary(summary.stdout, root, path_map) with open(text_report_dir / "summary.txt", "w", encoding="utf-8") as f: - f.write(summary.stdout) - print(summary.stdout, file=sys.stderr) + f.write(summary_text) + print(summary_text, file=sys.stderr) # Package everything into the output zip. directories = [html_report_dir, lcov_report_dir, text_report_dir] @@ -260,19 +215,231 @@ def redundant_baseline_variants(test_covered: dict[str, str], baseline_covered: return {raw for raw, name in baseline_covered.items() if name in covered_names and raw not in test_covered} -def _make_lcov_paths_relative(lcov_content: str, workspace_root: str) -> str: - """Rewrite absolute SF: paths under workspace_root to workspace-relative ones. +def canonical_path(path: str, path_map: dict[str, str] | None = None) -> str: + """The name a file is reported under. + + Drops the configuration prefix of a generated header and maps a + ``_virtual_includes/`` path to the header it was generated from (the + scope's path map). Plain source paths are returned unchanged. + """ + stripped = strip_config_prefix(path) + if path_map: + return path_map.get(stripped, stripped) + return stripped + + +def exclusion_regex(raw: str, roots: list[str]) -> str: + """``--ignore-filename-regex`` that matches exactly one compiled file. + + ``raw`` is the file's covmap path with the compilation directory + stripped (``src/a.cpp``, ``bazel-out//bin/.../x.h``). llvm-cov + matches the filter against the name joined with the recorded + compilation directory (``/proc/self/cwd/``) or with the + ``--compilation-dir`` we pass (``/``); a Rust file may also + appear bare. Anchoring on both ends keeps an excluded ``foo/bar.h`` from + also suppressing an in-scope ``src/foo/bar.h``. llvm-cov uses POSIX + extended regular expressions: no ``(?:`` groups. + """ + prefixes = ["/proc/self/cwd/"] + [root.rstrip("/") + "/" for root in roots] + return "^(" + "|".join(re.escape(prefix) for prefix in prefixes) + ")?" + re.escape(raw) + "$" + + +def duplicate_test_variants(test_covered: dict[str, str]) -> dict[str, list[str]]: + """Raw test-binary paths to drop because another raw path of the same file is kept. + + A file compiled under two names (its declared path and a virtual-includes + path) would otherwise produce two entries for one canonical name. The + variant equal to the canonical name is kept, else the first in sort order. + Returns {canonical name: dropped raw paths}. + """ + by_name: dict[str, list[str]] = {} + for raw, name in sorted(test_covered.items()): + by_name.setdefault(name, []).append(raw) + dropped: dict[str, list[str]] = {} + for name, raws in by_name.items(): + if len(raws) > 1: + keep = name if name in raws else raws[0] + dropped[name] = [raw for raw in raws if raw != keep] + return dropped + + +@dataclass +class FileSelection: + """Which compiled files stay in the report and under which name.""" + + staged: dict[str, str] = field(default_factory=dict) + """raw covmap path -> canonical name of every file that stays in the report.""" + excluded: set[str] = field(default_factory=set) + """raw covmap paths suppressed through --ignore-filename-regex.""" + baseline_only: set[str] = field(default_factory=set) + """canonical names that only the baseline archives contain (0 % entries).""" + duplicates: dict[str, list[str]] = field(default_factory=dict) + """canonical name -> raw variants dropped in favour of another variant.""" + + +def select_files( + test_covered: dict[str, str], + baseline_covered: dict[str, str], + allowlist: set[str] | None, +) -> FileSelection: + """Apply the scope allowlist to the raw files of test binaries and baseline archives. + + ``allowlist`` is a set of canonical names; ``None`` keeps every file. + """ + everything = {**baseline_covered, **test_covered} + + def in_scope(name: str) -> bool: + return allowlist is None or name in allowlist + + excluded = {raw for raw, name in everything.items() if not in_scope(name)} + excluded |= redundant_baseline_variants(test_covered, baseline_covered) + duplicates = duplicate_test_variants(test_covered) + for raws in duplicates.values(): + excluded.update(raws) + staged = {raw: name for raw, name in everything.items() if raw not in excluded} + baseline_only = {name for name in set(baseline_covered.values()) - set(test_covered.values()) if in_scope(name)} + return FileSelection(staged=staged, excluded=excluded, baseline_only=baseline_only, duplicates=duplicates) + + +def resolve_source(runfiles: RunfilesLike, canonical: str, workspace_root: str) -> str | None: + """Absolute path of an in-scope source: from the reporter's runfiles, else the workspace.""" + if canonical.startswith("external/"): + candidates = [runfiles.Rlocation(canonical[len("external/") :])] + else: + candidates = [runfiles.Rlocation(os.path.join("_main", canonical)), os.path.join(workspace_root, canonical)] + for candidate in candidates: + if candidate and os.path.isfile(candidate): + return candidate + return None + + +def stage_sources( + source_root: Path, + staged: dict[str, str], + runfiles: RunfilesLike, + workspace_root: str, +) -> list[str]: + """Create ``source_root/`` links to the real sources. + + llvm-cov then reads every in-scope file through one + ``--path-equivalence=/proc/self/cwd/,`` (C++) and + ``--compilation-dir=`` (Rust). Returns the canonical names + whose source could not be located. + """ + missing = [] + for raw, canonical in sorted(staged.items()): + if raw.startswith("/"): + continue # an absolute covmap path is read as it is + real = resolve_source(runfiles, canonical, workspace_root) + if real is None: + missing.append(canonical) + continue + link = source_root / raw + link.parent.mkdir(parents=True, exist_ok=True) + if not link.is_symlink() and not link.exists(): + link.symlink_to(os.path.realpath(real)) + return sorted(set(missing)) + + +_ASSET_LINK_RE = re.compile(r"((?:href|src)=')((?:\.\./)*)(style\.css|control\.js)'") + + +def _retarget_assets(text: str, depth: int) -> str: + """Point a page's style.css / control.js links ``depth`` directories up.""" + up = "../" * depth + return _ASSET_LINK_RE.sub(lambda m: m.group(1) + up + m.group(3) + "'", text) + + +def relocate_html_pages(html_dir: Path, source_root: str, path_map: dict[str, str] | None = None) -> dict[str, str]: + """Move llvm-cov's pages from ``coverage//.html`` to ``coverage/.html``. + + llvm-cov files each page under the absolute path it read the source from. + After the move the archive contains no machine-specific paths, every + index link points at a page that exists, and a header behind an include + prefix is filed under its declared path. Returns {old href: new href}. + """ + coverage_dir = html_dir / "coverage" + root_rel = source_root.strip("/") + base = coverage_dir / root_rel + moves: dict[str, str] = {} + for page in sorted(base.rglob("*.html")) if base.is_dir() else []: + raw = page.relative_to(base).as_posix()[: -len(".html")] + canonical = canonical_path(raw, path_map) + target = coverage_dir / (canonical + ".html") + if target.exists(): + print(f"WARNING: {canonical} was rendered twice; keeping the first page", file=sys.stderr) + page.unlink() + else: + target.parent.mkdir(parents=True, exist_ok=True) + page.rename(target) + text = target.read_text(encoding="utf-8", errors="replace") + target.write_text(_retarget_assets(text, canonical.count("/") + 1), encoding="utf-8") + moves[f"coverage/{root_rel}/{raw}.html"] = f"coverage/{canonical}.html" + shutil.rmtree(base, ignore_errors=True) + parent = base.parent + while parent != coverage_dir: + try: + parent.rmdir() + except OSError: + break + parent = parent.parent + index = html_dir / "index.html" + if index.is_file(): + text = index.read_text(encoding="utf-8", errors="replace") + for old_href, new_href in moves.items(): + label = new_href[len("coverage/") : -len(".html")] + text = re.sub( + r"[^<]*", + lambda _m, new_href=new_href, label=label: f"{label}", + text, + ) + # A row whose source llvm-cov could not read has no page; keep the + # row (its numbers are valid) but not the dead link. + text = re.sub( + r"[^<]*", + lambda m: canonical_path(m.group(1), path_map), + text, + ) + index.write_text(text, encoding="utf-8") + return moves + + +def _canonicalize_summary(text: str, source_root: str, path_map: dict[str, str] | None = None) -> str: + """Name the files of an llvm-cov text report the way LCOV and HTML do.""" + prefixes = (source_root.rstrip("/") + "/", "/proc/self/cwd/") + lines = [] + for line in text.splitlines(keepends=True): + token = line.split(" ", 1)[0] + name = token + for prefix in prefixes: + if name.startswith(prefix): + name = name[len(prefix) :] + break + name = canonical_path(name, path_map) if "/" in token else token + if name == token: + lines.append(line) + else: + lines.append(name + " " * max(len(token) - len(name), 0) + line[len(token) :]) + return "".join(lines) + + +def _make_lcov_paths_relative(lcov_content: str, source_root: str, path_map: dict[str, str] | None = None) -> str: + """Rewrite SF: paths under ``source_root`` (or ``/proc/self/cwd/``) to canonical names. - A configuration-specific ``bazel-out//bin/`` prefix (generated - virtual-includes headers) is dropped as well. Paths outside the workspace - (external deps that survived filtering) are left unchanged. + The configuration-specific ``bazel-out//bin/`` prefix of a + generated header is dropped and a ``_virtual_includes/`` path is mapped to + its declared header. Other paths are left unchanged. """ - prefix = workspace_root if workspace_root.endswith("/") else workspace_root + "/" - sf_prefix = "SF:" + prefix + prefix = source_root if source_root.endswith("/") else source_root + "/" lines = [] for line in lcov_content.splitlines(keepends=True): - if line.startswith(sf_prefix): - lines.append("SF:" + strip_config_prefix(line[len(sf_prefix) :])) + if line.startswith("SF:"): + path = line[3:].rstrip("\n") + for known in (prefix, "/proc/self/cwd/"): + if path.startswith(known): + path = path[len(known) :] + break + lines.append("SF:" + canonical_path(path, path_map) + "\n") else: lines.append(line) return "".join(lines) @@ -281,21 +448,22 @@ def _make_lcov_paths_relative(lcov_content: str, workspace_root: str) -> str: _SOURCE_TITLE_RE = re.compile(r"(
)([^<]*)(
)") -def _make_html_paths_relative(html_dir: Path, workspace_root: str) -> None: - """Rewrite absolute workspace paths in llvm-cov HTML page titles. +def _make_html_paths_relative(html_dir: Path, source_root: str, path_map: dict[str, str] | None = None) -> None: + """Rewrite absolute source paths in llvm-cov HTML page titles to canonical names. - Only the source-name-title header text is touched — hrefs and the on-disk - page layout embed the same path components without a leading slash, and a - blanket text replacement would corrupt them. + Only the source-name-title header text is touched; the page layout is + handled by :func:`relocate_html_pages`. """ if not html_dir.exists(): return - prefix = workspace_root if workspace_root.endswith("/") else workspace_root + "/" + prefix = source_root if source_root.endswith("/") else source_root + "/" def _repl(match: "re.Match") -> str: title = match.group(2) - if title.startswith(prefix): - title = strip_config_prefix(title[len(prefix) :]) + for known in (prefix, "/proc/self/cwd/"): + if title.startswith(known): + title = canonical_path(title[len(known) :], path_map) + break return match.group(1) + title + match.group(3) for page in html_dir.rglob("*.html"): @@ -337,15 +505,16 @@ def get_covered_files( objects: list[str], instr_profile: str | None, workspace_root: str, + path_map: dict[str, str] | None = None, ) -> dict[str, str]: """Run a quick llvm-cov report to discover all files with coverage data. Returns a dict mapping each raw file path as llvm-cov displays it (after - stripping the workspace root or ``/proc/self/cwd/``) to its normalized, - configuration-agnostic form. The raw form is what ``--ignore-filename-regex`` - must match to suppress one specific compiled variant of a generated file; - the normalized form is what the allowlist and the test/baseline set - arithmetic compare against. + stripping the workspace root or ``/proc/self/cwd/``) to its canonical name + (see :func:`canonical_path`). The raw form is what + ``--ignore-filename-regex`` must match to suppress one specific compiled + variant of a file; the canonical form is what the allowlist and the + test/baseline set arithmetic compare against. """ cmd = [ str(llvm_bin_path), @@ -382,15 +551,87 @@ def get_covered_files( # raw covmap path: for C++ that is the recorded compilation dir # /proc/self/cwd/, --path-equivalence does not rewrite the # DISPLAYED path. Rust covmap paths are already exec-root relative. - for prefix in (workspace_root, "/proc/self/cwd/"): + for prefix in (workspace_root.rstrip("/") + "/", "/proc/self/cwd/"): if filename.startswith(prefix): filename = filename[len(prefix) :] break - files[filename] = strip_config_prefix(filename) + files[filename] = canonical_path(filename, path_map) return files +def prepare_sources( + r: RunfilesLike, + args: argparse.Namespace, + llvm_bin_path: Path, + sorted_objects: list[str], + merged_profdata: str, + baseline_objects: list[str], +) -> tuple[FileSelection, dict[str, str], str, list[str]]: + """Apply the scope, stage the in-scope sources and build the exclusion filters. + + Returns the file selection, the path map, the staging root llvm-cov reads + from, and the ``--ignore-filename-regex`` values. + """ + workspace_root = args.workspace_root + path_map = load_path_map(r, args.path_map) + if path_map: + print( + f"INFO: {len(path_map)} headers behind include prefixes are reported under their declared path.", + file=sys.stderr, + ) + + allowlist_set: set[str] | None = None + if args.coverage_allowlist: + allowlist_files = load_coverage_allowlist(r, args.coverage_allowlist) + if not allowlist_files: + print("ERROR: Coverage allowlist is empty.", file=sys.stderr) + sys.exit(-1) + print(f"INFO: Using coverage allowlist with {len(allowlist_files)} source files.", file=sys.stderr) + allowlist_set = set(allowlist_files) + + # Files with coverage data: raw covmap path -> canonical name. Test + # binaries and baseline archives are inspected in SEPARATE llvm-cov runs; + # combining them in one invocation makes some files vanish (suspected + # llvm-cov deduplication issue). + test_covered = get_covered_files(llvm_bin_path, sorted_objects, merged_profdata, workspace_root, path_map) + print(f"INFO: Test binaries cover {len(set(test_covered.values()))} files.", file=sys.stderr) + baseline_covered: dict[str, str] = {} + if baseline_objects: + baseline_covered = get_covered_files(llvm_bin_path, baseline_objects, None, workspace_root, path_map) + print(f"INFO: Baseline archives contain {len(set(baseline_covered.values()))} files.", file=sys.stderr) + + selection = select_files(test_covered, baseline_covered, allowlist_set) + for name, dropped in sorted(selection.duplicates.items()): + print( + f"WARNING: {name} is compiled under several paths; reporting one, dropping {sorted(dropped)}", + file=sys.stderr, + ) + if selection.baseline_only: + print( + f"INFO: {len(selection.baseline_only)} allowlisted files only in baseline " + f"(e.g., {sorted(selection.baseline_only)[:5]})", + file=sys.stderr, + ) + # Stage the in-scope sources under the raw covmap layout so llvm-cov can + # read every one of them: generated headers (_virtual_includes/) and + # vendored external headers do not exist below the workspace directory at + # report time, and the workspace files themselves may not either (fresh + # CI checkout, remote execution). + source_root = Path.cwd() / "sources" + missing = stage_sources(source_root, selection.staged, r, workspace_root) + if missing: + print( + f"WARNING: {len(missing)} in-scope sources were not found; their HTML pages " + f"will be missing (e.g., {missing[:5]})", + file=sys.stderr, + ) + root = str(source_root) + regexes = [exclusion_regex(raw, [workspace_root, root]) for raw in sorted(selection.excluded)] + print(f"INFO: Excluding {len(regexes)} compiled files outside the scope.", file=sys.stderr) + return selection, path_map, root, regexes + + def run_llvm_cov_show( llvm_bin_path: Path, objects: list[str], @@ -674,6 +915,25 @@ def load_coverage_allowlist(runfiles: RunfilesLike, rlocation_path: str) -> list return [line.strip() for line in lines if line.strip() and not line.strip().startswith("#")] +def load_path_map(runfiles: RunfilesLike, rlocation_path: str | None) -> dict[str, str]: + """Load the scope's ``\\t`` map; empty when absent.""" + if not rlocation_path: + return {} + path = runfiles.Rlocation(rlocation_path) + if not path or not Path(path).exists(): + print(f"WARNING: Path map not found: {rlocation_path}", file=sys.stderr) + return {} + mapping: dict[str, str] = {} + for line in Path(path).read_text(encoding="utf-8").splitlines(): + line = line.strip() + if not line or line.startswith("#"): + continue + virtual, _, canonical = line.partition("\t") + if virtual and canonical: + mapping[virtual] = canonical + return mapping + + def load_baseline_objects( runfiles: RunfilesLike, rlocation_path: str | None, @@ -773,6 +1033,12 @@ def parse_args(argv: list[str] | None = None) -> argparse.Namespace: default=None, help="Rlocation path to the coverage allowlist file (preferred over filter_regexes)", ) + parser.add_argument( + "--path_map", + type=str, + default=None, + help="Rlocation path to the scope's virtual-includes -> declared header map", + ) parser.add_argument( "--baseline_objects", type=str, diff --git a/score_coverage/reporter_wrapper.bzl b/score_coverage/reporter_wrapper.bzl index 204bf2a..727f2e6 100644 --- a/score_coverage/reporter_wrapper.bzl +++ b/score_coverage/reporter_wrapper.bzl @@ -37,15 +37,20 @@ def _reporter_wrapper_impl(ctx): module_bazel = ctx.file.module_bazel coverage_scope = ctx.attr.coverage_scope allowlist_group = coverage_scope[OutputGroupInfo].allowlist.to_list() + path_map_group = coverage_scope[OutputGroupInfo].path_map.to_list() objects_group = coverage_scope[OutputGroupInfo].objects.to_list() object_files = coverage_scope[OutputGroupInfo].object_files + source_files = coverage_scope[OutputGroupInfo].source_files if len(allowlist_group) != 1: fail("coverage_scope must provide exactly one allowlist file") + if len(path_map_group) != 1: + fail("coverage_scope must provide exactly one path map file") if len(objects_group) != 1: fail("coverage_scope must provide exactly one objects manifest file") allowlist = allowlist_group[0] + path_map = path_map_group[0] baseline_objects = objects_group[0] cxxfilt_line = "" @@ -71,6 +76,7 @@ export RUNFILES_DIR WORKSPACE_ROOT="$(cd "$(dirname "$(readlink -f "${{RUNFILES_DIR}}/{module_bazel}")")" && pwd)/" exec "${{RUNFILES_DIR}}/{reporter}" \\ --coverage_allowlist="{allowlist}" \\ + --path_map="{path_map}" \\ --baseline_objects="{baseline_objects}" \\ --workspace_root="${{WORKSPACE_ROOT}}" \\ --llvm_cov="{llvm_cov}" \\ @@ -80,6 +86,7 @@ exec "${{RUNFILES_DIR}}/{reporter}" \\ module_bazel = _rlocation_path(ctx, module_bazel), reporter = _rlocation_path(ctx, reporter), allowlist = _rlocation_path(ctx, allowlist), + path_map = _rlocation_path(ctx, path_map), baseline_objects = _rlocation_path(ctx, baseline_objects), llvm_cov = _rlocation_path(ctx, ctx.file.llvm_cov), llvm_profdata = _rlocation_path(ctx, ctx.file.llvm_profdata), @@ -95,6 +102,7 @@ exec "${{RUNFILES_DIR}}/{reporter}" \\ direct_files = [ reporter, allowlist, + path_map, baseline_objects, module_bazel, ctx.file.llvm_cov, @@ -103,9 +111,12 @@ exec "${{RUNFILES_DIR}}/{reporter}" \\ if ctx.file.llvm_cxxfilt: direct_files.append(ctx.file.llvm_cxxfilt) + # The in-scope source files travel with the reporter: llvm-cov must read + # them at report time, and neither generated headers nor external + # repositories are reachable through the workspace directory then. runfiles = ctx.runfiles( files = direct_files, - transitive_files = object_files, + transitive_files = depset(transitive = [object_files, source_files]), ).merge(ctx.attr.reporter[DefaultInfo].default_runfiles) for tool in (ctx.attr.llvm_cov, ctx.attr.llvm_profdata, ctx.attr.llvm_cxxfilt): if tool: diff --git a/score_coverage/tests/reporter_test.py b/score_coverage/tests/reporter_test.py index c3ff025..8338d74 100644 --- a/score_coverage/tests/reporter_test.py +++ b/score_coverage/tests/reporter_test.py @@ -21,6 +21,7 @@ import io import json import os +import re import stat import sys import tempfile @@ -139,6 +140,16 @@ def test_workspace_root_without_trailing_slash(self): result = _make_lcov_paths_relative(lcov, "/ws/root") self.assertEqual(result, "SF:src/foo.cpp\nend_of_record\n") + def test_proc_self_cwd_prefix_and_path_map(self): + lcov = ( + "SF:/proc/self/cwd/src/a.cpp\nend_of_record\n" + "SF:/ws/bazel-out/k8-fastbuild/bin/src/_virtual_includes/v/api.h\nend_of_record\n" + ) + self.assertEqual( + reporter._make_lcov_paths_relative(lcov, "/ws", {"src/_virtual_includes/v/api.h": "src/v/api.h"}), + "SF:src/a.cpp\nend_of_record\nSF:src/v/api.h\nend_of_record\n", + ) + def test_external_paths_are_unchanged(self): lcov = "SF:/other/place/dep.cpp\nend_of_record\n" self.assertEqual(_make_lcov_paths_relative(lcov, "/ws/root/"), lcov) @@ -228,18 +239,31 @@ def _fake_llvm_cov(path: Path) -> Path: sys.stdout.write({REPORT_TABLE!r}) elif sub == "export": empty = "--empty-profile" in args + root = [a for a in args if a.startswith("--path-equivalence=")][0].split(",", 1)[1].rstrip("/") sys.stderr.write("warning: something cosmetic\\n") if empty: - sys.stdout.write("SF:/ws/src/b.cpp\\nDA:1,0\\nLF:1\\nLH:0\\nend_of_record\\n") + sys.stdout.write("SF:" + root + "/src/b.cpp\\nDA:1,0\\nLF:1\\nLH:0\\nend_of_record\\n") else: - sys.stdout.write("SF:/ws/src/a.cpp\\nDA:1,1\\nLF:1\\nLH:1\\nend_of_record\\n") + sys.stdout.write("SF:" + root + "/src/a.cpp\\nDA:1,1\\nLF:1\\nLH:1\\nend_of_record\\n") elif sub == "show": out = [a for a in args if a.startswith("--output-dir=")][0].split("=", 1)[1] - os.makedirs(os.path.join(out, "coverage", "ws", "src"), exist_ok=True) - open(os.path.join(out, "index.html"), "w").write("index") + root = [a for a in args if a.startswith("--path-equivalence=")][0].split(",", 1)[1].strip("/") + os.makedirs(out, exist_ok=True) open(os.path.join(out, "style.css"), "w").write("body {{}}") - open(os.path.join(out, "coverage", "ws", "src", "a.cpp.html"), "w").write( - "
/ws/src/a.cpp
") + open(os.path.join(out, "control.js"), "w").write("") + rows = [] + for rel in ("src/a.cpp", "bazel-out/k8-fastbuild/bin/src/_virtual_includes/v/api.h"): + page = os.path.join(out, "coverage", root, rel + ".html") + os.makedirs(os.path.dirname(page), exist_ok=True) + up = "../" * len(os.path.relpath(os.path.dirname(page), out).split(os.sep)) + open(page, "w").write( + "" + "" + "
/" + root + "/" + rel + "
") + rows.append("
" + rel + "
") + open(os.path.join(out, "index.html"), "w").write( + "" + "" + "".join(rows) + "
") else: sys.exit(2) """ @@ -450,6 +474,14 @@ def test_get_covered_files_normalises_paths(self): self.assertEqual(argv[:3], ["report", "--path-equivalence=/proc/self/cwd/,/ws/", "--empty-profile"]) self.assertEqual(argv[3:], ["/o/a.a", "--object", "/o/b.a"]) + def test_get_covered_files_applies_the_path_map(self): + with redirect_stderr(io.StringIO()): + files = reporter.get_covered_files( + self.cov, ["/o/a.a"], None, "/ws/", {"src/_virtual_includes/v/api.h": "src/v/api.h"} + ) + self.assertEqual(files["bazel-out/k8-fastbuild/bin/src/_virtual_includes/v/api.h"], "src/v/api.h") + self.assertEqual(files["src/a.cpp"], "src/a.cpp") + def test_show_html_flags(self): out = self.root / "html" with redirect_stderr(io.StringIO()): @@ -564,9 +596,16 @@ def test_full_report(self): names = set(zf.namelist()) lcov = zf.read("lcov_report/lcov.dat").decode() summary = zf.read("text_report/summary.txt").decode() - page = zf.read("html_report/coverage/ws/src/a.cpp.html").decode() + page = zf.read("html_report/coverage/src/a.cpp.html").decode() + index = zf.read("html_report/index.html").decode() self.assertIn("html_report/index.html", names) self.assertIn("html_report/style.css", names) + # Pages are filed under canonical paths: no machine-specific directory + # remains, the index links there, and the page's asset links still resolve. + self.assertNotIn("html_report/coverage/" + str(self.workdir).strip("/") + "/", "\n".join(names)) + self.assertIn("src/a.cpp", index) + self.assertIn("href='../../style.css'", page) + self.assertIn("src='../../control.js'", page) # SF paths are made workspace-relative, llvm-cov's stderr warning is not in the data. self.assertIn("SF:src/a.cpp\n", lcov) self.assertNotIn("warning", lcov) @@ -577,9 +616,46 @@ def test_full_report(self): self.assertEqual((self.workdir / "merged_coverage.profdata").read_bytes(), b"P0P1") # rust/lib.rs is not in the allowlist and must be excluded from the report. log = self.log.read_text(encoding="utf-8") - self.assertIn("--ignore-filename-regex=rust/lib\\.rs$", log) + self.assertIn(f"--ignore-filename-regex=^(/proc/self/cwd/|/ws/|{self.workdir}/sources/)?rust/lib\\.rs$", log) + # llvm-cov reads the sources through the staging directory, not the workspace. + self.assertIn(f"--path-equivalence=/proc/self/cwd/,{self.workdir}/sources", log) + self.assertIn(f"--compilation-dir={self.workdir}/sources", log) self.assertIn("Using coverage allowlist with 2 source files", err.getvalue()) + def test_path_map_files_headers_under_declared_path(self): + # The scope maps the generated virtual-includes path to the declared + # header; the page, the index link and the LCOV use the declared path. + self.allowlist.write_text("src/a.cpp\nsrc/v/api.h\n", encoding="utf-8") + path_map = self.root / "map.txt" + path_map.write_text("src/_virtual_includes/v/api.h\tsrc/v/api.h\n", encoding="utf-8") + ws = self.root / "ws" + (ws / "src" / "v").mkdir(parents=True) + (ws / "src" / "a.cpp").write_text("int a;\n", encoding="utf-8") + (ws / "src" / "v" / "api.h").write_text("int api();\n", encoding="utf-8") + argv = self._argv(path_map=str(path_map)) + argv[argv.index("--workspace_root") + 1] = str(ws) + err = io.StringIO() + with redirect_stderr(err): + reporter.main(argv) + with zipfile.ZipFile(self.output) as zf: + names = set(zf.namelist()) + index = zf.read("html_report/index.html").decode() + page = zf.read("html_report/coverage/src/v/api.h.html").decode() + self.assertIn("html_report/coverage/src/v/api.h.html", names) + self.assertNotIn("html_report/coverage/src/_virtual_includes/v/api.h.html", names) + self.assertIn("src/v/api.h", index) + self.assertIn("
src/v/api.h
", page) + self.assertIn("href='../../../style.css'", page) + # The header is in scope: no exclusion regex names it, and its source + # was staged under the raw covmap path for llvm-cov to read. + log = self.log.read_text(encoding="utf-8") + self.assertNotIn("api", "\n".join(a for a in log.split() if a.startswith("--ignore-filename-regex"))) + staged = self.workdir / "sources" / "bazel-out/k8-fastbuild/bin/src/_virtual_includes/v/api.h" + self.assertTrue(staged.is_symlink()) + self.assertEqual(os.path.realpath(staged), os.path.realpath(ws / "src" / "v" / "api.h")) + self.assertTrue((self.workdir / "sources" / "src" / "a.cpp").is_symlink()) + self.assertIn("1 headers behind include prefixes", err.getvalue()) + def test_no_reports_writes_empty_zip(self): self.reports_file.write_text("", encoding="utf-8") with redirect_stderr(io.StringIO()): @@ -606,6 +682,301 @@ def test_empty_allowlist_is_an_error(self): reporter.main(self._argv()) +@verifies("tool_req__coverage_scope_transitive", "tool_req__coverage_report_relative_paths") +class CanonicalPathTest(unittest.TestCase): + """The reported name of a file: config prefix dropped, virtual path mapped to the declared header.""" + + MAP = {"src/_virtual_includes/v/api.h": "src/v/include/api.h"} + + def test_plain_paths_are_unchanged(self): + self.assertEqual(reporter.canonical_path("src/a.cpp", self.MAP), "src/a.cpp") + self.assertEqual(reporter.canonical_path("external/ext+/x.h", None), "external/ext+/x.h") + + def test_virtual_path_maps_to_declared_header_under_any_config(self): + for cfg in ("k8-fastbuild", "k8-opt-exec-ST-1234"): + self.assertEqual( + reporter.canonical_path(f"bazel-out/{cfg}/bin/src/_virtual_includes/v/api.h", self.MAP), + "src/v/include/api.h", + ) + + def test_unmapped_virtual_path_keeps_its_config_free_form(self): + self.assertEqual( + reporter.canonical_path("bazel-out/k8-fastbuild/bin/src/_virtual_includes/u/y.h", self.MAP), + "src/_virtual_includes/u/y.h", + ) + + +@verifies("tool_req__coverage_report_allowlist") +class ExclusionRegexTest(unittest.TestCase): + """--ignore-filename-regex must hit exactly one compiled file.""" + + def _matches(self, regex, filename): + # llvm-cov uses POSIX ERE; the pattern must not need Python-only syntax. + self.assertNotIn("(?", regex) + return re.search(regex, filename) is not None + + def test_matches_the_raw_path_under_every_known_prefix(self): + regex = reporter.exclusion_regex("foo/bar.h", ["/ws/", "/work/sources"]) + for filename in ("/proc/self/cwd/foo/bar.h", "/ws/foo/bar.h", "/work/sources/foo/bar.h", "foo/bar.h"): + self.assertTrue(self._matches(regex, filename), filename) + + def test_does_not_hit_a_longer_in_scope_path_with_the_same_suffix(self): + regex = reporter.exclusion_regex("foo/bar.h", ["/ws"]) + for filename in ("/proc/self/cwd/src/foo/bar.h", "src/foo/bar.h", "/ws/src/foo/bar.h", "foo/bar.hpp"): + self.assertFalse(self._matches(regex, filename), filename) + + def test_special_characters_are_literal(self): + regex = reporter.exclusion_regex("external/openssl+/crypto/x.h", ["/ws"]) + self.assertTrue(self._matches(regex, "/proc/self/cwd/external/openssl+/crypto/x.h")) + self.assertFalse(self._matches(regex, "/proc/self/cwd/external/opensslX/crypto/x.h")) + + +@verifies("tool_req__coverage_report_allowlist", "tool_req__coverage_report_baseline_zero") +class SelectFilesTest(unittest.TestCase): + """Which raw compiled files stay, which are suppressed, which come from the baseline only.""" + + def test_out_of_scope_and_redundant_variants_are_excluded(self): + test = { + "src/a.cpp": "src/a.cpp", + "bazel-out/k8-fastbuild/bin/src/_virtual_includes/v/api.h": "src/v/api.h", + "external/openssl+/x.h": "external/openssl+/x.h", + } + baseline = { + "src/a.cpp": "src/a.cpp", + "src/untested.cpp": "src/untested.cpp", + "bazel-out/k8-opt-exec-ST-1/bin/src/_virtual_includes/v/api.h": "src/v/api.h", + "external/openssl+/y.h": "external/openssl+/y.h", + } + sel = reporter.select_files(test, baseline, {"src/a.cpp", "src/v/api.h", "src/untested.cpp"}) + self.assertEqual( + sel.excluded, + { + "external/openssl+/x.h", + "external/openssl+/y.h", + "bazel-out/k8-opt-exec-ST-1/bin/src/_virtual_includes/v/api.h", + }, + ) + self.assertEqual( + sel.staged, + { + "src/a.cpp": "src/a.cpp", + "src/untested.cpp": "src/untested.cpp", + "bazel-out/k8-fastbuild/bin/src/_virtual_includes/v/api.h": "src/v/api.h", + }, + ) + self.assertEqual(sel.baseline_only, {"src/untested.cpp"}) + self.assertEqual(sel.duplicates, {}) + + def test_duplicate_test_variants_keep_the_declared_path(self): + test = { + "src/v/api.h": "src/v/api.h", + "bazel-out/k8-fastbuild/bin/src/_virtual_includes/v/api.h": "src/v/api.h", + "bazel-out/k8-fastbuild/bin/src/_virtual_includes/w/w.h": "src/w/w.h", + "bazel-out/k8-fastbuild-ST-2/bin/src/_virtual_includes/w/w.h": "src/w/w.h", + } + self.assertEqual( + reporter.duplicate_test_variants(test), + { + "src/v/api.h": ["bazel-out/k8-fastbuild/bin/src/_virtual_includes/v/api.h"], + # no declared-path variant: the first in sort order is kept ('-' < '/') + "src/w/w.h": ["bazel-out/k8-fastbuild/bin/src/_virtual_includes/w/w.h"], + }, + ) + sel = reporter.select_files(test, {}, None) + self.assertEqual( + set(sel.staged), {"src/v/api.h", "bazel-out/k8-fastbuild-ST-2/bin/src/_virtual_includes/w/w.h"} + ) + self.assertEqual(len(sel.excluded), 2) + + def test_no_allowlist_keeps_everything(self): + sel = reporter.select_files({"a": "a"}, {"b": "b"}, None) + self.assertEqual(sel.staged, {"a": "a", "b": "b"}) + self.assertEqual(sel.excluded, set()) + self.assertEqual(sel.baseline_only, {"b"}) + + +@verifies("tool_req__coverage_report_relative_paths", "tool_req__coverage_report_outputs") +class StageSourcesTest(unittest.TestCase): + """In-scope sources are linked under their raw covmap path for llvm-cov to read.""" + + def setUp(self): + self.tmp = tempfile.TemporaryDirectory() + self.root = Path(self.tmp.name) + self.runfiles_dir = self.root / "runfiles" + (self.runfiles_dir / "_main" / "src").mkdir(parents=True) + (self.runfiles_dir / "ext+" / "inc").mkdir(parents=True) + (self.runfiles_dir / "_main" / "src" / "a.cpp").write_text("a", encoding="utf-8") + (self.runfiles_dir / "ext+" / "inc" / "x.h").write_text("x", encoding="utf-8") + self.ws = self.root / "ws" + (self.ws / "rust").mkdir(parents=True) + (self.ws / "rust" / "lib.rs").write_text("r", encoding="utf-8") + self.runfiles = _FakeRunfiles( + { + "_main/src/a.cpp": str(self.runfiles_dir / "_main" / "src" / "a.cpp"), + "ext+/inc/x.h": str(self.runfiles_dir / "ext+" / "inc" / "x.h"), + "_main/rust/lib.rs": str(self.runfiles_dir / "_main" / "rust" / "lib.rs"), # not present + } + ) + + def tearDown(self): + self.tmp.cleanup() + + def test_resolution_order_runfiles_then_workspace(self): + self.assertEqual( + reporter.resolve_source(self.runfiles, "src/a.cpp", str(self.ws)), + str(self.runfiles_dir / "_main" / "src" / "a.cpp"), + ) + self.assertEqual( + reporter.resolve_source(self.runfiles, "external/ext+/inc/x.h", str(self.ws)), + str(self.runfiles_dir / "ext+" / "inc" / "x.h"), + ) + self.assertEqual( + reporter.resolve_source(self.runfiles, "rust/lib.rs", str(self.ws)), str(self.ws / "rust/lib.rs") + ) + self.assertIsNone(reporter.resolve_source(self.runfiles, "src/missing.cpp", str(self.ws))) + + def test_links_follow_the_raw_layout_and_missing_files_are_reported(self): + stage = self.root / "sources" + staged = { + "src/a.cpp": "src/a.cpp", + "bazel-out/k8-fastbuild/bin/src/_virtual_includes/v/x.h": "external/ext+/inc/x.h", + "rust/lib.rs": "rust/lib.rs", + "src/missing.cpp": "src/missing.cpp", + "/usr/include/abs.h": "/usr/include/abs.h", + } + missing = reporter.stage_sources(stage, staged, self.runfiles, str(self.ws)) + self.assertEqual(missing, ["src/missing.cpp"]) + self.assertEqual((stage / "src" / "a.cpp").read_text(encoding="utf-8"), "a") + virtual = stage / "bazel-out/k8-fastbuild/bin/src/_virtual_includes/v/x.h" + self.assertTrue(virtual.is_symlink()) + self.assertEqual(virtual.read_text(encoding="utf-8"), "x") + self.assertEqual((stage / "rust" / "lib.rs").read_text(encoding="utf-8"), "r") + self.assertFalse((stage / "usr").exists()) + # idempotent + self.assertEqual(reporter.stage_sources(stage, staged, self.runfiles, str(self.ws)), ["src/missing.cpp"]) + + +@verifies("tool_req__coverage_report_relative_paths", "tool_req__coverage_report_outputs") +class RelocateHtmlPagesTest(unittest.TestCase): + """llvm-cov pages move from coverage//.html to coverage/.html.""" + + ROOT = "/tmp/work/sources" + + def _page(self, html_dir, rel, depth): + page = html_dir / "coverage" / self.ROOT.strip("/") / (rel + ".html") + page.parent.mkdir(parents=True, exist_ok=True) + up = "../" * depth + page.write_text( + f"" + f"
{self.ROOT}/{rel}
", + encoding="utf-8", + ) + return page + + def test_pages_index_and_asset_links(self): + with tempfile.TemporaryDirectory() as tmp: + html_dir = Path(tmp) + (html_dir / "style.css").write_text("", encoding="utf-8") + (html_dir / "control.js").write_text("", encoding="utf-8") + raw_v = "bazel-out/k8-fastbuild/bin/src/_virtual_includes/v/api.h" + self._page(html_dir, "src/a.cpp", 6) + self._page(html_dir, raw_v, 10) + self._page(html_dir, "external/ext+/inc/x.h", 8) + (html_dir / "index.html").write_text( + "" + f"" + f"" + f"" + "" + "
src/a.cpp
{raw_v}
external/ext+/inc/x.h"
+                "
src/missing.cpp
", + encoding="utf-8", + ) + moves = reporter.relocate_html_pages(html_dir, self.ROOT, {"src/_virtual_includes/v/api.h": "src/v/api.h"}) + reporter._make_html_paths_relative(html_dir, self.ROOT, {"src/_virtual_includes/v/api.h": "src/v/api.h"}) + + self.assertEqual( + moves, + { + "coverage/tmp/work/sources/src/a.cpp.html": "coverage/src/a.cpp.html", + f"coverage/tmp/work/sources/{raw_v}.html": "coverage/src/v/api.h.html", + "coverage/tmp/work/sources/external/ext+/inc/x.h.html": "coverage/external/ext+/inc/x.h.html", + }, + ) + self.assertFalse((html_dir / "coverage" / "tmp").exists()) + a = (html_dir / "coverage/src/a.cpp.html").read_text(encoding="utf-8") + self.assertIn("href='../../style.css'", a) + self.assertIn("src='../../control.js'", a) + self.assertIn("
src/a.cpp
", a) + v = (html_dir / "coverage/src/v/api.h.html").read_text(encoding="utf-8") + self.assertIn("href='../../../style.css'", v) + self.assertIn("
src/v/api.h
", v) + x = (html_dir / "coverage/external/ext+/inc/x.h.html").read_text(encoding="utf-8") + self.assertIn("href='../../../../style.css'", x) + index = (html_dir / "index.html").read_text(encoding="utf-8") + self.assertIn("src/a.cpp", index) + self.assertIn("src/v/api.h", index) + self.assertIn("external/ext+/inc/x.h", index) + self.assertNotIn("tmp/work/sources", index) + self.assertIn("
src/missing.cpp
", index) + # every link resolves + for href in re.findall(r"href='(coverage/[^']+)'", index): + self.assertTrue((html_dir / href).is_file(), href) + + def test_second_page_for_the_same_file_is_dropped_with_a_warning(self): + with tempfile.TemporaryDirectory() as tmp: + html_dir = Path(tmp) + self._page(html_dir, "src/v/api.h", 7) + self._page(html_dir, "bazel-out/k8-fastbuild/bin/src/_virtual_includes/v/api.h", 10) + (html_dir / "index.html").write_text("", encoding="utf-8") + err = io.StringIO() + with redirect_stderr(err): + reporter.relocate_html_pages(html_dir, self.ROOT, {"src/_virtual_includes/v/api.h": "src/v/api.h"}) + self.assertEqual(sorted(p.name for p in (html_dir / "coverage").rglob("*.html")), ["api.h.html"]) + self.assertIn("rendered twice", err.getvalue()) + + def test_missing_coverage_dir_is_a_noop(self): + with tempfile.TemporaryDirectory() as tmp: + self.assertEqual(reporter.relocate_html_pages(Path(tmp), self.ROOT, {}), {}) + + +@verifies("tool_req__coverage_report_relative_paths") +class PathMapAndSummaryTest(unittest.TestCase): + """Loading the scope's path map and naming files consistently in the text summary.""" + + def test_load_path_map(self): + with tempfile.TemporaryDirectory() as tmp: + f = Path(tmp) / "map.txt" + f.write_text("# comment\n\nsrc/_virtual_includes/v/api.h\tsrc/v/api.h\nbroken-line\n", encoding="utf-8") + runfiles = _FakeRunfiles({"m/map.txt": str(f)}) + self.assertEqual( + reporter.load_path_map(runfiles, "m/map.txt"), {"src/_virtual_includes/v/api.h": "src/v/api.h"} + ) + with redirect_stderr(io.StringIO()): + self.assertEqual(reporter.load_path_map(runfiles, "m/nope.txt"), {}) + self.assertEqual(reporter.load_path_map(runfiles, None), {}) + + def test_summary_names_are_canonical_and_aligned(self): + text = ( + "Filename Regions\n" + "-------------------------------------\n" + "/tmp/work/sources/src/a.cpp 4\n" + "/proc/self/cwd/bazel-out/k8-fastbuild/bin/src/_virtual_includes/v/api.h 2\n" + "rust/lib.rs 3\n" + "TOTAL 9\n" + ) + path_map = {"src/_virtual_includes/v/api.h": "src/v/api.h"} + out = reporter._canonicalize_summary(text, "/tmp/work/sources", path_map) + lines = out.splitlines() + padding = " " * (len("/tmp/work/sources/src/a.cpp") - len("src/a.cpp")) + self.assertEqual(lines[2], "src/a.cpp" + padding + " 4") + self.assertTrue(lines[3].startswith("src/v/api.h ")) + self.assertTrue(lines[3].endswith(" 2")) + self.assertEqual(lines[4], "rust/lib.rs 3") + self.assertEqual(lines[5], "TOTAL 9") + self.assertEqual(lines[0], text.splitlines()[0]) + + @verifies("tool_req__coverage_scope_transitive", "tool_req__coverage_report_relative_paths") class ConfigPrefixTest(unittest.TestCase): """Generated virtual-includes headers carry a configuration-specific bazel-out prefix.""" diff --git a/score_coverage/tests/starlark/coverage_scope_tests.bzl b/score_coverage/tests/starlark/coverage_scope_tests.bzl index 2adf03f..96970f7 100644 --- a/score_coverage/tests/starlark/coverage_scope_tests.bzl +++ b/score_coverage/tests/starlark/coverage_scope_tests.bzl @@ -13,9 +13,10 @@ """Analysis tests for the coverage_scope rule and its aspect. -The rule writes two text files; rules_testing lets us read the content of +The rule writes three text files; rules_testing lets us read the content of the FileWrite actions without executing anything, so these tests pin down -exactly which source files and archives end up in the coverage scope. +exactly which source files, path mappings and archives end up in the +coverage scope. """ load("@rules_testing//lib:analysis_test.bzl", "analysis_test", "test_suite") @@ -35,6 +36,11 @@ def _objects(env, target): _PKG + "/" + target.label.name + "_objects.txt", ).content() +def _path_map(env, target): + return env.expect.that_target(target).action_generating( + _PKG + "/" + target.label.name + "_path_map.txt", + ).content() + # --- transitive deps ------------------------------------------------------- def _test_transitive_deps_are_collected(name): @@ -125,13 +131,24 @@ def _test_virtual_include_headers_are_in_scope(name): analysis_test(name = name, impl = _test_virtual_include_headers_are_in_scope_impl, target = name + "_subject") def _test_virtual_include_headers_are_in_scope_impl(env, target): - # Both the declared header and the generated path the compiler records - # (eclipse-score/baselibs#558) are listed. - _allowlist(env, target).equals( - "\n".join([ - _PKG + "/fixtures/_virtual_includes/vendored/vendored/api.h", - _PKG + "/fixtures/vendor/include/vendored/api.h", - ]) + "\n", + # The allowlist names the declared header; the path map ties the generated + # path the compiler records (eclipse-score/baselibs#558) back to it. + _allowlist(env, target).equals(_PKG + "/fixtures/vendor/include/vendored/api.h\n") + _path_map(env, target).equals( + _PKG + "/fixtures/_virtual_includes/vendored/vendored/api.h\t" + + _PKG + "/fixtures/vendor/include/vendored/api.h\n", + ) + +def _test_include_prefix_is_mapped(name): + coverage_scope(name = name + "_subject", testonly = True, deps = [_FIX + ":prefixed"]) + analysis_test(name = name, impl = _test_include_prefix_is_mapped_impl, target = name + "_subject") + +def _test_include_prefix_is_mapped_impl(env, target): + # include_prefix adds components in the virtual tree that the declared + # header does not have; the map still resolves to the declared file. + _path_map(env, target).equals( + _PKG + "/fixtures/_virtual_includes/prefixed/pfx/vendored/api.h\t" + + _PKG + "/fixtures/vendor/include/vendored/api.h\n", ) # --- headers vendored from an external repository --------------------------- @@ -142,14 +159,13 @@ def _test_external_vendored_header_behind_strip_prefix(name): def _test_external_vendored_header_behind_strip_prefix_impl(env, target): # The declared header is an external source file ("..//..." short_path, - # listed as "external//..."), and the compiler records the generated - # virtual-includes path; both identities are in scope. External TARGETS are - # still not traversed (the fixture module has none in deps). - _allowlist(env, target).equals( - "\n".join([ - "external/coverage_external_fixture+/include/ext/ext.h", - _PKG + "/fixtures/_virtual_includes/vendored_external/ext/ext.h", - ]) + "\n", + # listed as "external//..."); the compiler records the generated + # virtual-includes path, which the map resolves to that file. External + # TARGETS are still not listed (see the forwarded fixture below). + _allowlist(env, target).equals("external/coverage_external_fixture+/include/ext/ext.h\n") + _path_map(env, target).equals( + _PKG + "/fixtures/_virtual_includes/vendored_external/ext/ext.h\t" + + "external/coverage_external_fixture+/include/ext/ext.h\n", ) def _test_external_vendored_header_plain(name): @@ -158,6 +174,19 @@ def _test_external_vendored_header_plain(name): def _test_external_vendored_header_plain_impl(env, target): _allowlist(env, target).equals("external/coverage_external_fixture+/include/ext/ext.h\n") + _path_map(env, target).equals("") + +def _test_forwarded_external_library_stays_out(name): + coverage_scope(name = name + "_subject", testonly = True, deps = [_FIX + ":uses_forwarded"]) + analysis_test(name = name, impl = _test_forwarded_external_library_stays_out_impl, target = name + "_subject") + +def _test_forwarded_external_library_stays_out_impl(env, target): + # A workspace rule that only forwards an external library's CcInfo makes + # that library's headers its direct_public_headers; they are neither + # declared by nor generated by the workspace target and stay out of the + # scope (eclipse-score/coverage_tool#5). + _allowlist(env, target).equals(_PKG + "/fixtures/user.cpp\n") + _path_map(env, target).equals("") # --- Rust: rust_library (CcInfo) and rust_binary (CrateInfo only) ----------- @@ -195,10 +224,20 @@ def _test_output_groups(name): def _test_output_groups_impl(env, target): subject = env.expect.that_target(target) subject.output_group("allowlist").contains_exactly([_PKG + "/" + target.label.name + "_allowlist.txt"]) + subject.output_group("path_map").contains_exactly([_PKG + "/" + target.label.name + "_path_map.txt"]) subject.output_group("objects").contains_exactly([_PKG + "/" + target.label.name + "_objects.txt"]) subject.output_group("object_files").contains_predicate(matching.file_basename_equals("libleaf.a")) + + # The sources themselves travel to the reporter (staged for llvm-cov). + subject.output_group("source_files").contains_exactly([ + _PKG + "/fixtures/leaf.cpp", + _PKG + "/fixtures/leaf.h", + _PKG + "/fixtures/mid.cpp", + _PKG + "/fixtures/mid.h", + ]) subject.default_outputs().contains_at_least([ _PKG + "/" + target.label.name + "_allowlist.txt", + _PKG + "/" + target.label.name + "_path_map.txt", _PKG + "/" + target.label.name + "_objects.txt", ]) @@ -212,8 +251,10 @@ def coverage_scope_test_suite(name): _test_header_only_library_has_no_archive, _test_generated_sources_are_excluded, _test_virtual_include_headers_are_in_scope, + _test_include_prefix_is_mapped, _test_external_vendored_header_behind_strip_prefix, _test_external_vendored_header_plain, + _test_forwarded_external_library_stays_out, _test_rust_library_sources_and_archive, _test_rust_binary_collects_crate_sources_and_executable, _test_output_groups, diff --git a/score_coverage/tests/starlark/external_fixture/BUILD b/score_coverage/tests/starlark/external_fixture/BUILD index 8fe436f..d8d34a8 100644 --- a/score_coverage/tests/starlark/external_fixture/BUILD +++ b/score_coverage/tests/starlark/external_fixture/BUILD @@ -11,4 +11,14 @@ # SPDX-License-Identifier: Apache-2.0 # ******************************************************************************* +load("@rules_cc//cc:cc_library.bzl", "cc_library") + exports_files(["include/ext/ext.h"]) + +# Stand-in for a third-party library a workspace rule forwards. +cc_library( + name = "extlib", + hdrs = ["include/ext/ext.h"], + strip_include_prefix = "include", + visibility = ["//visibility:public"], +) diff --git a/score_coverage/tests/starlark/external_fixture/MODULE.bazel b/score_coverage/tests/starlark/external_fixture/MODULE.bazel index 0ad478b..e873167 100644 --- a/score_coverage/tests/starlark/external_fixture/MODULE.bazel +++ b/score_coverage/tests/starlark/external_fixture/MODULE.bazel @@ -16,3 +16,5 @@ # local_path_override in the root MODULE.bazel; excluded from //... via # .bazelignore. module(name = "coverage_external_fixture") + +bazel_dep(name = "rules_cc", version = "0.2.16") diff --git a/score_coverage/tests/starlark/fixtures/BUILD b/score_coverage/tests/starlark/fixtures/BUILD index aff50de..59e44ed 100644 --- a/score_coverage/tests/starlark/fixtures/BUILD +++ b/score_coverage/tests/starlark/fixtures/BUILD @@ -13,6 +13,7 @@ load("@rules_cc//cc:cc_library.bzl", "cc_library") load("@rules_rust//rust:defs.bzl", "rust_binary", "rust_library") +load(":forward.bzl", "forward_cc") # Fixtures for the coverage_scope analysis tests. They are only analyzed, # never built: `manual` keeps them out of wildcard builds. @@ -103,3 +104,25 @@ cc_library( hdrs = ["@coverage_external_fixture//:include/ext/ext.h"], tags = MANUAL, ) + +cc_library( + name = "prefixed", + hdrs = ["vendor/include/vendored/api.h"], + include_prefix = "pfx", + strip_include_prefix = "vendor/include", + tags = MANUAL, +) + +# A workspace target that only forwards an external library (its CcInfo). +forward_cc( + name = "forwarded_external", + dep = "@coverage_external_fixture//:extlib", + tags = MANUAL, +) + +cc_library( + name = "uses_forwarded", + srcs = ["user.cpp"], + tags = MANUAL, + deps = [":forwarded_external"], +) diff --git a/score_coverage/tests/starlark/fixtures/forward.bzl b/score_coverage/tests/starlark/fixtures/forward.bzl new file mode 100644 index 0000000..415c986 --- /dev/null +++ b/score_coverage/tests/starlark/fixtures/forward.bzl @@ -0,0 +1,30 @@ +# ******************************************************************************* +# Copyright (c) 2026 Contributors to the Eclipse Foundation +# +# See the NOTICE file(s) distributed with this work for additional +# information regarding copyright ownership. +# +# This program and the accompanying materials are made available under the +# terms of the Apache License Version 2.0 which is available at +# https://www.apache.org/licenses/LICENSE-2.0 +# +# SPDX-License-Identifier: Apache-2.0 +# ******************************************************************************* +"""A workspace rule that forwards the CcInfo of another target (fixture). + +Consumers wrap third-party libraries this way, e.g. to apply a transition +(eclipse-score/baselibs ``third_party/openssl``). The forwarded library's +headers are then the workspace target's ``direct_public_headers`` although +the workspace target declares nothing itself (eclipse-score/coverage_tool#5). +""" + +def _forward_cc_impl(ctx): + dep = ctx.attr.dep + return [dep[DefaultInfo], dep[CcInfo]] + +forward_cc = rule( + implementation = _forward_cc_impl, + attrs = { + "dep": attr.label(providers = [CcInfo], mandatory = True), + }, +) From 961b00977cefc6a8ff360fa5fca434e37a3cc8ec Mon Sep 17 00:00:00 2001 From: Dan Calavrezo <195309321+dcalavrezo-qorix@users.noreply.github.com> Date: Tue, 15 Sep 2026 17:04:23 +0300 Subject: [PATCH 2/6] Report in-scope files that carry no coverage data A file in the scope allowlist for which no test binary and no baseline archive carries a coverage mapping cannot be rendered by llvm-cov, not even at 0 %: a header no translation unit includes, or template code that is never instantiated. Until now such a file was silently absent from every report format, which contradicts the promise that untested in-scope files show up. baselibs' flatbuffers/buffer_ref.h, a vendored public API header nothing in baselibs uses, went unnoticed that way. - reporter: the selection computes the allowlisted files without any data. Headers whose same-named source file has data (foo.h next to a compiled foo.cpp) hold declarations only and are kept apart, so the actionable list is not buried under them. Both lists are written to text_report/unmapped_files.txt and text_report/declaration_only_headers.txt (always present) and the reporter warns about the first. - generate_coverage_html: prints the unmapped files, copies both lists into the archive, and hands them to the summary. - coverage_summary: a table row "In-scope files without coverage data" and two collapsible sections; older reports without the lists render as before. Tests: selection, reporter main, summary rendering and loading, generator wiring and archive layout; the integration workspace gains a template-only header nobody includes (src/unused_api.h) and checks the lists, the summary and that no LCOV record is invented for it. Docs: new tool_req__coverage_report_unmapped, known problems, release notes 0.2.0, user manual, verification report inventory. Follow-up of #5 (discussion of vendored third-party headers). Signed-off-by: Dan Calavrezo <195309321+dcalavrezo-qorix@users.noreply.github.com> --- docs/manual/known_problems.rst | 10 +++ docs/manual/user_manual.rst | 5 +- docs/release/release_notes.rst | 7 ++ docs/requirements/tool_requirements.rst | 21 ++++++ docs/verification/verification_report.rst | 18 ++--- integration_tests/expected_lcov.dat | 4 ++ integration_tests/run_integration_test.sh | 23 ++++++- integration_tests/src/BUILD | 7 ++ integration_tests/src/unused_api.h | 30 ++++++++ integration_tests/tools/coverage/BUILD | 1 + score_coverage/coverage_summary.py | 69 ++++++++++++++++++- score_coverage/generate_coverage_html.py | 42 +++++++++-- score_coverage/reporter.py | 59 +++++++++++++++- score_coverage/tests/coverage_summary_test.py | 34 +++++++++ .../tests/generate_coverage_html_test.py | 38 +++++++++- score_coverage/tests/reporter_test.py | 31 +++++++++ 16 files changed, 377 insertions(+), 22 deletions(-) create mode 100644 integration_tests/src/unused_api.h diff --git a/docs/manual/known_problems.rst b/docs/manual/known_problems.rst index ed8421e..eb9b547 100644 --- a/docs/manual/known_problems.rst +++ b/docs/manual/known_problems.rst @@ -70,6 +70,16 @@ stay listed with their upstream references. - ``WARNING: is compiled under several paths`` in the reporter log. - Expected; hits recorded only through the dropped variant are not counted. Include the header consistently. + * - **An in-scope header is absent from the report.** A header no + translation unit includes, or one that contains only templates that are + never instantiated, produces no code and therefore no coverage mapping; + ``llvm-cov`` cannot show it, not even at 0 %. + - The file is named in the job summary under "In-scope files without + coverage data", in ``unmapped_files.txt`` of the archive and in a + reporter ``WARNING``. Headers whose same-named source file has data are + listed apart as declaration-only (``declaration_only_headers.txt``). + - Decide per file: write a test that instantiates it (it is shipped API), + or remove it from the target's ``hdrs`` (it is not needed). * - **A source could not be staged for llvm-cov.** The reporter reads the sources from the scope's exported files; a file that is neither there nor in the workspace directory gets no HTML page (its numbers stay in diff --git a/docs/manual/user_manual.rst b/docs/manual/user_manual.rst index 39eedc1..8e0b6ec 100644 --- a/docs/manual/user_manual.rst +++ b/docs/manual/user_manual.rst @@ -249,8 +249,9 @@ Command reference * - ``--yaml `` - Justification YAML, relative to the workspace root. * - ``--archive-dir `` - - Assemble HTML report, ``coverage_report.dat`` (LCOV), justification - report and JUnit XMLs into ```` for artifact upload. + - Assemble HTML report, ``coverage_report.dat`` (LCOV), + ``unmapped_files.txt`` (in-scope files without any coverage data), + justification report and JUnit XMLs into ```` for artifact upload. * - ``--archive `` - Same content as a local ``.zip`` (do not upload it: upload-artifact zips again). diff --git a/docs/release/release_notes.rst b/docs/release/release_notes.rst index 441f3a1..ae8b340 100644 --- a/docs/release/release_notes.rst +++ b/docs/release/release_notes.rst @@ -43,6 +43,13 @@ Release notes ``_virtual_includes/`` path must be updated. - Changed: HTML pages live at ``coverage/.html``; the archive contains no directory of the producing machine any more. +- New: in-scope files that carry no coverage data at all (a header nothing + includes, template-only code that is never instantiated) are no longer + silently absent. The reporter writes ``text_report/unmapped_files.txt`` and + warns; ``generate_coverage_html`` prints them, archives the list as + ``unmapped_files.txt`` and adds a row and a section to the job summary + (``tool_req__coverage_report_unmapped``). Headers whose same-named source + file has data are listed apart as declaration-only. - Fixed: the exclusion filter matches each out-of-scope compiled file exactly; an excluded ``foo/bar.h`` no longer suppresses an in-scope ``src/foo/bar.h``. - ``score_coverage_scope`` gained the ``path_map`` and ``source_files`` output diff --git a/docs/requirements/tool_requirements.rst b/docs/requirements/tool_requirements.rst index a55ea15..c635b97 100644 --- a/docs/requirements/tool_requirements.rst +++ b/docs/requirements/tool_requirements.rst @@ -169,6 +169,27 @@ Report with all instrumented lines and branches at zero hits, so that the LCOV record shows ``LH:0``. +.. tool_req:: In-scope files without any coverage data are listed + :id: tool_req__coverage_report_unmapped + :version: 1 + :implemented: YES + :tags: report, ERR-01 + :safety: ASIL_B + :satisfies: stkh_req__coverage__uc_scope_completeness + + An allowlisted file for which neither a test binary nor a baseline object + carries a coverage mapping (no translation unit includes it, or it holds + only template code that is never instantiated) cannot be rendered by + ``llvm-cov``, not even at 0 %. The reporter shall write the sorted list of + such files to ``text_report/unmapped_files.txt`` (always present, empty + when there are none) and emit a warning naming them; ``generate_coverage_html`` shall + print them, copy the list into the archive as ``unmapped_files.txt`` and + show their number and names in the job summary. A header whose same-named + source file (same path without extension) has coverage data holds + declarations only; the reporter shall list such headers separately in + ``text_report/declaration_only_headers.txt`` and the summary shall show + them apart from the files above. Neither list contributes to any total. + .. tool_req:: Rust rlib archives are expanded into object members :id: tool_req__coverage_report_rlib_expansion :version: 1 diff --git a/docs/verification/verification_report.rst b/docs/verification/verification_report.rst index 7daef34..c1dac55 100644 --- a/docs/verification/verification_report.rst +++ b/docs/verification/verification_report.rst @@ -48,12 +48,12 @@ Test inventory - 20 - merge_profraw, merge_no_data, merge_tool_error * - ``//score_coverage/tests:reporter_test`` - - 58 + - 60 - report_merged_profile, report_allowlist, report_baseline_zero, report_rlib_expansion, report_missing_baseline, report_relative_paths, - report_outputs, scope_transitive + report_outputs, report_unmapped, scope_transitive * - ``//score_coverage/tests:justify_test`` - - 41 + - 55 - just_yaml, just_markers, just_unknown_id, just_platform, just_missing_file * - ``//score_coverage/tests:effective_coverage_test`` @@ -61,20 +61,20 @@ Test inventory - eff_metric, eff_stale, eff_branch_only, eff_path_match, eff_html, eff_gcovr * - ``//score_coverage/tests:generate_coverage_html_test`` - - 43 + - 63 - gate_threshold, gate_metric, gate_unrounded, gate_exit_codes, gate_no_verdict, summary_first, artifacts * - ``//score_coverage/tests:coverage_summary_test`` - - 17 + - 19 - summary_first * - ``//score_coverage/tests/starlark:coverage_scope_tests`` (13 analysis tests) - 13 - scope_transitive, scope_excludes, scope_baseline_objects - * - ``integration_tests/run_integration_test.sh`` (16 end-to-end checks) - - 16 + * - ``integration_tests/run_integration_test.sh`` (18 end-to-end checks) + - 18 - validation_ground_truth, report_baseline_zero, report_relative_paths, - report_allowlist, gate_exit_codes, gate_no_verdict, just_unknown_id, - artifacts, summary_first + report_allowlist, report_unmapped, gate_exit_codes, gate_no_verdict, + just_unknown_id, artifacts, summary_first Requirement coverage -------------------- diff --git a/integration_tests/expected_lcov.dat b/integration_tests/expected_lcov.dat index 0609f97..b0c4b64 100644 --- a/integration_tests/expected_lcov.dat +++ b/integration_tests/expected_lcov.dat @@ -42,6 +42,10 @@ # same shape, thrice() (18-20) called once, never_used() # (22-24) at 0 => 3 of 6 lines. Reported under the file's own # path in the external repository. +# (absent) src/unused_api.h: in scope, but no translation unit includes it +# and it is template-only, so no object carries a coverage +# mapping for it. Not in this file; reported by the pipeline +# as an in-scope file without coverage data instead. # (absent) external/itest_external+/extlib.cpp and extlib.h: compiled # with coverage and executed by coverable_test, but reached # only through the forwarding target //third_party:extlib, so diff --git a/integration_tests/run_integration_test.sh b/integration_tests/run_integration_test.sh index 2808705..c8cc4e7 100755 --- a/integration_tests/run_integration_test.sh +++ b/integration_tests/run_integration_test.sh @@ -66,8 +66,10 @@ rm -f summary.md COVERAGE_THRESHOLD=10 bazel run @score_coverage//:generate_coverage_html -- \ --yaml "${YAML}" --summary-md summary.md for marker in "## Coverage summary" "| Lines |" "Raw vs effective" \ - "Coverage by directory" "Files at exact 0% (2)"; do - if ! grep -qF "${marker}" summary.md; then + "Coverage by directory" "Files at exact 0% (2)" \ + "| In-scope files without coverage data | 1 |" \ + "In-scope files without coverage data (1)" '- `src/unused_api.h`'; do + if ! grep -qF -- "${marker}" summary.md; then echo "ERROR: '${marker}' missing from summary.md" >&2 exit 1 fi @@ -105,6 +107,16 @@ for f in artifacts_dir/coverage_linux/index.html artifacts_dir/coverage_report.d exit 1 fi done +if [[ "$(cat artifacts_dir/unmapped_files.txt)" != "src/unused_api.h" ]]; then + echo "ERROR: unmapped_files.txt should list exactly src/unused_api.h, got: $(cat artifacts_dir/unmapped_files.txt)" >&2 + exit 1 +fi +# coverable.h / uncovered.h hold declarations for compiled .cpp files: not findings. +if [[ "$(cat artifacts_dir/declaration_only_headers.txt | tr '\n' ' ')" != "src/coverable.h src/uncovered.h " ]]; then + echo "ERROR: declaration_only_headers.txt unexpected: $(cat artifacts_dir/declaration_only_headers.txt)" >&2 + exit 1 +fi +echo "OK: in-scope file without coverage data is listed in the archive; declaration-only headers kept apart" rm -rf artifacts_dir echo "OK: --archive-dir works" @@ -190,6 +202,13 @@ fi rm -rf link_check echo "OK: $(echo "${LINKS}" | wc -l) index links resolve, canonical paths only, third-party code excluded" +echo "=== A header nothing includes must be reported as unmapped, not invented in the LCOV ===" +if grep -q "unused_api" lcov.dat; then + echo "ERROR: src/unused_api.h has no compiled code and must not have an LCOV record" >&2 + exit 1 +fi +echo "OK" + echo "=== Covered files must be present with hits ===" grep -q "SF:.*src/coverable.cpp" lcov.dat || { echo "ERROR: coverable.cpp missing" >&2; exit 1; } grep -q "SF:.*rust/lib.rs" lcov.dat || { echo "ERROR: lib.rs missing" >&2; exit 1; } diff --git a/integration_tests/src/BUILD b/integration_tests/src/BUILD index 5425fc7..a4908a4 100644 --- a/integration_tests/src/BUILD +++ b/integration_tests/src/BUILD @@ -34,6 +34,13 @@ cc_library( # the compiler sees it via Bazel's generated _virtual_includes/ tree, the same # mechanism vendored third-party headers use (eclipse-score/baselibs#558). # One inline function is called by the test, the other never. +# In scope, but no source includes it and it is template-only: reported as +# "in-scope file without coverage data" (coverage_tool#5 follow-up). +cc_library( + name = "unused_api", + hdrs = ["unused_api.h"], +) + cc_library( name = "vendored_math", hdrs = ["vendored/include/vendored/inline_math.h"], diff --git a/integration_tests/src/unused_api.h b/integration_tests/src/unused_api.h new file mode 100644 index 0000000..732da73 --- /dev/null +++ b/integration_tests/src/unused_api.h @@ -0,0 +1,30 @@ +/******************************************************************************** + * Copyright (c) 2026 Contributors to the Eclipse Foundation + * + * See the NOTICE file(s) distributed with this work for additional + * information regarding copyright ownership. + * + * This program and the accompanying materials are made available under the + * terms of the Apache License Version 2.0 which is available at + * https://www.apache.org/licenses/LICENSE-2.0 + * + * SPDX-License-Identifier: Apache-2.0 + ********************************************************************************/ +#ifndef COVERAGE_INTEGRATION_TESTS_SRC_UNUSED_API_H +#define COVERAGE_INTEGRATION_TESTS_SRC_UNUSED_API_H + +// Public API nobody in this workspace includes, and template-only on top: +// no translation unit produces code from it, so llvm-cov has no coverage +// mapping for the file. The pipeline must still name it as an in-scope file +// without coverage data instead of silently leaving it out. +namespace coverage_integration { + +template +struct UnusedApi { + T value; + bool IsPositive() const { return value > T{}; } +}; + +} // namespace coverage_integration + +#endif // COVERAGE_INTEGRATION_TESTS_SRC_UNUSED_API_H diff --git a/integration_tests/tools/coverage/BUILD b/integration_tests/tools/coverage/BUILD index 75a1f40..8915e5a 100644 --- a/integration_tests/tools/coverage/BUILD +++ b/integration_tests/tools/coverage/BUILD @@ -25,6 +25,7 @@ score_coverage_scope( "//rust:untested_tool", "//src:coverable", "//src:uncovered", + "//src:unused_api", "//src:vendored_math", # Wrapper of a third-party library: visited by the aspect, must # contribute nothing (coverage_tool#5). diff --git a/score_coverage/coverage_summary.py b/score_coverage/coverage_summary.py index fc4d435..8c81224 100644 --- a/score_coverage/coverage_summary.py +++ b/score_coverage/coverage_summary.py @@ -181,6 +181,17 @@ def load_justification_summary(path: Path) -> dict | None: return summary +def load_unmapped_files(path: Path | None) -> list[str] | None: + """Read the reporter's unmapped-files list; None when no file was given or it is missing.""" + if path is None: + return None + if not path.is_file(): + print(f"WARNING: unmapped files list not found: {path}", file=sys.stderr) + return None + lines = path.read_text(encoding="utf-8", errors="replace").splitlines() + return sorted({line.strip() for line in lines if line.strip() and not line.startswith("#")}) + + def directory_key(path: str) -> str: """Group by the first one or two path segments (generic, layout-agnostic).""" parts = path.split("/") @@ -215,13 +226,46 @@ def rollup_by_directory(files: list[FileCoverage]) -> list[dict]: return rows -def render_markdown(files: list[FileCoverage], justification: dict | None) -> str: - """Render the full markdown summary (totals, raw vs effective, per-directory rollup, 0% files).""" +def _render_unmapped(unmapped: list[str], declaration_only: list[str] | None) -> list[str]: + """Sections for in-scope files that carry no coverage data at all.""" + out: list[str] = [] + if unmapped: + out += ["
", f"In-scope files without coverage data ({len(unmapped)})", ""] + out.append( + "These files are in the coverage scope, but no test binary or library archive contains " + "compiled code from them: nothing includes them, or they hold only template code that is " + "never instantiated. They count in no total above; each one is either untested or not needed." + ) + out.append("") + for path in sorted(unmapped): + out.append(f"- `{path}`") + out.extend(["", "
", ""]) + if declaration_only: + out += ["
", f"Declaration-only headers ({len(declaration_only)})", ""] + out.append( + "Headers without coverage data whose same-named source file has data; they hold " + "declarations only. Listed for completeness, no action expected." + ) + out.append("") + for path in sorted(declaration_only): + out.append(f"- `{path}`") + out.extend(["", "
", ""]) + return out + + +def render_markdown( + files: list[FileCoverage], + justification: dict | None, + unmapped: list[str] | None = None, + declaration_only: list[str] | None = None, +) -> str: + """Render the full markdown summary (totals, raw vs effective, rollup, 0 % files, unmapped files).""" out: list[str] = ["## Coverage summary", ""] if not files: out.append("_No coverage records found in the LCOV report._") out.append("") + out.extend(_render_unmapped(unmapped or [], declaration_only)) return "\n".join(out) total_lf = sum(f.lines_found for f in files) @@ -248,6 +292,8 @@ def render_markdown(files: list[FileCoverage], justification: dict | None) -> st f"{fmt_pct(touched_pct)} | {progress_bar(touched_pct)} |" ) out.append(f"| Files at exact 0% | {len(zero)} | {len(files)} | | |") + if unmapped is not None: + out.append(f"| In-scope files without coverage data | {len(unmapped)} | | | |") out.append("") if justification is not None: @@ -313,6 +359,8 @@ def render_markdown(files: list[FileCoverage], justification: dict | None) -> st out.append("") out.append("") + out.extend(_render_unmapped(unmapped or [], declaration_only)) + out.append("_Full per-line HTML report: download the coverage artifact of this run._") out.append("") return "\n".join(out) @@ -323,6 +371,18 @@ def main(argv: list[str] | None = None) -> None: parser = argparse.ArgumentParser(description="Markdown coverage summary from LCOV") parser.add_argument("--lcov", type=Path, required=True) parser.add_argument("--justification-report", type=Path, default=None) + parser.add_argument( + "--unmapped-files", + type=Path, + default=None, + help="reporter's text_report/unmapped_files.txt: in-scope files without any coverage data", + ) + parser.add_argument( + "--declaration-only-files", + type=Path, + default=None, + help="reporter's text_report/declaration_only_headers.txt: headers without own code", + ) parser.add_argument("--output", type=Path, required=True) parser.add_argument("--append", action="store_true") args = parser.parse_args(argv) @@ -336,7 +396,10 @@ def main(argv: list[str] | None = None) -> None: if args.justification_report is not None: justification = load_justification_summary(args.justification_report) - markdown = render_markdown(files, justification) + unmapped = load_unmapped_files(args.unmapped_files) + declaration_only = load_unmapped_files(args.declaration_only_files) + + markdown = render_markdown(files, justification, unmapped, declaration_only) args.output.parent.mkdir(parents=True, exist_ok=True) mode = "a" if args.append else "w" diff --git a/score_coverage/generate_coverage_html.py b/score_coverage/generate_coverage_html.py index 5704233..b2033b3 100644 --- a/score_coverage/generate_coverage_html.py +++ b/score_coverage/generate_coverage_html.py @@ -278,11 +278,17 @@ def write_summary( justification_dir: Path | None, summary_md: str | None, step_summary: str | None, + unmapped: Path | None = None, ) -> None: """Emit the markdown summary to --summary-md or, failing that, GITHUB_STEP_SUMMARY.""" args: list[str] = ["--lcov", str(lcov)] if justification_dir is not None and (justification_dir / "report.json").is_file(): args += ["--justification-report", str(justification_dir / "report.json")] + if unmapped is not None and unmapped.is_file(): + args += ["--unmapped-files", str(unmapped)] + declaration_only = unmapped.with_name("declaration_only_headers.txt") + if declaration_only.is_file(): + args += ["--declaration-only-files", str(declaration_only)] if summary_md: target = Path(summary_md) if not target.is_absolute(): @@ -294,6 +300,24 @@ def write_summary( print("Coverage summary appended to GITHUB_STEP_SUMMARY") +def report_unmapped_files(unmapped: Path) -> int: + """Print the in-scope files that have no coverage data; returns their number.""" + if not unmapped.is_file(): + return 0 + names = [line.strip() for line in unmapped.read_text(encoding="utf-8").splitlines() if line.strip()] + if names: + print( + f"WARNING: {len(names)} in-scope files have no coverage data (never compiled into a test " + f"binary or archive); see unmapped_files.txt in the archive and the job summary.", + file=sys.stderr, + ) + for name in names[:20]: + print(f" - {name}", file=sys.stderr) + if len(names) > 20: + print(f" ... and {len(names) - 20} more", file=sys.stderr) + return len(names) + + def assemble_artifacts( dest: Path, workspace: Path, @@ -301,8 +325,9 @@ def assemble_artifacts( output_dir: Path, lcov: Path, justification_dir: Path | None, + unmapped: Path | None = None, ) -> None: - """Copy JUnit XMLs (tree preserved), the HTML report, the LCOV and the justification report.""" + """Copy JUnit XMLs (tree preserved), HTML report, LCOV, unmapped-files list and justification report.""" dest.mkdir(parents=True, exist_ok=True) testlogs = workspace / "bazel-testlogs" / testlogs_subdir if testlogs_subdir else workspace / "bazel-testlogs" if not testlogs.is_dir(): @@ -315,6 +340,11 @@ def assemble_artifacts( shutil.copytree(output_dir, dest / output_dir.name, dirs_exist_ok=True) if lcov.is_file(): shutil.copy2(lcov, dest / "coverage_report.dat") + if unmapped is not None and unmapped.is_file(): + shutil.copy2(unmapped, dest / "unmapped_files.txt") + declaration_only = unmapped.with_name("declaration_only_headers.txt") + if declaration_only.is_file(): + shutil.copy2(declaration_only, dest / "declaration_only_headers.txt") if justification_dir is not None and justification_dir.is_dir(): shutil.copytree(justification_dir, dest / justification_dir.name, dirs_exist_ok=True) @@ -341,6 +371,8 @@ def run(opts: Options, workspace: Path, environ: dict | None = None) -> int: print(f"Coverage report written to: {output_dir}") lcov = extract_dir / "lcov_report" / "lcov.dat" + unmapped = extract_dir / "text_report" / "unmapped_files.txt" + report_unmapped_files(unmapped) justification_dir: Path | None = None if opts.yaml: justification_dir = extract_dir / "justification_report" @@ -354,7 +386,7 @@ def run(opts: Options, workspace: Path, environ: dict | None = None) -> int: # The summary is emitted BEFORE the gate decides, so a failing gate # still leaves it on the workflow run page. - write_summary(workspace, lcov, justification_dir, opts.summary_md, env.get("GITHUB_STEP_SUMMARY")) + write_summary(workspace, lcov, justification_dir, opts.summary_md, env.get("GITHUB_STEP_SUMMARY"), unmapped) if gate_passes(gate_pct, threshold): rc = EXIT_OK @@ -369,14 +401,16 @@ def run(opts: Options, workspace: Path, environ: dict | None = None) -> int: archive_dir = workspace / opts.archive_dir if archive_dir.exists(): shutil.rmtree(archive_dir) - assemble_artifacts(archive_dir, workspace, opts.testlogs_subdir, output_dir, lcov, justification_dir) + assemble_artifacts( + archive_dir, workspace, opts.testlogs_subdir, output_dir, lcov, justification_dir, unmapped + ) print(f"Coverage artifacts written to: {opts.archive_dir}/") if opts.archive: tree = workspace / "artifacts" if tree.exists(): shutil.rmtree(tree) - assemble_artifacts(tree, workspace, opts.testlogs_subdir, output_dir, lcov, justification_dir) + assemble_artifacts(tree, workspace, opts.testlogs_subdir, output_dir, lcov, justification_dir, unmapped) shutil.make_archive(str(workspace / opts.archive), "zip", root_dir=workspace, base_dir="artifacts") shutil.rmtree(tree) print(f"Coverage archive written to: {opts.archive}.zip") diff --git a/score_coverage/reporter.py b/score_coverage/reporter.py index 4fa1d3c..4a59a25 100644 --- a/score_coverage/reporter.py +++ b/score_coverage/reporter.py @@ -178,6 +178,12 @@ def show_html(objects: list[str]) -> None: with open(text_report_dir / "summary.txt", "w", encoding="utf-8") as f: f.write(summary_text) print(summary_text, file=sys.stderr) + # Always written, so consumers can rely on the file: one canonical path + # per line, empty when every in-scope file has coverage data. + with open(text_report_dir / "unmapped_files.txt", "w", encoding="utf-8") as f: + f.write("".join(name + "\n" for name in sorted(selection.unmapped))) + with open(text_report_dir / "declaration_only_headers.txt", "w", encoding="utf-8") as f: + f.write("".join(name + "\n" for name in sorted(selection.declaration_only))) # Package everything into the output zip. directories = [html_report_dir, lcov_report_dir, text_report_dir] @@ -275,6 +281,22 @@ class FileSelection: """canonical names that only the baseline archives contain (0 % entries).""" duplicates: dict[str, list[str]] = field(default_factory=dict) """canonical name -> raw variants dropped in favour of another variant.""" + unmapped: set[str] = field(default_factory=set) + """allowlisted files that no test binary or baseline archive has coverage data for.""" + declaration_only: set[str] = field(default_factory=set) + """unmapped headers whose same-named source file has coverage data (declarations only).""" + + +_HEADER_SUFFIXES = (".h", ".hpp", ".hh", ".hxx", ".inl", ".ipp", ".tpp") + + +def _is_header(path: str) -> bool: + return path.endswith(_HEADER_SUFFIXES) + + +def _stem(path: str) -> str: + """Path without its last extension: ``src/foo.h`` and ``src/foo.cpp`` share ``src/foo``.""" + return os.path.splitext(path)[0] def select_files( @@ -298,7 +320,28 @@ def in_scope(name: str) -> bool: excluded.update(raws) staged = {raw: name for raw, name in everything.items() if raw not in excluded} baseline_only = {name for name in set(baseline_covered.values()) - set(test_covered.values()) if in_scope(name)} - return FileSelection(staged=staged, excluded=excluded, baseline_only=baseline_only, duplicates=duplicates) + # In scope, but compiled into nothing: a header no translation unit + # includes, or template code that is never instantiated. llvm-cov cannot + # report such a file, not even at 0 %, so the reporter must. + unmapped: set[str] = set() + declaration_only: set[str] = set() + if allowlist is not None: + with_data = set(test_covered.values()) | set(baseline_covered.values()) + unmapped = allowlist - with_data + # A header whose same-named source file has data (foo.h next to a + # compiled foo.cpp) holds declarations only; that is expected and is + # kept apart from headers nothing compiles. + stems_with_data = {_stem(name) for name in with_data} + declaration_only = {name for name in unmapped if _is_header(name) and _stem(name) in stems_with_data} + unmapped -= declaration_only + return FileSelection( + staged=staged, + excluded=excluded, + baseline_only=baseline_only, + duplicates=duplicates, + unmapped=unmapped, + declaration_only=declaration_only, + ) def resolve_source(runfiles: RunfilesLike, canonical: str, workspace_root: str) -> str | None: @@ -613,6 +656,20 @@ def prepare_sources( f"(e.g., {sorted(selection.baseline_only)[:5]})", file=sys.stderr, ) + if selection.unmapped: + print( + f"WARNING: {len(selection.unmapped)} in-scope files have no coverage data at all (never " + f"included by a compiled translation unit, or template code that is never instantiated); " + f"listed in text_report/unmapped_files.txt (e.g., {sorted(selection.unmapped)[:5]})", + file=sys.stderr, + ) + if selection.declaration_only: + print( + f"INFO: {len(selection.declaration_only)} in-scope headers carry no code of their own " + f"(their same-named source file has coverage data); listed in " + f"text_report/declaration_only_headers.txt", + file=sys.stderr, + ) # Stage the in-scope sources under the raw covmap layout so llvm-cov can # read every one of them: generated headers (_virtual_includes/) and # vendored external headers do not exist below the workspace directory at diff --git a/score_coverage/tests/coverage_summary_test.py b/score_coverage/tests/coverage_summary_test.py index bba82f7..e77b9ab 100644 --- a/score_coverage/tests/coverage_summary_test.py +++ b/score_coverage/tests/coverage_summary_test.py @@ -18,11 +18,14 @@ # pylint: disable=missing-function-docstring,missing-class-docstring,protected-access,consider-using-with # pylint: disable=too-many-instance-attributes +import io import json import tempfile import unittest +from contextlib import redirect_stderr from pathlib import Path +from score_coverage import coverage_summary from score_coverage.coverage_summary import ( directory_key, load_justification_summary, @@ -168,6 +171,37 @@ def test_justification_section(self): self.assertIn("| Line coverage | 20.0% | 40.0% |", md) self.assertIn("1 justification entries applied", md) + def test_unmapped_files_row_and_section(self): + with tempfile.TemporaryDirectory() as tmp: + files = parse_lcov(_write(tmp, "l.dat", LCOV_TWO_FILES)) + assert files is not None + md = render_markdown(files, None, ["src/never.h", "src/api/tmpl.h"], ["src/a.h"]) + self.assertIn("| In-scope files without coverage data | 2 | | | |", md) + self.assertIn("In-scope files without coverage data (2)", md) + self.assertIn("- `src/api/tmpl.h`\n- `src/never.h`", md) + self.assertIn("never instantiated", md) + self.assertIn("Declaration-only headers (1)", md) + self.assertIn("- `src/a.h`", md) + # an empty list: row with 0, no section; None: neither + md = render_markdown(files, None, []) + self.assertIn("| In-scope files without coverage data | 0 | | | |", md) + self.assertNotIn("In-scope files without coverage data", md) + md = render_markdown(files, None) + self.assertNotIn("In-scope files without coverage data", md) + # even with no LCOV records the list is shown + md = render_markdown([], None, ["src/never.h"]) + self.assertIn("No coverage records", md) + self.assertIn("- `src/never.h`", md) + + def test_load_unmapped_files(self): + with tempfile.TemporaryDirectory() as tmp: + listing = _write(tmp, "u.txt", "# comment\nsrc/b.h\n\nsrc/a.h\nsrc/b.h\n") + self.assertEqual(coverage_summary.load_unmapped_files(listing), ["src/a.h", "src/b.h"]) + self.assertEqual(coverage_summary.load_unmapped_files(_write(tmp, "e.txt", "")), []) + with redirect_stderr(io.StringIO()): + self.assertIsNone(coverage_summary.load_unmapped_files(Path(tmp) / "missing.txt")) + self.assertIsNone(coverage_summary.load_unmapped_files(None)) + def test_branch_dash_when_no_branch_data(self): with tempfile.TemporaryDirectory() as tmp: files = parse_lcov(_write(tmp, "l.dat", "SF:a.cpp\nLF:2\nLH:1\nend_of_record\n")) diff --git a/score_coverage/tests/generate_coverage_html_test.py b/score_coverage/tests/generate_coverage_html_test.py index d3f8574..080dd51 100644 --- a/score_coverage/tests/generate_coverage_html_test.py +++ b/score_coverage/tests/generate_coverage_html_test.py @@ -48,7 +48,11 @@ def _write(path: Path, content: str) -> Path: def _make_workspace( - root: Path, lcov: str = LCOV_25_PERCENT, with_html: bool = True, with_testlogs: bool = True + root: Path, + lcov: str = LCOV_25_PERCENT, + with_html: bool = True, + with_testlogs: bool = True, + unmapped: str | None = "", ) -> Path: """Create a fake consumer workspace with a reporter zip and test logs.""" report = root / gch.COVERAGE_REPORT_REL @@ -59,6 +63,9 @@ def _make_workspace( zf.writestr("html_report/style.css", "body {}") zf.writestr("lcov_report/lcov.dat", lcov) zf.writestr("text_report/summary.txt", "TOTAL 50%") + if unmapped is not None: + zf.writestr("text_report/unmapped_files.txt", unmapped) + zf.writestr("text_report/declaration_only_headers.txt", "src/decl.h\n") if with_testlogs: _write(root / "bazel-testlogs" / "pkg" / "some_test" / "test.xml", "") _write(root / "bazel-testlogs" / "pkg" / "some_test" / "test.log", "log") @@ -295,6 +302,35 @@ def test_explicit_summary_md_wins_over_step_summary(self): self.assertEqual(step.read_text(encoding="utf-8"), "# existing content\n") self.assertTrue((self.root / "s.md").is_file()) + def test_unmapped_files_are_reported_summarised_and_archived(self): + with tempfile.TemporaryDirectory() as tmp: + root = _make_workspace(Path(tmp), unmapped="src/never.h\nsrc/api/tmpl.h\n") + rc, _, err = _run( + root, + ["--summary-md", "summary.md", "--archive-dir", "artifacts_dir", "--testlogs-subdir", "pkg"], + {"COVERAGE_THRESHOLD": "0"}, + ) + self.assertEqual(rc, gch.EXIT_OK) + self.assertIn("WARNING: 2 in-scope files have no coverage data", err) + self.assertIn(" - src/never.h", err) + summary = (root / "summary.md").read_text(encoding="utf-8") + self.assertIn("| In-scope files without coverage data | 2 | | | |", summary) + self.assertIn("- `src/never.h`", summary) + self.assertIn("Declaration-only headers (1)", summary) + archived = (root / "artifacts_dir" / "unmapped_files.txt").read_text(encoding="utf-8") + self.assertEqual(archived, "src/never.h\nsrc/api/tmpl.h\n") + decl = (root / "artifacts_dir" / "declaration_only_headers.txt").read_text(encoding="utf-8") + self.assertEqual(decl, "src/decl.h\n") + + def test_missing_unmapped_list_is_tolerated(self): + # Reports produced by an older reporter carry no list: no warning, no row. + with tempfile.TemporaryDirectory() as tmp: + root = _make_workspace(Path(tmp), unmapped=None) + rc, _, err = _run(root, ["--summary-md", "summary.md"], {"COVERAGE_THRESHOLD": "0"}) + self.assertEqual(rc, gch.EXIT_OK) + self.assertNotIn("no coverage data", err) + self.assertNotIn("In-scope files without coverage data", (root / "summary.md").read_text(encoding="utf-8")) + def test_archive_dir_layout(self): rc, out, _ = _run( self.root, ["--archive-dir", "artifacts_dir", "--testlogs-subdir", "pkg"], {"COVERAGE_THRESHOLD": "0"} diff --git a/score_coverage/tests/reporter_test.py b/score_coverage/tests/reporter_test.py index 8338d74..9ef0679 100644 --- a/score_coverage/tests/reporter_test.py +++ b/score_coverage/tests/reporter_test.py @@ -600,6 +600,9 @@ def test_full_report(self): index = zf.read("html_report/index.html").decode() self.assertIn("html_report/index.html", names) self.assertIn("html_report/style.css", names) + # Every in-scope file has data here, so the unmapped lists exist and are empty. + self.assertIn("text_report/unmapped_files.txt", names) + self.assertIn("text_report/declaration_only_headers.txt", names) # Pages are filed under canonical paths: no machine-specific directory # remains, the index links there, and the page's asset links still resolve. self.assertNotIn("html_report/coverage/" + str(self.workdir).strip("/") + "/", "\n".join(names)) @@ -656,6 +659,18 @@ def test_path_map_files_headers_under_declared_path(self): self.assertTrue((self.workdir / "sources" / "src" / "a.cpp").is_symlink()) self.assertIn("1 headers behind include prefixes", err.getvalue()) + def test_in_scope_file_without_coverage_data_is_listed(self): + self.allowlist.write_text("src/a.cpp\nsrc/a.h\nsrc/b.cpp\nsrc/never.h\n", encoding="utf-8") + err = io.StringIO() + with redirect_stderr(err): + reporter.main(self._argv()) + with zipfile.ZipFile(self.output) as zf: + self.assertEqual(zf.read("text_report/unmapped_files.txt").decode(), "src/never.h\n") + self.assertEqual(zf.read("text_report/declaration_only_headers.txt").decode(), "src/a.h\n") + self.assertIn("1 in-scope files have no coverage data at all", err.getvalue()) + self.assertIn("src/never.h", err.getvalue()) + self.assertIn("1 in-scope headers carry no code of their own", err.getvalue()) + def test_no_reports_writes_empty_zip(self): self.reports_file.write_text("", encoding="utf-8") with redirect_stderr(io.StringIO()): @@ -766,6 +781,21 @@ def test_out_of_scope_and_redundant_variants_are_excluded(self): ) self.assertEqual(sel.baseline_only, {"src/untested.cpp"}) self.assertEqual(sel.duplicates, {}) + self.assertEqual(sel.unmapped, set()) + + def test_allowlisted_file_without_any_coverage_data_is_reported(self): + # A header nothing includes (or template-only code) has no coverage + # mapping anywhere; llvm-cov cannot show it, so the selection must. + test = {"src/a.cpp": "src/a.cpp"} + baseline = {"src/a.cpp": "src/a.cpp", "src/b.cpp": "src/b.cpp"} + allowlist = {"src/a.cpp", "src/a.h", "src/b.cpp", "src/b.hpp", "src/never.h", "src/tmpl.h", "src/orphan.cpp"} + sel = reporter.select_files(test, baseline, allowlist) + # a.h / b.hpp sit next to compiled a.cpp / b.cpp: declarations only. + # never.h, tmpl.h and a source file nobody built are the real findings. + self.assertEqual(sel.unmapped, {"src/never.h", "src/tmpl.h", "src/orphan.cpp"}) + self.assertEqual(sel.declaration_only, {"src/a.h", "src/b.hpp"}) + self.assertEqual(sel.baseline_only, {"src/b.cpp"}) + self.assertEqual(sel.excluded, set()) def test_duplicate_test_variants_keep_the_declared_path(self): test = { @@ -793,6 +823,7 @@ def test_no_allowlist_keeps_everything(self): self.assertEqual(sel.staged, {"a": "a", "b": "b"}) self.assertEqual(sel.excluded, set()) self.assertEqual(sel.baseline_only, {"b"}) + self.assertEqual(sel.unmapped, set()) @verifies("tool_req__coverage_report_relative_paths", "tool_req__coverage_report_outputs") From 194541c9ed5240fb13f20673ca37d8700654caec Mon Sep 17 00:00:00 2001 From: Dan Calavrezo <195309321+dcalavrezo-qorix@users.noreply.github.com> Date: Tue, 15 Sep 2026 17:20:25 +0300 Subject: [PATCH 3/6] Categorise files without coverage data; keep archives with empty members Follow-up on the unmapped-files diagnostic, driven by what it showed on baselibs (377 entries for three packages, most of them not findings): - The list is now "\t" with three categories: declaration-only (a header whose same-named source has data), empty-translation-unit (a source compiled into a baseline archive whose object carries no coverage mapping, i.e. the placeholder .cpp of a header-only library), and no-data (the findings: a header nothing includes, template code never instantiated, a source never built). Only no-data is warned about and counted in the job summary table; each category gets its own collapsible section. - Real bug found on the way: llvm-cov rejects an archive as a whole ("no coverage data found") as soon as ONE member has no __llvm_covmap section. A library with a placeholder .cpp next to real sources therefore silently lost the 0 % baseline of all its other files. The reporter now inspects every member's ELF section table and passes only members with a mapping to llvm-cov, generalising the rlib expansion (tool_req__coverage_report_rlib_expansion v2). The dropped members are what identifies the empty-translation-unit category. Tests: synthetic ELF64 objects for the covmap probe and archive splitting (rlib, plain, mixed, all-empty, executable); selection with the three categories; summary loader and renderer; generator wiring. The integration workspace's uncovered library gains a placeholder .cpp, which without the archive fix removes uncovered.cpp from the golden LCOV, and the archive check expects the categorised list. Docs: tool_req__coverage_report_unmapped and _rlib_expansion, known problems, release notes, verification inventory. Signed-off-by: Dan Calavrezo <195309321+dcalavrezo-qorix@users.noreply.github.com> --- docs/manual/known_problems.rst | 18 +- docs/release/release_notes.rst | 11 +- docs/requirements/tool_requirements.rst | 38 +-- docs/verification/verification_report.rst | 2 +- integration_tests/expected_lcov.dat | 3 + integration_tests/run_integration_test.sh | 18 +- integration_tests/src/BUILD | 5 +- integration_tests/src/empty_unit.cpp | 16 ++ score_coverage/coverage_summary.py | 96 +++++--- score_coverage/generate_coverage_html.py | 21 +- score_coverage/reporter.py | 231 +++++++++++++----- score_coverage/tests/coverage_summary_test.py | 27 +- .../tests/generate_coverage_html_test.py | 9 +- score_coverage/tests/reporter_test.py | 166 +++++++++++-- 14 files changed, 481 insertions(+), 180 deletions(-) create mode 100644 integration_tests/src/empty_unit.cpp diff --git a/docs/manual/known_problems.rst b/docs/manual/known_problems.rst index eb9b547..7217673 100644 --- a/docs/manual/known_problems.rst +++ b/docs/manual/known_problems.rst @@ -57,11 +57,13 @@ stay listed with their upstream references. - Exit 2 with ``is not the LLVM pipeline zip report`` on a gcov run. - Use the Linux host pipeline; QNX centralisation is tracked in tooling issue #427. - * - **Rust rlib archives are rejected by llvm-cov** because of the leading - ``lib.rmeta`` member. - - ``no coverage data found`` on a Rust archive. - - Handled since the pipeline expands rlibs into their object members; if - seen, the installed version predates the fix. + * - **An archive is rejected by llvm-cov** because one member has no + coverage mapping: the ``lib.rmeta`` of a Rust rlib, or the object of an + empty translation unit. + - ``no coverage data found`` on an archive; untested files of that library + missing from the report. + - Handled since the pipeline passes only members with a mapping to + llvm-cov; if seen, the installed version predates the fix. * - **A header compiled under two different paths is reported once.** When a translation unit includes a header through its ``_virtual_includes/`` path and another through the declared path, the compiler produces two @@ -76,8 +78,10 @@ stay listed with their upstream references. ``llvm-cov`` cannot show it, not even at 0 %. - The file is named in the job summary under "In-scope files without coverage data", in ``unmapped_files.txt`` of the archive and in a - reporter ``WARNING``. Headers whose same-named source file has data are - listed apart as declaration-only (``declaration_only_headers.txt``). + reporter ``WARNING``. Headers whose same-named source file has data + (declaration-only) and placeholder sources compiled into an archive + without code are listed in the same file under their own category and + are not findings. - Decide per file: write a test that instantiates it (it is shipped API), or remove it from the target's ``hdrs`` (it is not needed). * - **A source could not be staged for llvm-cov.** The reporter reads the diff --git a/docs/release/release_notes.rst b/docs/release/release_notes.rst index ae8b340..0365fcb 100644 --- a/docs/release/release_notes.rst +++ b/docs/release/release_notes.rst @@ -48,8 +48,15 @@ Release notes silently absent. The reporter writes ``text_report/unmapped_files.txt`` and warns; ``generate_coverage_html`` prints them, archives the list as ``unmapped_files.txt`` and adds a row and a section to the job summary - (``tool_req__coverage_report_unmapped``). Headers whose same-named source - file has data are listed apart as declaration-only. + (``tool_req__coverage_report_unmapped``). Declaration-only headers and + placeholder sources compiled without code are categorised separately and + are not findings. +- Fixed: a library archive with one member lacking a coverage mapping (the + placeholder ``.cpp`` of a header-only library) was rejected by ``llvm-cov`` + as a whole, so the library's other untested files silently lost their 0 % + baseline. Archive members are now inspected and only those with a mapping + are passed on, the way Rust rlibs were already handled + (``tool_req__coverage_report_rlib_expansion`` v2). - Fixed: the exclusion filter matches each out-of-scope compiled file exactly; an excluded ``foo/bar.h`` no longer suppresses an in-scope ``src/foo/bar.h``. - ``score_coverage_scope`` gained the ``path_map`` and ``source_files`` output diff --git a/docs/requirements/tool_requirements.rst b/docs/requirements/tool_requirements.rst index c635b97..f54caaa 100644 --- a/docs/requirements/tool_requirements.rst +++ b/docs/requirements/tool_requirements.rst @@ -180,27 +180,35 @@ Report An allowlisted file for which neither a test binary nor a baseline object carries a coverage mapping (no translation unit includes it, or it holds only template code that is never instantiated) cannot be rendered by - ``llvm-cov``, not even at 0 %. The reporter shall write the sorted list of - such files to ``text_report/unmapped_files.txt`` (always present, empty - when there are none) and emit a warning naming them; ``generate_coverage_html`` shall - print them, copy the list into the archive as ``unmapped_files.txt`` and - show their number and names in the job summary. A header whose same-named - source file (same path without extension) has coverage data holds - declarations only; the reporter shall list such headers separately in - ``text_report/declaration_only_headers.txt`` and the summary shall show - them apart from the files above. Neither list contributes to any total. - -.. tool_req:: Rust rlib archives are expanded into object members + ``llvm-cov``, not even at 0 %. The reporter shall write every such file to + ``text_report/unmapped_files.txt`` (always present, empty when there are + none) as ``\t``, sorted, with one of three categories: + ``declaration-only`` for a header whose same-named source file (same path + without extension) has coverage data, ``empty-translation-unit`` for a + source whose object is a member of a baseline archive, and ``no-data`` for + everything else. The reporter shall emit a warning naming the ``no-data`` + files; ``generate_coverage_html`` shall print them, copy the list into the + archive as ``unmapped_files.txt`` and show the ``no-data`` count in the job + summary table with one collapsible section per category. None of the + categories contributes to any total. + +.. tool_req:: Baseline archives are reduced to members with a coverage mapping :id: tool_req__coverage_report_rlib_expansion - :version: 1 + :version: 2 :implemented: YES :tags: report, ERR-07 :safety: ASIL_B :satisfies: stkh_req__coverage__uc_scope_completeness - Before passing baseline archives to ``llvm-cov``, the reporter shall detect - archives with a ``lib.rmeta`` member and replace them by their ``.o`` - members, so that Rust libraries are not rejected as having no coverage data. + ``llvm-cov`` rejects an archive as a whole as soon as one member has no + ``__llvm_covmap`` section: the ``lib.rmeta`` member of a Rust rlib, or the + object of a translation unit without code (the placeholder source of a + header-only library). Before passing baseline archives to ``llvm-cov``, the + reporter shall inspect every member's ELF section table and replace such + an archive by its members that carry a mapping, so that no library loses + its zero-coverage baseline because of one member; the dropped object + members shall identify the sources reported as compiled without code + (:need:`tool_req__coverage_report_unmapped`). .. tool_req:: A missing baseline object is an error :id: tool_req__coverage_report_missing_baseline diff --git a/docs/verification/verification_report.rst b/docs/verification/verification_report.rst index c1dac55..b512ab7 100644 --- a/docs/verification/verification_report.rst +++ b/docs/verification/verification_report.rst @@ -48,7 +48,7 @@ Test inventory - 20 - merge_profraw, merge_no_data, merge_tool_error * - ``//score_coverage/tests:reporter_test`` - - 60 + - 64 - report_merged_profile, report_allowlist, report_baseline_zero, report_rlib_expansion, report_missing_baseline, report_relative_paths, report_outputs, report_unmapped, scope_transitive diff --git a/integration_tests/expected_lcov.dat b/integration_tests/expected_lcov.dat index b0c4b64..8b85446 100644 --- a/integration_tests/expected_lcov.dat +++ b/integration_tests/expected_lcov.dat @@ -46,6 +46,9 @@ # and it is template-only, so no object carries a coverage # mapping for it. Not in this file; reported by the pipeline # as an in-scope file without coverage data instead. +# (absent) src/empty_unit.cpp: placeholder translation unit of the +# uncovered library, compiled into its archive without a +# coverage mapping; reported as compiled source without code. # (absent) external/itest_external+/extlib.cpp and extlib.h: compiled # with coverage and executed by coverable_test, but reached # only through the forwarding target //third_party:extlib, so diff --git a/integration_tests/run_integration_test.sh b/integration_tests/run_integration_test.sh index c8cc4e7..2f143fe 100755 --- a/integration_tests/run_integration_test.sh +++ b/integration_tests/run_integration_test.sh @@ -68,7 +68,8 @@ COVERAGE_THRESHOLD=10 bazel run @score_coverage//:generate_coverage_html -- \ for marker in "## Coverage summary" "| Lines |" "Raw vs effective" \ "Coverage by directory" "Files at exact 0% (2)" \ "| In-scope files without coverage data | 1 |" \ - "In-scope files without coverage data (1)" '- `src/unused_api.h`'; do + "In-scope files without coverage data (1)" '- `src/unused_api.h`' \ + "Declaration-only headers (2)" "Compiled sources without code (1)" '- `src/empty_unit.cpp`'; do if ! grep -qF -- "${marker}" summary.md; then echo "ERROR: '${marker}' missing from summary.md" >&2 exit 1 @@ -107,16 +108,15 @@ for f in artifacts_dir/coverage_linux/index.html artifacts_dir/coverage_report.d exit 1 fi done -if [[ "$(cat artifacts_dir/unmapped_files.txt)" != "src/unused_api.h" ]]; then - echo "ERROR: unmapped_files.txt should list exactly src/unused_api.h, got: $(cat artifacts_dir/unmapped_files.txt)" >&2 +# unused_api.h is the finding; coverable.h / uncovered.h hold declarations for +# compiled .cpp files and empty_unit.cpp is a compiled placeholder: categorised. +EXPECTED_UNMAPPED=$'declaration-only\tsrc/coverable.h\ndeclaration-only\tsrc/uncovered.h\nempty-translation-unit\tsrc/empty_unit.cpp\nno-data\tsrc/unused_api.h' +if [[ "$(cat artifacts_dir/unmapped_files.txt)" != "${EXPECTED_UNMAPPED}" ]]; then + echo "ERROR: unmapped_files.txt unexpected:" >&2 + cat artifacts_dir/unmapped_files.txt >&2 exit 1 fi -# coverable.h / uncovered.h hold declarations for compiled .cpp files: not findings. -if [[ "$(cat artifacts_dir/declaration_only_headers.txt | tr '\n' ' ')" != "src/coverable.h src/uncovered.h " ]]; then - echo "ERROR: declaration_only_headers.txt unexpected: $(cat artifacts_dir/declaration_only_headers.txt)" >&2 - exit 1 -fi -echo "OK: in-scope file without coverage data is listed in the archive; declaration-only headers kept apart" +echo "OK: in-scope files without coverage data are listed and categorised in the archive" rm -rf artifacts_dir echo "OK: --archive-dir works" diff --git a/integration_tests/src/BUILD b/integration_tests/src/BUILD index a4908a4..49f5082 100644 --- a/integration_tests/src/BUILD +++ b/integration_tests/src/BUILD @@ -26,7 +26,10 @@ cc_library( # report via the --empty-profile baseline. cc_library( name = "uncovered", - srcs = ["uncovered.cpp"], + srcs = [ + "empty_unit.cpp", + "uncovered.cpp", + ], hdrs = ["uncovered.h"], ) diff --git a/integration_tests/src/empty_unit.cpp b/integration_tests/src/empty_unit.cpp new file mode 100644 index 0000000..2eacd4f --- /dev/null +++ b/integration_tests/src/empty_unit.cpp @@ -0,0 +1,16 @@ +/******************************************************************************** + * Copyright (c) 2026 Contributors to the Eclipse Foundation + * + * See the NOTICE file(s) distributed with this work for additional + * information regarding copyright ownership. + * + * This program and the accompanying materials are made available under the + * terms of the Apache License Version 2.0 which is available at + * https://www.apache.org/licenses/LICENSE-2.0 + * + * SPDX-License-Identifier: Apache-2.0 + ********************************************************************************/ +// Placeholder translation unit of a header-only library: compiled into the +// archive, but it produces no coverage mapping. Reported as "compiled source +// without code", not as a finding. +#include "src/uncovered.h" diff --git a/score_coverage/coverage_summary.py b/score_coverage/coverage_summary.py index 8c81224..48e1351 100644 --- a/score_coverage/coverage_summary.py +++ b/score_coverage/coverage_summary.py @@ -181,15 +181,26 @@ def load_justification_summary(path: Path) -> dict | None: return summary -def load_unmapped_files(path: Path | None) -> list[str] | None: - """Read the reporter's unmapped-files list; None when no file was given or it is missing.""" +def load_unmapped_files(path: Path | None) -> dict[str, list[str]] | None: + """Read the reporter's ``\\t`` list; None when no file was given or it is missing. + + A line without a tab is a bare path of the ``no-data`` category. + """ if path is None: return None if not path.is_file(): print(f"WARNING: unmapped files list not found: {path}", file=sys.stderr) return None - lines = path.read_text(encoding="utf-8", errors="replace").splitlines() - return sorted({line.strip() for line in lines if line.strip() and not line.startswith("#")}) + result: dict[str, set[str]] = {} + for line in path.read_text(encoding="utf-8", errors="replace").splitlines(): + line = line.strip() + if not line or line.startswith("#"): + continue + category, tab, name = line.partition("\t") + if not tab: + category, name = "no-data", category + result.setdefault(category, set()).add(name.strip()) + return {category: sorted(names) for category, names in result.items()} def directory_key(path: str) -> str: @@ -226,28 +237,38 @@ def rollup_by_directory(files: list[FileCoverage]) -> list[dict]: return rows -def _render_unmapped(unmapped: list[str], declaration_only: list[str] | None) -> list[str]: - """Sections for in-scope files that carry no coverage data at all.""" +UNMAPPED_SECTIONS = ( + ( + "no-data", + "In-scope files without coverage data", + "These files are in the coverage scope, but no test binary or library archive contains " + "compiled code from them: nothing includes them, or they hold only template code that is " + "never instantiated. They count in no total above; each one is either untested or not needed.", + ), + ( + "declaration-only", + "Declaration-only headers", + "Headers without coverage data whose same-named source file has data; they hold " + "declarations only. Listed for completeness, no action expected.", + ), + ( + "empty-translation-unit", + "Compiled sources without code", + "Sources compiled into a library archive that produced no coverage mapping, typically " + "the placeholder .cpp of a header-only library. Nothing to cover.", + ), +) + + +def _render_unmapped(unmapped: dict[str, list[str]]) -> list[str]: + """One collapsible section per category of in-scope files without coverage data.""" out: list[str] = [] - if unmapped: - out += ["
", f"In-scope files without coverage data ({len(unmapped)})", ""] - out.append( - "These files are in the coverage scope, but no test binary or library archive contains " - "compiled code from them: nothing includes them, or they hold only template code that is " - "never instantiated. They count in no total above; each one is either untested or not needed." - ) - out.append("") - for path in sorted(unmapped): - out.append(f"- `{path}`") - out.extend(["", "
", ""]) - if declaration_only: - out += ["
", f"Declaration-only headers ({len(declaration_only)})", ""] - out.append( - "Headers without coverage data whose same-named source file has data; they hold " - "declarations only. Listed for completeness, no action expected." - ) - out.append("") - for path in sorted(declaration_only): + for category, title, explanation in UNMAPPED_SECTIONS: + paths = unmapped.get(category) + if not paths: + continue + out += ["
", f"{title} ({len(paths)})", "", explanation, ""] + for path in sorted(paths): out.append(f"- `{path}`") out.extend(["", "
", ""]) return out @@ -256,16 +277,20 @@ def _render_unmapped(unmapped: list[str], declaration_only: list[str] | None) -> def render_markdown( files: list[FileCoverage], justification: dict | None, - unmapped: list[str] | None = None, - declaration_only: list[str] | None = None, + unmapped: dict[str, list[str]] | None = None, ) -> str: - """Render the full markdown summary (totals, raw vs effective, rollup, 0 % files, unmapped files).""" + """Render the full markdown summary (totals, raw vs effective, rollup, 0 % files, unmapped files). + + ``unmapped`` maps a category (see :data:`UNMAPPED_SECTIONS`) to the + in-scope files without coverage data; ``None`` when the report carries + no such list. + """ out: list[str] = ["## Coverage summary", ""] if not files: out.append("_No coverage records found in the LCOV report._") out.append("") - out.extend(_render_unmapped(unmapped or [], declaration_only)) + out.extend(_render_unmapped(unmapped or {})) return "\n".join(out) total_lf = sum(f.lines_found for f in files) @@ -293,7 +318,7 @@ def render_markdown( ) out.append(f"| Files at exact 0% | {len(zero)} | {len(files)} | | |") if unmapped is not None: - out.append(f"| In-scope files without coverage data | {len(unmapped)} | | | |") + out.append(f"| In-scope files without coverage data | {len(unmapped.get('no-data', []))} | | | |") out.append("") if justification is not None: @@ -359,7 +384,7 @@ def render_markdown( out.append("
") out.append("") - out.extend(_render_unmapped(unmapped or [], declaration_only)) + out.extend(_render_unmapped(unmapped or {})) out.append("_Full per-line HTML report: download the coverage artifact of this run._") out.append("") @@ -377,12 +402,6 @@ def main(argv: list[str] | None = None) -> None: default=None, help="reporter's text_report/unmapped_files.txt: in-scope files without any coverage data", ) - parser.add_argument( - "--declaration-only-files", - type=Path, - default=None, - help="reporter's text_report/declaration_only_headers.txt: headers without own code", - ) parser.add_argument("--output", type=Path, required=True) parser.add_argument("--append", action="store_true") args = parser.parse_args(argv) @@ -397,9 +416,8 @@ def main(argv: list[str] | None = None) -> None: justification = load_justification_summary(args.justification_report) unmapped = load_unmapped_files(args.unmapped_files) - declaration_only = load_unmapped_files(args.declaration_only_files) - markdown = render_markdown(files, justification, unmapped, declaration_only) + markdown = render_markdown(files, justification, unmapped) args.output.parent.mkdir(parents=True, exist_ok=True) mode = "a" if args.append else "w" diff --git a/score_coverage/generate_coverage_html.py b/score_coverage/generate_coverage_html.py index b2033b3..dda41bf 100644 --- a/score_coverage/generate_coverage_html.py +++ b/score_coverage/generate_coverage_html.py @@ -286,9 +286,6 @@ def write_summary( args += ["--justification-report", str(justification_dir / "report.json")] if unmapped is not None and unmapped.is_file(): args += ["--unmapped-files", str(unmapped)] - declaration_only = unmapped.with_name("declaration_only_headers.txt") - if declaration_only.is_file(): - args += ["--declaration-only-files", str(declaration_only)] if summary_md: target = Path(summary_md) if not target.is_absolute(): @@ -301,10 +298,21 @@ def write_summary( def report_unmapped_files(unmapped: Path) -> int: - """Print the in-scope files that have no coverage data; returns their number.""" + """Print the in-scope files that have no coverage data and no benign explanation; returns their number. + + The list is ``\\t``; only the ``no-data`` category is a + finding, the others (declaration-only headers, compiled sources without + code) stay in the file and the job summary. + """ if not unmapped.is_file(): return 0 - names = [line.strip() for line in unmapped.read_text(encoding="utf-8").splitlines() if line.strip()] + names = [] + for line in unmapped.read_text(encoding="utf-8").splitlines(): + category, tab, name = line.strip().partition("\t") + if not tab: + category, name = "no-data", category + if name and category == "no-data": + names.append(name) if names: print( f"WARNING: {len(names)} in-scope files have no coverage data (never compiled into a test " @@ -342,9 +350,6 @@ def assemble_artifacts( shutil.copy2(lcov, dest / "coverage_report.dat") if unmapped is not None and unmapped.is_file(): shutil.copy2(unmapped, dest / "unmapped_files.txt") - declaration_only = unmapped.with_name("declaration_only_headers.txt") - if declaration_only.is_file(): - shutil.copy2(declaration_only, dest / "declaration_only_headers.txt") if justification_dir is not None and justification_dir.is_dir(): shutil.copytree(justification_dir, dest / justification_dir.name, dirs_exist_ok=True) diff --git a/score_coverage/reporter.py b/score_coverage/reporter.py index 4a59a25..fa950a3 100644 --- a/score_coverage/reporter.py +++ b/score_coverage/reporter.py @@ -96,16 +96,15 @@ def main(argv: list[str] | None = None) -> None: ) # Load baseline objects (production library archives) for zero-coverage baseline. - baseline_objects = load_baseline_objects(r, args.baseline_objects) - - # Rust rlib archives (exposed as .a symlinks by rules_rust) start with a - # lib.rmeta member, which makes llvm-cov reject the whole archive with - # "no coverage data found" even though the .o members carry the covmap. - # Expand such archives into their object members. - baseline_objects = expand_rlib_archives(baseline_objects, Path.cwd() / "rlib_baseline_objects") + # Load baseline objects (production library archives) for zero-coverage + # baseline. llvm-cov rejects an archive as a whole when one member lacks + # a coverage mapping (the lib.rmeta of a Rust rlib, the object of an empty + # translation unit), so such archives are replaced by their usable members. + baseline_manifest = load_baseline_manifest(r, args.baseline_objects) + baseline_objects, empty_stems = expand_baseline_archives(baseline_manifest, Path.cwd() / "baseline_objects") selection, path_map, root, regexes = prepare_sources( - r, args, llvm_bin_path, sorted_objects, str(merged_profdata), baseline_objects + r, args, llvm_bin_path, sorted_objects, str(merged_profdata), baseline_objects, empty_stems ) # All valid baseline archives are passed when baseline-only files exist; # _filter_lcov keeps only those files' records. @@ -178,12 +177,10 @@ def show_html(objects: list[str]) -> None: with open(text_report_dir / "summary.txt", "w", encoding="utf-8") as f: f.write(summary_text) print(summary_text, file=sys.stderr) - # Always written, so consumers can rely on the file: one canonical path + # Always written, so consumers can rely on the file: "\t" # per line, empty when every in-scope file has coverage data. with open(text_report_dir / "unmapped_files.txt", "w", encoding="utf-8") as f: - f.write("".join(name + "\n" for name in sorted(selection.unmapped))) - with open(text_report_dir / "declaration_only_headers.txt", "w", encoding="utf-8") as f: - f.write("".join(name + "\n" for name in sorted(selection.declaration_only))) + f.write(format_unmapped_files(selection)) # Package everything into the output zip. directories = [html_report_dir, lcov_report_dir, text_report_dir] @@ -282,9 +279,11 @@ class FileSelection: duplicates: dict[str, list[str]] = field(default_factory=dict) """canonical name -> raw variants dropped in favour of another variant.""" unmapped: set[str] = field(default_factory=set) - """allowlisted files that no test binary or baseline archive has coverage data for.""" + """allowlisted files without coverage data anywhere, and no benign explanation: the findings.""" declaration_only: set[str] = field(default_factory=set) """unmapped headers whose same-named source file has coverage data (declarations only).""" + empty_units: set[str] = field(default_factory=set) + """unmapped sources that were compiled into a baseline archive but produced no coverage mapping.""" _HEADER_SUFFIXES = (".h", ".hpp", ".hh", ".hxx", ".inl", ".ipp", ".tpp") @@ -303,10 +302,14 @@ def select_files( test_covered: dict[str, str], baseline_covered: dict[str, str], allowlist: set[str] | None, + empty_stems: set[str] | None = None, ) -> FileSelection: """Apply the scope allowlist to the raw files of test binaries and baseline archives. ``allowlist`` is a set of canonical names; ``None`` keeps every file. + ``empty_stems`` are ``/`` stems of baseline archive members + that carry no coverage mapping (see :func:`expand_baseline_archives`); an + allowlisted source with such a stem was compiled and holds no code. """ everything = {**baseline_covered, **test_covered} @@ -325,6 +328,7 @@ def in_scope(name: str) -> bool: # report such a file, not even at 0 %, so the reporter must. unmapped: set[str] = set() declaration_only: set[str] = set() + empty_units: set[str] = set() if allowlist is not None: with_data = set(test_covered.values()) | set(baseline_covered.values()) unmapped = allowlist - with_data @@ -334,6 +338,12 @@ def in_scope(name: str) -> bool: stems_with_data = {_stem(name) for name in with_data} declaration_only = {name for name in unmapped if _is_header(name) and _stem(name) in stems_with_data} unmapped -= declaration_only + # A source whose object sits in a baseline archive without a coverage + # mapping is an empty translation unit (a placeholder .cpp of a + # header-only library): compiled, nothing to cover. + if empty_stems: + empty_units = {name for name in unmapped if not _is_header(name) and _stem(name) in empty_stems} + unmapped -= empty_units return FileSelection( staged=staged, excluded=excluded, @@ -341,7 +351,23 @@ def in_scope(name: str) -> bool: duplicates=duplicates, unmapped=unmapped, declaration_only=declaration_only, + empty_units=empty_units, + ) + + +UNMAPPED_NO_DATA = "no-data" +UNMAPPED_DECLARATION_ONLY = "declaration-only" +UNMAPPED_EMPTY_UNIT = "empty-translation-unit" + + +def format_unmapped_files(selection: FileSelection) -> str: + """``\t`` lines for every in-scope file without coverage data.""" + rows = ( + [(UNMAPPED_NO_DATA, name) for name in selection.unmapped] + + [(UNMAPPED_DECLARATION_ONLY, name) for name in selection.declaration_only] + + [(UNMAPPED_EMPTY_UNIT, name) for name in selection.empty_units] ) + return "".join(f"{category}\t{name}\n" for category, name in sorted(rows)) def resolve_source(runfiles: RunfilesLike, canonical: str, workspace_root: str) -> str | None: @@ -610,9 +636,13 @@ def prepare_sources( sorted_objects: list[str], merged_profdata: str, baseline_objects: list[str], + empty_stems: set[str] | None = None, ) -> tuple[FileSelection, dict[str, str], str, list[str]]: """Apply the scope, stage the in-scope sources and build the exclusion filters. + ``empty_stems`` names the sources whose objects carry no coverage mapping + (see :func:`expand_baseline_archives`). + Returns the file selection, the path map, the staging root llvm-cov reads from, and the ``--ignore-filename-regex`` values. """ @@ -644,7 +674,7 @@ def prepare_sources( baseline_covered = get_covered_files(llvm_bin_path, baseline_objects, None, workspace_root, path_map) print(f"INFO: Baseline archives contain {len(set(baseline_covered.values()))} files.", file=sys.stderr) - selection = select_files(test_covered, baseline_covered, allowlist_set) + selection = select_files(test_covered, baseline_covered, allowlist_set, empty_stems) for name, dropped in sorted(selection.duplicates.items()): print( f"WARNING: {name} is compiled under several paths; reporting one, dropping {sorted(dropped)}", @@ -663,11 +693,11 @@ def prepare_sources( f"listed in text_report/unmapped_files.txt (e.g., {sorted(selection.unmapped)[:5]})", file=sys.stderr, ) - if selection.declaration_only: + if selection.declaration_only or selection.empty_units: print( - f"INFO: {len(selection.declaration_only)} in-scope headers carry no code of their own " - f"(their same-named source file has coverage data); listed in " - f"text_report/declaration_only_headers.txt", + f"INFO: {len(selection.declaration_only)} in-scope headers hold declarations only and " + f"{len(selection.empty_units)} compiled sources hold no code; listed in " + f"text_report/unmapped_files.txt with their category", file=sys.stderr, ) # Stage the in-scope sources under the raw covmap layout so llvm-cov can @@ -882,34 +912,107 @@ def _read_ar_members(path: str) -> list[tuple]: return members -def expand_rlib_archives(objects: list[str], workdir: Path) -> list[str]: - """Replace Rust rlib archives with their extracted object members. +_ELF_MAGIC = b"\x7fELF" + + +def object_has_covmap(data: bytes) -> bool: + """True when ``data`` is an ELF64 object with a ``__llvm_covmap`` section. - llvm-cov rejects rlib archives ("no coverage data found") because of the - leading lib.rmeta member, even though the .o members carry the coverage - mapping. Non-rlib entries (C++ .a archives, executables) pass through - unchanged. + Only the ELF header, the section header table and the section name table + are inspected. Anything that is not an ELF64 object (an rlib's lib.rmeta, + a text member) has no coverage mapping by definition. """ - result = [] - extracted = 0 - for obj in objects: - members = _read_ar_members(obj) if obj.endswith((".a", ".rlib")) else [] - if not any(name == "lib.rmeta" for name, _, _ in members): - result.append(obj) + if len(data) < 64 or data[:4] != _ELF_MAGIC or data[4] != 2: # not ELF, or not ELFCLASS64 + return False + order = "little" if data[5] == 1 else "big" + shoff = int.from_bytes(data[0x28:0x30], order) + shentsize = int.from_bytes(data[0x3A:0x3C], order) + shnum = int.from_bytes(data[0x3C:0x3E], order) + shstrndx = int.from_bytes(data[0x3E:0x40], order) + if shoff == 0 or shentsize < 64 or shstrndx >= shnum: + return False + + def section(index: int) -> tuple[int, int, int] | None: + base = shoff + index * shentsize + header = data[base : base + 64] + if len(header) < 64: + return None + return ( + int.from_bytes(header[0:4], order), # sh_name + int.from_bytes(header[0x18:0x20], order), # sh_offset + int.from_bytes(header[0x20:0x28], order), # sh_size + ) + + strtab = section(shstrndx) + if strtab is None: + return False + names = data[strtab[1] : strtab[1] + strtab[2]] + for index in range(shnum): + entry = section(index) + if entry is None: + continue + end = names.find(b"\x00", entry[0]) + if end != -1 and names[entry[0] : end] == b"__llvm_covmap": + return True + return False + + +def expand_baseline_archives(manifest: dict[str, str], workdir: Path) -> tuple[list[str], set[str]]: + """Give llvm-cov only the baseline archive members that carry a coverage mapping. + + llvm-cov rejects an archive as a whole ("no coverage data found") as soon + as one member has no ``__llvm_covmap`` section: the ``lib.rmeta`` member + of a Rust rlib (exposed as a .a symlink by rules_rust), or the object of + an empty translation unit such as the placeholder .cpp of a header-only + C++ library. Every other file of that library would then lose its 0 % + baseline. Such an archive is replaced by its members that do carry a + mapping, extracted into ``workdir``; archives whose members all carry one + and non-archive objects (executables) pass through unchanged. + + ``manifest`` maps absolute paths to short paths (see + :func:`load_baseline_manifest`). Returns the object list for llvm-cov and + the ``/`` stems of the dropped object members, which identify + the sources compiled without code. + """ + result: list[str] = [] + empty_stems: set[str] = set() + extracted = archives_split = 0 + for path in sorted(manifest): + members = _read_ar_members(path) if path.endswith((".a", ".rlib")) else [] + if not members: + result.append(path) continue - workdir.mkdir(parents=True, exist_ok=True) - with open(obj, "rb") as f: - for index, (name, offset, size) in enumerate(members): - if not name.endswith(".o"): - continue + with open(path, "rb") as f: + usable = [] + for name, offset, size in members: f.seek(offset) - out_path = workdir / f"{Path(obj).stem}.{index}.o" + if object_has_covmap(f.read(size)): + usable.append((name, offset, size)) + elif name.endswith(".o") and not name.endswith(".rcgu.o"): + empty_stems.add(os.path.join(os.path.dirname(manifest[path]), _stem(name))) + if len(usable) == len(members): + result.append(path) + continue + archives_split += 1 + workdir.mkdir(parents=True, exist_ok=True) + for index, (_name, offset, size) in enumerate(usable): + f.seek(offset) + out_path = workdir / f"{Path(path).stem}.{index}.o" out_path.write_bytes(f.read(size)) result.append(str(out_path)) extracted += 1 - if extracted: - print(f"INFO: Expanded {extracted} object(s) from Rust rlib baseline archives.", file=sys.stderr) - return result + if archives_split: + print( + f"INFO: {archives_split} baseline archive(s) had members without a coverage mapping; " + f"passing their {extracted} usable object(s) to llvm-cov individually.", + file=sys.stderr, + ) + return result, empty_stems + + +def expand_rlib_archives(objects: list[str], workdir: Path) -> list[str]: + """Backwards-compatible wrapper of :func:`expand_baseline_archives` for a plain path list.""" + return expand_baseline_archives({path: os.path.basename(path) for path in objects}, workdir)[0] def resolve_tool( @@ -991,41 +1094,39 @@ def load_path_map(runfiles: RunfilesLike, rlocation_path: str | None) -> dict[st return mapping -def load_baseline_objects( - runfiles: RunfilesLike, - rlocation_path: str | None, -) -> list[str]: - """Load baseline object archive paths and resolve them to absolute paths. +def load_baseline_manifest(runfiles: RunfilesLike, rlocation_path: str | None) -> dict[str, str]: + """{absolute path: short path} of the baseline objects listed in the scope's manifest. - The objects manifest lists relative paths to .a files. When the reporter runs - in the exec config, the manifest paths use the exec config dir - (e.g., k8-opt-exec-*). + The manifest lists short_paths of files built by the CONSUMER repository, + which is always the root module ("_main") in a coverage run. A listed + object that cannot be found is a hard error: a silently missing archive + would hide every untested file of that library. """ if not rlocation_path: - return [] - + return {} path = runfiles.Rlocation(rlocation_path) if not path or not Path(path).exists(): print(f"WARNING: Baseline objects manifest not found: {rlocation_path}", file=sys.stderr) - return [] - - lines = Path(path).read_text(encoding="utf-8").splitlines() - resolved = [] - for line in lines: - line = line.strip() - if not line or line.startswith("#"): + return {} + resolved: dict[str, str] = {} + for line in Path(path).read_text(encoding="utf-8").splitlines(): + short_path = line.strip() + if not short_path or short_path.startswith("#"): continue - # The manifest lists short_paths of files built by the CONSUMER - # repository, which is always the root module ("_main") in a coverage - # run. Do NOT use runfiles.CurrentRepository() here: this script lives - # in score_coverage, so that would resolve against the wrong repo. - path = runfiles.Rlocation(os.path.join("_main", line)) - if path and os.path.exists(path): - resolved.append(path) + # Do NOT use runfiles.CurrentRepository() here: this script lives in + # score_coverage, so that would resolve against the wrong repo. + location = runfiles.Rlocation(os.path.join("_main", short_path)) + if location and os.path.exists(location): + resolved[location] = short_path else: - print(f"ERROR: Baseline object not found: {line}", file=sys.stderr) + print(f"ERROR: Baseline object not found: {short_path}", file=sys.stderr) sys.exit(-1) - return sorted(resolved) + return resolved + + +def load_baseline_objects(runfiles: RunfilesLike, rlocation_path: str | None) -> list[str]: + """Absolute paths of the baseline objects, sorted (see :func:`load_baseline_manifest`).""" + return sorted(load_baseline_manifest(runfiles, rlocation_path)) def run_command(cmd: list[str], separate_stderr: bool = False) -> subprocess.CompletedProcess: diff --git a/score_coverage/tests/coverage_summary_test.py b/score_coverage/tests/coverage_summary_test.py index e77b9ab..3af4f04 100644 --- a/score_coverage/tests/coverage_summary_test.py +++ b/score_coverage/tests/coverage_summary_test.py @@ -175,29 +175,42 @@ def test_unmapped_files_row_and_section(self): with tempfile.TemporaryDirectory() as tmp: files = parse_lcov(_write(tmp, "l.dat", LCOV_TWO_FILES)) assert files is not None - md = render_markdown(files, None, ["src/never.h", "src/api/tmpl.h"], ["src/a.h"]) + unmapped = { + "no-data": ["src/never.h", "src/api/tmpl.h"], + "declaration-only": ["src/a.h"], + "empty-translation-unit": ["src/empty.cpp"], + } + md = render_markdown(files, None, unmapped) self.assertIn("| In-scope files without coverage data | 2 | | | |", md) self.assertIn("In-scope files without coverage data (2)", md) self.assertIn("- `src/api/tmpl.h`\n- `src/never.h`", md) self.assertIn("never instantiated", md) self.assertIn("Declaration-only headers (1)", md) self.assertIn("- `src/a.h`", md) - # an empty list: row with 0, no section; None: neither - md = render_markdown(files, None, []) + self.assertIn("Compiled sources without code (1)", md) + self.assertIn("- `src/empty.cpp`", md) + # an empty dict: row with 0, no section; None: neither + md = render_markdown(files, None, {}) self.assertIn("| In-scope files without coverage data | 0 | | | |", md) self.assertNotIn("In-scope files without coverage data", md) md = render_markdown(files, None) self.assertNotIn("In-scope files without coverage data", md) # even with no LCOV records the list is shown - md = render_markdown([], None, ["src/never.h"]) + md = render_markdown([], None, {"no-data": ["src/never.h"]}) self.assertIn("No coverage records", md) self.assertIn("- `src/never.h`", md) def test_load_unmapped_files(self): with tempfile.TemporaryDirectory() as tmp: - listing = _write(tmp, "u.txt", "# comment\nsrc/b.h\n\nsrc/a.h\nsrc/b.h\n") - self.assertEqual(coverage_summary.load_unmapped_files(listing), ["src/a.h", "src/b.h"]) - self.assertEqual(coverage_summary.load_unmapped_files(_write(tmp, "e.txt", "")), []) + listing = _write( + tmp, "u.txt", "# comment\nno-data\tsrc/b.h\n\nsrc/a.h\ndeclaration-only\tsrc/c.h\nno-data\tsrc/b.h\n" + ) + # bare lines are no-data; duplicates collapse; categories kept apart + self.assertEqual( + coverage_summary.load_unmapped_files(listing), + {"no-data": ["src/a.h", "src/b.h"], "declaration-only": ["src/c.h"]}, + ) + self.assertEqual(coverage_summary.load_unmapped_files(_write(tmp, "e.txt", "")), {}) with redirect_stderr(io.StringIO()): self.assertIsNone(coverage_summary.load_unmapped_files(Path(tmp) / "missing.txt")) self.assertIsNone(coverage_summary.load_unmapped_files(None)) diff --git a/score_coverage/tests/generate_coverage_html_test.py b/score_coverage/tests/generate_coverage_html_test.py index 080dd51..25a594a 100644 --- a/score_coverage/tests/generate_coverage_html_test.py +++ b/score_coverage/tests/generate_coverage_html_test.py @@ -65,7 +65,6 @@ def _make_workspace( zf.writestr("text_report/summary.txt", "TOTAL 50%") if unmapped is not None: zf.writestr("text_report/unmapped_files.txt", unmapped) - zf.writestr("text_report/declaration_only_headers.txt", "src/decl.h\n") if with_testlogs: _write(root / "bazel-testlogs" / "pkg" / "some_test" / "test.xml", "") _write(root / "bazel-testlogs" / "pkg" / "some_test" / "test.log", "log") @@ -304,7 +303,8 @@ def test_explicit_summary_md_wins_over_step_summary(self): def test_unmapped_files_are_reported_summarised_and_archived(self): with tempfile.TemporaryDirectory() as tmp: - root = _make_workspace(Path(tmp), unmapped="src/never.h\nsrc/api/tmpl.h\n") + listing = "declaration-only\tsrc/decl.h\nno-data\tsrc/api/tmpl.h\nno-data\tsrc/never.h\n" + root = _make_workspace(Path(tmp), unmapped=listing) rc, _, err = _run( root, ["--summary-md", "summary.md", "--archive-dir", "artifacts_dir", "--testlogs-subdir", "pkg"], @@ -317,10 +317,9 @@ def test_unmapped_files_are_reported_summarised_and_archived(self): self.assertIn("| In-scope files without coverage data | 2 | | | |", summary) self.assertIn("- `src/never.h`", summary) self.assertIn("Declaration-only headers (1)", summary) + self.assertNotIn(" - src/decl.h", err) # not a finding, not printed archived = (root / "artifacts_dir" / "unmapped_files.txt").read_text(encoding="utf-8") - self.assertEqual(archived, "src/never.h\nsrc/api/tmpl.h\n") - decl = (root / "artifacts_dir" / "declaration_only_headers.txt").read_text(encoding="utf-8") - self.assertEqual(decl, "src/decl.h\n") + self.assertEqual(archived, listing) def test_missing_unmapped_list_is_tolerated(self): # Reports produced by an older reporter carry no list: no warning, no row. diff --git a/score_coverage/tests/reporter_test.py b/score_coverage/tests/reporter_test.py index 9ef0679..04d45bf 100644 --- a/score_coverage/tests/reporter_test.py +++ b/score_coverage/tests/reporter_test.py @@ -58,6 +58,64 @@ def _make_archive(members) -> bytes: return blob +def _make_elf(section_names) -> bytes: + """Build a minimal ELF64 relocatable object whose section table names ``section_names``.""" + names = [".shstrtab"] + list(section_names) + strtab = b"\x00" + name_offsets = [] + for name in names: + name_offsets.append(len(strtab)) + strtab += name.encode() + b"\x00" + shnum = len(names) + 1 # + the null section + shoff = 64 + len(strtab) + shoff += (-shoff) % 8 + header = ( + b"\x7fELF" + + bytes([2, 1, 1, 0]) # ELFCLASS64, little endian, version 1, SysV ABI + + b"\x00" * 8 + + (1).to_bytes(2, "little") # ET_REL + + (0x3E).to_bytes(2, "little") # x86-64 + + (1).to_bytes(4, "little") + + (0).to_bytes(8, "little") # e_entry + + (0).to_bytes(8, "little") # e_phoff + + shoff.to_bytes(8, "little") + + (0).to_bytes(4, "little") # e_flags + + (64).to_bytes(2, "little") # e_ehsize + + (0).to_bytes(2, "little") # e_phentsize + + (0).to_bytes(2, "little") # e_phnum + + (64).to_bytes(2, "little") # e_shentsize + + shnum.to_bytes(2, "little") + + (1).to_bytes(2, "little") # e_shstrndx + ) + assert len(header) == 64 + blob = header + strtab + blob += b"\x00" * (shoff - len(blob)) + + def shdr(name_off, offset, size): + return ( + name_off.to_bytes(4, "little") + + (1).to_bytes(4, "little") # SHT_PROGBITS + + (0).to_bytes(8, "little") + + (0).to_bytes(8, "little") + + offset.to_bytes(8, "little") + + size.to_bytes(8, "little") + + (0).to_bytes(4, "little") + + (0).to_bytes(4, "little") + + (1).to_bytes(8, "little") + + (0).to_bytes(8, "little") + ) + + blob += b"\x00" * 64 # null section + blob += shdr(name_offsets[0], 64, len(strtab)) # .shstrtab + for name_off in name_offsets[1:]: + blob += shdr(name_off, 0, 0) + return blob + + +COVMAP_OBJ = _make_elf([".text", "__llvm_covmap", "__llvm_covfun"]) +PLAIN_OBJ = _make_elf([".text", "__llvm_covfun"]) + + @verifies("tool_req__coverage_report_rlib_expansion") class ReadArMembersTest(unittest.TestCase): def test_non_archive_returns_empty(self): @@ -85,33 +143,82 @@ def test_gnu_long_name_table_is_resolved(self): @verifies("tool_req__coverage_report_rlib_expansion") -class ExpandRlibArchivesTest(unittest.TestCase): +class ObjectHasCovmapTest(unittest.TestCase): + def test_detects_the_section_by_name(self): + self.assertTrue(reporter.object_has_covmap(COVMAP_OBJ)) + self.assertFalse(reporter.object_has_covmap(PLAIN_OBJ)) + self.assertFalse(reporter.object_has_covmap(_make_elf([".text", "__llvm_covmap_not"]))) + + def test_non_elf_and_truncated_input(self): + self.assertFalse(reporter.object_has_covmap(b"META")) + self.assertFalse(reporter.object_has_covmap(b"")) + self.assertFalse(reporter.object_has_covmap(b"\x7fELF" + b"\x01" * 60)) # ELFCLASS32 + self.assertFalse(reporter.object_has_covmap(COVMAP_OBJ[:100])) # section table cut off + + +@verifies("tool_req__coverage_report_rlib_expansion", "tool_req__coverage_report_baseline_zero") +class ExpandBaselineArchivesTest(unittest.TestCase): + """llvm-cov gets only archive members that carry a coverage mapping.""" + + def _expand(self, tmp, name, members, short_path=None): + archive = Path(tmp) / name + archive.write_bytes(_make_archive(members)) + manifest = {str(archive): short_path or f"pkg/{name}"} + return reporter.expand_baseline_archives(manifest, Path(tmp) / "extracted"), archive + def test_rlib_is_expanded_to_object_members(self): - """Archives with a lib.rmeta member are replaced by their .o members.""" - blob = _make_archive([("lib.rmeta/", b"META"), ("crate.o/", b"OBJ1"), ("notes.txt/", b"TXT")]) + """lib.rmeta has no mapping, so the rlib is replaced by its .o members.""" with tempfile.TemporaryDirectory() as tmp: - rlib = Path(tmp) / "libcrate.a" - rlib.write_bytes(blob) - workdir = Path(tmp) / "extracted" - result = expand_rlib_archives([str(rlib)], workdir) + (result, empty), _ = self._expand( + tmp, "libcrate.a", [("lib.rmeta/", b"META"), ("crate.rcgu.o/", COVMAP_OBJ), ("notes.txt/", b"TXT")] + ) self.assertEqual(len(result), 1) self.assertTrue(result[0].endswith(".o")) - self.assertEqual(Path(result[0]).read_bytes(), b"OBJ1") + self.assertEqual(Path(result[0]).read_bytes(), COVMAP_OBJ) + self.assertEqual(empty, set()) # rlib codegen units are not sources def test_plain_cc_archive_passes_through(self): - blob = _make_archive([("mylib.o/", b"OBJ1")]) with tempfile.TemporaryDirectory() as tmp: - archive = Path(tmp) / "libcc.a" - archive.write_bytes(blob) - result = expand_rlib_archives([str(archive)], Path(tmp) / "x") + (result, empty), archive = self._expand( + tmp, "libcc.a", [("mylib.o/", COVMAP_OBJ), ("other.o/", COVMAP_OBJ)] + ) self.assertEqual(result, [str(archive)]) + self.assertEqual(empty, set()) + + def test_member_without_mapping_is_dropped_and_the_rest_kept(self): + """The empty placeholder object would make llvm-cov reject the whole archive.""" + with tempfile.TemporaryDirectory() as tmp: + err = io.StringIO() + with redirect_stderr(err): + (result, empty), _ = self._expand( + tmp, + "libuncovered.a", + [("empty_unit.o/", PLAIN_OBJ), ("uncovered.o/", COVMAP_OBJ)], + short_path="src/libuncovered.a", + ) + self.assertEqual(len(result), 1) + self.assertEqual(Path(result[0]).read_bytes(), COVMAP_OBJ) + self.assertEqual(empty, {"src/empty_unit"}) + self.assertIn("1 baseline archive(s) had members without a coverage mapping", err.getvalue()) + + def test_archive_without_any_mapping_contributes_nothing(self): + with tempfile.TemporaryDirectory() as tmp: + with redirect_stderr(io.StringIO()): + (result, empty), _ = self._expand( + tmp, "libexecutor.a", [("executor.o/", PLAIN_OBJ), ("task.o/", PLAIN_OBJ)], "score/c/libexecutor.a" + ) + self.assertEqual(result, []) + self.assertEqual(empty, {"score/c/executor", "score/c/task"}) def test_executable_passes_through(self): with tempfile.TemporaryDirectory() as tmp: binary = Path(tmp) / "my_tool" binary.write_bytes(b"\x7fELF" + b"\x00" * 12) - result = expand_rlib_archives([str(binary)], Path(tmp) / "x") + result, empty = reporter.expand_baseline_archives({str(binary): "pkg/my_tool"}, Path(tmp) / "x") self.assertEqual(result, [str(binary)]) + self.assertEqual(empty, set()) + # the plain-list wrapper keeps working + self.assertEqual(expand_rlib_archives([str(binary)], Path(tmp) / "y"), [str(binary)]) @verifies("tool_req__coverage_report_baseline_zero") @@ -600,9 +707,8 @@ def test_full_report(self): index = zf.read("html_report/index.html").decode() self.assertIn("html_report/index.html", names) self.assertIn("html_report/style.css", names) - # Every in-scope file has data here, so the unmapped lists exist and are empty. + # Every in-scope file has data here, so the unmapped list exists and is empty. self.assertIn("text_report/unmapped_files.txt", names) - self.assertIn("text_report/declaration_only_headers.txt", names) # Pages are filed under canonical paths: no machine-specific directory # remains, the index links there, and the page's asset links still resolve. self.assertNotIn("html_report/coverage/" + str(self.workdir).strip("/") + "/", "\n".join(names)) @@ -665,11 +771,13 @@ def test_in_scope_file_without_coverage_data_is_listed(self): with redirect_stderr(err): reporter.main(self._argv()) with zipfile.ZipFile(self.output) as zf: - self.assertEqual(zf.read("text_report/unmapped_files.txt").decode(), "src/never.h\n") - self.assertEqual(zf.read("text_report/declaration_only_headers.txt").decode(), "src/a.h\n") + self.assertEqual( + zf.read("text_report/unmapped_files.txt").decode(), + "declaration-only\tsrc/a.h\nno-data\tsrc/never.h\n", + ) self.assertIn("1 in-scope files have no coverage data at all", err.getvalue()) self.assertIn("src/never.h", err.getvalue()) - self.assertIn("1 in-scope headers carry no code of their own", err.getvalue()) + self.assertIn("1 in-scope headers hold declarations only", err.getvalue()) def test_no_reports_writes_empty_zip(self): self.reports_file.write_text("", encoding="utf-8") @@ -788,12 +896,28 @@ def test_allowlisted_file_without_any_coverage_data_is_reported(self): # mapping anywhere; llvm-cov cannot show it, so the selection must. test = {"src/a.cpp": "src/a.cpp"} baseline = {"src/a.cpp": "src/a.cpp", "src/b.cpp": "src/b.cpp"} - allowlist = {"src/a.cpp", "src/a.h", "src/b.cpp", "src/b.hpp", "src/never.h", "src/tmpl.h", "src/orphan.cpp"} - sel = reporter.select_files(test, baseline, allowlist) + allowlist = { + "src/a.cpp", + "src/a.h", + "src/b.cpp", + "src/b.hpp", + "src/never.h", + "src/tmpl.h", + "src/orphan.cpp", + "src/empty.cpp", + } # a.h / b.hpp sit next to compiled a.cpp / b.cpp: declarations only. - # never.h, tmpl.h and a source file nobody built are the real findings. + # empty.cpp was compiled (its object is an archive member) but has no + # mapping: no code. never.h, tmpl.h and a source nobody built remain. + sel = reporter.select_files(test, baseline, allowlist, empty_stems={"src/empty"}) self.assertEqual(sel.unmapped, {"src/never.h", "src/tmpl.h", "src/orphan.cpp"}) self.assertEqual(sel.declaration_only, {"src/a.h", "src/b.hpp"}) + self.assertEqual(sel.empty_units, {"src/empty.cpp"}) + self.assertEqual( + reporter.format_unmapped_files(sel), + "declaration-only\tsrc/a.h\ndeclaration-only\tsrc/b.hpp\nempty-translation-unit\tsrc/empty.cpp\n" + "no-data\tsrc/never.h\nno-data\tsrc/orphan.cpp\nno-data\tsrc/tmpl.h\n", + ) self.assertEqual(sel.baseline_only, {"src/b.cpp"}) self.assertEqual(sel.excluded, set()) From 09531dc3f408e44bac2228e72e4b58802aaca040 Mon Sep 17 00:00:00 2001 From: Dan Calavrezo <195309321+dcalavrezo-qorix@users.noreply.github.com> Date: Tue, 15 Sep 2026 17:29:04 +0300 Subject: [PATCH 4/6] Classify compiled placeholder sources by archive membership On baselibs the placeholder .cpp of a header-only library usually does carry a coverage mapping, for the inline functions of the headers it includes, while the file itself has no regions. Deciding "compiled but no code of its own" by the absence of a mapping therefore caught 37 of 82 such files and left the rest among the findings. The category is now decided by the object being a member of a baseline archive at all, and renamed compiled-without-code; the mapping probe is kept for what it is needed for, handing llvm-cov only loadable members. The archive reader also skips the GNU symbol table member, which was counted as a member without mapping and made every archive look mixed. baselibs, whole tree: 531 files reported, all index links valid, baseline 446 files (431 before the archive split), findings 243, of which 172 are template headers of score/language/futurecpp. Signed-off-by: Dan Calavrezo <195309321+dcalavrezo-qorix@users.noreply.github.com> --- docs/manual/known_problems.rst | 6 +-- docs/requirements/tool_requirements.rst | 10 ++-- integration_tests/expected_lcov.dat | 4 +- integration_tests/run_integration_test.sh | 4 +- integration_tests/src/empty_unit.cpp | 4 +- score_coverage/coverage_summary.py | 9 ++-- score_coverage/reporter.py | 51 ++++++++++--------- score_coverage/tests/coverage_summary_test.py | 4 +- score_coverage/tests/reporter_test.py | 20 ++++---- 9 files changed, 60 insertions(+), 52 deletions(-) diff --git a/docs/manual/known_problems.rst b/docs/manual/known_problems.rst index 7217673..d793213 100644 --- a/docs/manual/known_problems.rst +++ b/docs/manual/known_problems.rst @@ -79,9 +79,9 @@ stay listed with their upstream references. - The file is named in the job summary under "In-scope files without coverage data", in ``unmapped_files.txt`` of the archive and in a reporter ``WARNING``. Headers whose same-named source file has data - (declaration-only) and placeholder sources compiled into an archive - without code are listed in the same file under their own category and - are not findings. + (declaration-only) and placeholder sources that were compiled but hold + no code of their own are listed in the same file under their own + category and are not findings. - Decide per file: write a test that instantiates it (it is shipped API), or remove it from the target's ``hdrs`` (it is not needed). * - **A source could not be staged for llvm-cov.** The reporter reads the diff --git a/docs/requirements/tool_requirements.rst b/docs/requirements/tool_requirements.rst index f54caaa..b763653 100644 --- a/docs/requirements/tool_requirements.rst +++ b/docs/requirements/tool_requirements.rst @@ -184,9 +184,9 @@ Report ``text_report/unmapped_files.txt`` (always present, empty when there are none) as ``\t``, sorted, with one of three categories: ``declaration-only`` for a header whose same-named source file (same path - without extension) has coverage data, ``empty-translation-unit`` for a - source whose object is a member of a baseline archive, and ``no-data`` for - everything else. The reporter shall emit a warning naming the ``no-data`` + without extension) has coverage data, ``compiled-without-code`` for a + source whose object is a member of a baseline archive (it was compiled and + holds no code of its own), and ``no-data`` for everything else. The reporter shall emit a warning naming the ``no-data`` files; ``generate_coverage_html`` shall print them, copy the list into the archive as ``unmapped_files.txt`` and show the ``no-data`` count in the job summary table with one collapsible section per category. None of the @@ -206,8 +206,8 @@ Report header-only library). Before passing baseline archives to ``llvm-cov``, the reporter shall inspect every member's ELF section table and replace such an archive by its members that carry a mapping, so that no library loses - its zero-coverage baseline because of one member; the dropped object - members shall identify the sources reported as compiled without code + its zero-coverage baseline because of one member. The object members of + the archives identify the sources that were compiled (:need:`tool_req__coverage_report_unmapped`). .. tool_req:: A missing baseline object is an error diff --git a/integration_tests/expected_lcov.dat b/integration_tests/expected_lcov.dat index 8b85446..589179f 100644 --- a/integration_tests/expected_lcov.dat +++ b/integration_tests/expected_lcov.dat @@ -47,8 +47,8 @@ # mapping for it. Not in this file; reported by the pipeline # as an in-scope file without coverage data instead. # (absent) src/empty_unit.cpp: placeholder translation unit of the -# uncovered library, compiled into its archive without a -# coverage mapping; reported as compiled source without code. +# uncovered library, compiled into its archive but without +# code of its own; reported as compiled source without code. # (absent) external/itest_external+/extlib.cpp and extlib.h: compiled # with coverage and executed by coverable_test, but reached # only through the forwarding target //third_party:extlib, so diff --git a/integration_tests/run_integration_test.sh b/integration_tests/run_integration_test.sh index 2f143fe..9ad99e8 100755 --- a/integration_tests/run_integration_test.sh +++ b/integration_tests/run_integration_test.sh @@ -69,7 +69,7 @@ for marker in "## Coverage summary" "| Lines |" "Raw vs effective" \ "Coverage by directory" "Files at exact 0% (2)" \ "| In-scope files without coverage data | 1 |" \ "In-scope files without coverage data (1)" '- `src/unused_api.h`' \ - "Declaration-only headers (2)" "Compiled sources without code (1)" '- `src/empty_unit.cpp`'; do + "Declaration-only headers (2)" "Compiled sources without code of their own (1)" '- `src/empty_unit.cpp`'; do if ! grep -qF -- "${marker}" summary.md; then echo "ERROR: '${marker}' missing from summary.md" >&2 exit 1 @@ -110,7 +110,7 @@ for f in artifacts_dir/coverage_linux/index.html artifacts_dir/coverage_report.d done # unused_api.h is the finding; coverable.h / uncovered.h hold declarations for # compiled .cpp files and empty_unit.cpp is a compiled placeholder: categorised. -EXPECTED_UNMAPPED=$'declaration-only\tsrc/coverable.h\ndeclaration-only\tsrc/uncovered.h\nempty-translation-unit\tsrc/empty_unit.cpp\nno-data\tsrc/unused_api.h' +EXPECTED_UNMAPPED=$'compiled-without-code\tsrc/empty_unit.cpp\ndeclaration-only\tsrc/coverable.h\ndeclaration-only\tsrc/uncovered.h\nno-data\tsrc/unused_api.h' if [[ "$(cat artifacts_dir/unmapped_files.txt)" != "${EXPECTED_UNMAPPED}" ]]; then echo "ERROR: unmapped_files.txt unexpected:" >&2 cat artifacts_dir/unmapped_files.txt >&2 diff --git a/integration_tests/src/empty_unit.cpp b/integration_tests/src/empty_unit.cpp index 2eacd4f..59a70f4 100644 --- a/integration_tests/src/empty_unit.cpp +++ b/integration_tests/src/empty_unit.cpp @@ -11,6 +11,6 @@ * SPDX-License-Identifier: Apache-2.0 ********************************************************************************/ // Placeholder translation unit of a header-only library: compiled into the -// archive, but it produces no coverage mapping. Reported as "compiled source -// without code", not as a finding. +// archive, but it holds no code of its own. Reported as "compiled source +// without code of its own", not as a finding. #include "src/uncovered.h" diff --git a/score_coverage/coverage_summary.py b/score_coverage/coverage_summary.py index 48e1351..786ad2c 100644 --- a/score_coverage/coverage_summary.py +++ b/score_coverage/coverage_summary.py @@ -252,10 +252,11 @@ def rollup_by_directory(files: list[FileCoverage]) -> list[dict]: "declarations only. Listed for completeness, no action expected.", ), ( - "empty-translation-unit", - "Compiled sources without code", - "Sources compiled into a library archive that produced no coverage mapping, typically " - "the placeholder .cpp of a header-only library. Nothing to cover.", + "compiled-without-code", + "Compiled sources without code of their own", + "Sources compiled into a library archive that contain no code themselves, typically the " + "placeholder .cpp of a header-only library that only includes headers. Nothing to cover " + "in these files.", ), ) diff --git a/score_coverage/reporter.py b/score_coverage/reporter.py index fa950a3..a4fa993 100644 --- a/score_coverage/reporter.py +++ b/score_coverage/reporter.py @@ -101,10 +101,10 @@ def main(argv: list[str] | None = None) -> None: # a coverage mapping (the lib.rmeta of a Rust rlib, the object of an empty # translation unit), so such archives are replaced by their usable members. baseline_manifest = load_baseline_manifest(r, args.baseline_objects) - baseline_objects, empty_stems = expand_baseline_archives(baseline_manifest, Path.cwd() / "baseline_objects") + baseline_objects, compiled_stems = expand_baseline_archives(baseline_manifest, Path.cwd() / "baseline_objects") selection, path_map, root, regexes = prepare_sources( - r, args, llvm_bin_path, sorted_objects, str(merged_profdata), baseline_objects, empty_stems + r, args, llvm_bin_path, sorted_objects, str(merged_profdata), baseline_objects, compiled_stems ) # All valid baseline archives are passed when baseline-only files exist; # _filter_lcov keeps only those files' records. @@ -283,7 +283,7 @@ class FileSelection: declaration_only: set[str] = field(default_factory=set) """unmapped headers whose same-named source file has coverage data (declarations only).""" empty_units: set[str] = field(default_factory=set) - """unmapped sources that were compiled into a baseline archive but produced no coverage mapping.""" + """unmapped sources that were compiled into a baseline archive: no code of their own.""" _HEADER_SUFFIXES = (".h", ".hpp", ".hh", ".hxx", ".inl", ".ipp", ".tpp") @@ -302,14 +302,15 @@ def select_files( test_covered: dict[str, str], baseline_covered: dict[str, str], allowlist: set[str] | None, - empty_stems: set[str] | None = None, + compiled_stems: set[str] | None = None, ) -> FileSelection: """Apply the scope allowlist to the raw files of test binaries and baseline archives. ``allowlist`` is a set of canonical names; ``None`` keeps every file. - ``empty_stems`` are ``/`` stems of baseline archive members - that carry no coverage mapping (see :func:`expand_baseline_archives`); an - allowlisted source with such a stem was compiled and holds no code. + ``compiled_stems`` are ``/`` stems of the baseline archives' + object members (see :func:`expand_baseline_archives`); an allowlisted + source with such a stem but no coverage data was compiled and holds no + code of its own. """ everything = {**baseline_covered, **test_covered} @@ -338,11 +339,11 @@ def in_scope(name: str) -> bool: stems_with_data = {_stem(name) for name in with_data} declaration_only = {name for name in unmapped if _is_header(name) and _stem(name) in stems_with_data} unmapped -= declaration_only - # A source whose object sits in a baseline archive without a coverage - # mapping is an empty translation unit (a placeholder .cpp of a - # header-only library): compiled, nothing to cover. - if empty_stems: - empty_units = {name for name in unmapped if not _is_header(name) and _stem(name) in empty_stems} + # A source whose object sits in a baseline archive but that has no + # coverage data of its own is a placeholder translation unit of a + # header-only library: compiled, nothing to cover in that file. + if compiled_stems: + empty_units = {name for name in unmapped if not _is_header(name) and _stem(name) in compiled_stems} unmapped -= empty_units return FileSelection( staged=staged, @@ -357,7 +358,7 @@ def in_scope(name: str) -> bool: UNMAPPED_NO_DATA = "no-data" UNMAPPED_DECLARATION_ONLY = "declaration-only" -UNMAPPED_EMPTY_UNIT = "empty-translation-unit" +UNMAPPED_EMPTY_UNIT = "compiled-without-code" def format_unmapped_files(selection: FileSelection) -> str: @@ -636,11 +637,11 @@ def prepare_sources( sorted_objects: list[str], merged_profdata: str, baseline_objects: list[str], - empty_stems: set[str] | None = None, + compiled_stems: set[str] | None = None, ) -> tuple[FileSelection, dict[str, str], str, list[str]]: """Apply the scope, stage the in-scope sources and build the exclusion filters. - ``empty_stems`` names the sources whose objects carry no coverage mapping + ``compiled_stems`` names the sources compiled into a baseline archive (see :func:`expand_baseline_archives`). Returns the file selection, the path map, the staging root llvm-cov reads @@ -674,7 +675,7 @@ def prepare_sources( baseline_covered = get_covered_files(llvm_bin_path, baseline_objects, None, workspace_root, path_map) print(f"INFO: Baseline archives contain {len(set(baseline_covered.values()))} files.", file=sys.stderr) - selection = select_files(test_covered, baseline_covered, allowlist_set, empty_stems) + selection = select_files(test_covered, baseline_covered, allowlist_set, compiled_stems) for name, dropped in sorted(selection.duplicates.items()): print( f"WARNING: {name} is compiled under several paths; reporting one, dropping {sorted(dropped)}", @@ -905,7 +906,8 @@ def _read_ar_members(path: str) -> list[tuple]: name = longnames[start:end].decode(errors="replace").rstrip("/") elif name.endswith("/"): name = name[:-1] - members.append((name, data_offset, size)) + if name not in ("", "/SYM64", "__.SYMDEF", "__.SYMDEF SORTED"): # skip symbol tables + members.append((name, data_offset, size)) f.seek(size, 1) if size % 2 == 1: f.seek(1, 1) @@ -971,11 +973,14 @@ def expand_baseline_archives(manifest: dict[str, str], workdir: Path) -> tuple[l ``manifest`` maps absolute paths to short paths (see :func:`load_baseline_manifest`). Returns the object list for llvm-cov and - the ``/`` stems of the dropped object members, which identify - the sources compiled without code. + the ``/`` stems of every C/C++ object member + (``score/concurrency/libexecutor.a`` with ``executor.o`` gives + ``score/concurrency/executor``): the sources that were compiled. An + allowlisted source with such a stem and no coverage data holds no code of + its own, whether or not its object carries a mapping for included headers. """ result: list[str] = [] - empty_stems: set[str] = set() + compiled_stems: set[str] = set() extracted = archives_split = 0 for path in sorted(manifest): members = _read_ar_members(path) if path.endswith((".a", ".rlib")) else [] @@ -985,11 +990,11 @@ def expand_baseline_archives(manifest: dict[str, str], workdir: Path) -> tuple[l with open(path, "rb") as f: usable = [] for name, offset, size in members: + if name.endswith(".o") and not name.endswith(".rcgu.o"): + compiled_stems.add(os.path.join(os.path.dirname(manifest[path]), _stem(name))) f.seek(offset) if object_has_covmap(f.read(size)): usable.append((name, offset, size)) - elif name.endswith(".o") and not name.endswith(".rcgu.o"): - empty_stems.add(os.path.join(os.path.dirname(manifest[path]), _stem(name))) if len(usable) == len(members): result.append(path) continue @@ -1007,7 +1012,7 @@ def expand_baseline_archives(manifest: dict[str, str], workdir: Path) -> tuple[l f"passing their {extracted} usable object(s) to llvm-cov individually.", file=sys.stderr, ) - return result, empty_stems + return result, compiled_stems def expand_rlib_archives(objects: list[str], workdir: Path) -> list[str]: diff --git a/score_coverage/tests/coverage_summary_test.py b/score_coverage/tests/coverage_summary_test.py index 3af4f04..3553976 100644 --- a/score_coverage/tests/coverage_summary_test.py +++ b/score_coverage/tests/coverage_summary_test.py @@ -178,7 +178,7 @@ def test_unmapped_files_row_and_section(self): unmapped = { "no-data": ["src/never.h", "src/api/tmpl.h"], "declaration-only": ["src/a.h"], - "empty-translation-unit": ["src/empty.cpp"], + "compiled-without-code": ["src/empty.cpp"], } md = render_markdown(files, None, unmapped) self.assertIn("| In-scope files without coverage data | 2 | | | |", md) @@ -187,7 +187,7 @@ def test_unmapped_files_row_and_section(self): self.assertIn("never instantiated", md) self.assertIn("Declaration-only headers (1)", md) self.assertIn("- `src/a.h`", md) - self.assertIn("Compiled sources without code (1)", md) + self.assertIn("Compiled sources without code of their own (1)", md) self.assertIn("- `src/empty.cpp`", md) # an empty dict: row with 0, no section; None: neither md = render_markdown(files, None, {}) diff --git a/score_coverage/tests/reporter_test.py b/score_coverage/tests/reporter_test.py index 04d45bf..9ad5ed6 100644 --- a/score_coverage/tests/reporter_test.py +++ b/score_coverage/tests/reporter_test.py @@ -125,7 +125,8 @@ def test_non_archive_returns_empty(self): self.assertEqual(_read_ar_members(f.name), []) def test_members_are_listed_with_sizes(self): - blob = _make_archive([("lib.rmeta/", b"META"), ("foo.o/", b"OBJDATA")]) + # "/" is the GNU symbol table, not a member + blob = _make_archive([("/", b"SYMS"), ("lib.rmeta/", b"META"), ("foo.o/", b"OBJDATA")]) with tempfile.NamedTemporaryFile(suffix=".a") as f: f.write(blob) f.flush() @@ -179,11 +180,11 @@ def test_rlib_is_expanded_to_object_members(self): def test_plain_cc_archive_passes_through(self): with tempfile.TemporaryDirectory() as tmp: - (result, empty), archive = self._expand( - tmp, "libcc.a", [("mylib.o/", COVMAP_OBJ), ("other.o/", COVMAP_OBJ)] + (result, compiled), archive = self._expand( + tmp, "libcc.a", [("/", b"SYMS"), ("mylib.o/", COVMAP_OBJ), ("other.o/", COVMAP_OBJ)] ) - self.assertEqual(result, [str(archive)]) - self.assertEqual(empty, set()) + self.assertEqual(result, [str(archive)]) # the symbol table is not a member without mapping + self.assertEqual(compiled, {"pkg/mylib", "pkg/other"}) def test_member_without_mapping_is_dropped_and_the_rest_kept(self): """The empty placeholder object would make llvm-cov reject the whole archive.""" @@ -198,7 +199,7 @@ def test_member_without_mapping_is_dropped_and_the_rest_kept(self): ) self.assertEqual(len(result), 1) self.assertEqual(Path(result[0]).read_bytes(), COVMAP_OBJ) - self.assertEqual(empty, {"src/empty_unit"}) + self.assertEqual(empty, {"src/empty_unit", "src/uncovered"}) # both were compiled self.assertIn("1 baseline archive(s) had members without a coverage mapping", err.getvalue()) def test_archive_without_any_mapping_contributes_nothing(self): @@ -908,14 +909,15 @@ def test_allowlisted_file_without_any_coverage_data_is_reported(self): } # a.h / b.hpp sit next to compiled a.cpp / b.cpp: declarations only. # empty.cpp was compiled (its object is an archive member) but has no - # mapping: no code. never.h, tmpl.h and a source nobody built remain. - sel = reporter.select_files(test, baseline, allowlist, empty_stems={"src/empty"}) + # data of its own: no code. never.h, tmpl.h and a source nobody built + # (orphan.cpp: no object anywhere) remain findings. + sel = reporter.select_files(test, baseline, allowlist, compiled_stems={"src/a", "src/b", "src/empty"}) self.assertEqual(sel.unmapped, {"src/never.h", "src/tmpl.h", "src/orphan.cpp"}) self.assertEqual(sel.declaration_only, {"src/a.h", "src/b.hpp"}) self.assertEqual(sel.empty_units, {"src/empty.cpp"}) self.assertEqual( reporter.format_unmapped_files(sel), - "declaration-only\tsrc/a.h\ndeclaration-only\tsrc/b.hpp\nempty-translation-unit\tsrc/empty.cpp\n" + "compiled-without-code\tsrc/empty.cpp\ndeclaration-only\tsrc/a.h\ndeclaration-only\tsrc/b.hpp\n" "no-data\tsrc/never.h\nno-data\tsrc/orphan.cpp\nno-data\tsrc/tmpl.h\n", ) self.assertEqual(sel.baseline_only, {"src/b.cpp"}) From 2cd93d38cc862ed1fbe6fcd6b495a34359422a69 Mon Sep 17 00:00:00 2001 From: Dan Calavrezo <195309321+dcalavrezo-qorix@users.noreply.github.com> Date: Wed, 16 Sep 2026 10:58:31 +0300 Subject: [PATCH 5/6] Release notes 0.2.0 in plain language, explain unmapped_files.txt Lead with what changed as a reader without C++ background would read it, then explain the new unmapped_files.txt and its three categories (no-data, declaration-only, compiled-without-code) with what each means and what to do. The technical bullets stay under a separate heading for integrators. Signed-off-by: Dan Calavrezo <195309321+dcalavrezo-qorix@users.noreply.github.com> --- docs/release/release_notes.rst | 79 ++++++++++++++++++++++++++++++++++ 1 file changed, 79 insertions(+) diff --git a/docs/release/release_notes.rst b/docs/release/release_notes.rst index 0365fcb..3096537 100644 --- a/docs/release/release_notes.rst +++ b/docs/release/release_notes.rst @@ -26,6 +26,85 @@ Release notes 0.2.0 (unreleased) ------------------ +In plain words +~~~~~~~~~~~~~~ + +This release fixes what the first baselibs reports showed, and adds one new +piece of information to every report. + +**Third-party code no longer leaks into a module's report.** A module that +wraps a third-party library (baselibs wraps OpenSSL this way) got that +library's 72 header files counted as its own code, at 0 %. They are gone. +Only files a module lists itself in its build targets are part of its +report. + +**Every link in the HTML report opens.** Some rows of the report pointed at +pages that had never been generated: files that Bazel generates during the +build and files from other repositories were not readable at the moment the +report was produced. The report now brings every source file along, so every +row opens. File names in the report are the paths of the source files, no +longer build-internal paths such as ``bazel-out/.../_virtual_includes/...``. +If a justification was written against such a build-internal path, it needs +the source path now. + +**Untested files that the old report could not show are listed.** This is +the new piece. A coverage tool can only measure files that were compiled into +a test or a library. A file that nothing compiles has no lines to count, and +llvm-cov cannot show it, not even at 0 %. Until now such files were simply +absent from the report and nobody noticed. The report now lists them, +see :ref:`unmapped_files` below. + +**More untested files show their 0 %.** A library archive with one object that +contains no code (typical for header-only libraries, see below) was rejected +by llvm-cov as a whole, so the other files of that library lost their 0 % +entries. This is fixed; in baselibs 15 files reappeared. + +.. _unmapped_files: + +The file ``unmapped_files.txt`` +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +The archive of every coverage run now contains ``unmapped_files.txt``, and the +job summary shows the same information as a table row and three collapsible +sections. It lists every file that belongs to the module's coverage scope but +for which no coverage data exists anywhere: no test and no library contains +compiled code from it. Such a file counts in no percentage. Each line reads +```` TAB ````; there are three categories. + +.. list-table:: + :header-rows: 1 + :widths: 22 48 30 + + * - Category + - What it means + - What to do + * - ``no-data`` + - **The findings.** Nothing in the module compiles this file: no source + includes the header, or it holds only templates that no test ever + uses with a concrete type. For a public API this means no test + exercises it. It also catches headers that contain nothing executable + (forward declarations, type traits, test mocks); the tool cannot tell + those apart from unused API. + - Look at each file: write a test that uses it, remove it if nobody + needs it, or note that it holds nothing testable. + * - ``declaration-only`` + - A header that only announces functions. The code behind it lives in a + source file with the same name (``timerfd.h`` next to ``timerfd.cpp``), + and that source file is measured. + - Nothing. Listed for completeness. + * - ``compiled-without-code`` + - A source file that Bazel compiled but that contains no code of its + own, only ``#include`` lines. Header-only libraries carry such a + placeholder file so that Bazel produces a library archive. + - Nothing. Listed for completeness. + +Two things this list is not: it is not part of the coverage percentage, and +it does not say *why* a ``no-data`` file was never compiled. That needs a +look at the file. + +Details for integrators +~~~~~~~~~~~~~~~~~~~~~~~ + - Fixed (eclipse-score/coverage_tool#5): a workspace rule that forwards the ``CcInfo`` of a third-party library (e.g. a transition wrapper around OpenSSL) no longer puts that library's headers into the scope; only headers From 7c25ea017cfae904033a0b887b21a319b419c8e6 Mon Sep 17 00:00:00 2001 From: Dan Calavrezo <195309321+dcalavrezo-qorix@users.noreply.github.com> Date: Wed, 16 Sep 2026 11:28:49 +0300 Subject: [PATCH 6/6] Attribute virtual-include paths of out-of-scope targets to the header baselibs' futurecpp tests compile the library's headers through a test-only twin target (futurecpp_internal: same hdrs behind strip_include_prefix, test copts). The coverage mapping therefore names bazel-out/.../_virtual_includes/futurecpp_internal/score/.hpp. The scope's path map only knows trees of targets the aspect visited, so the reporter took these for unknown files, excluded them as out of scope, and then listed 172 futurecpp headers as having no coverage data, although 68 tests exercise them. The reporter now resolves such a name by its path below the tree: the single allowlisted file that ends with it (shorter tails are tried for an include_prefix). A tail matching several in-scope files is left unresolved and warned about. The resolved names go into the path map, so LCOV, HTML and summary use the declared header path. Tests: resolver (unique, ambiguous, include_prefix, no match), reporter main with a foreign tree and no path map entry. The integration workspace's test now compiles the vendored header through a test-only twin target, which reproduces the futurecpp pattern: without the fix the header vanishes from the golden LCOV. Docs: tool_req__coverage_report_relative_paths, known problems, release notes (plain words and integrator details), inventory. Signed-off-by: Dan Calavrezo <195309321+dcalavrezo-qorix@users.noreply.github.com> --- docs/manual/known_problems.rst | 7 +++ docs/release/release_notes.rst | 10 ++++ docs/requirements/tool_requirements.rst | 5 +- docs/verification/verification_report.rst | 6 +- integration_tests/expected_lcov.dat | 10 ++-- integration_tests/run_integration_test.sh | 9 +++ integration_tests/src/BUILD | 14 ++++- score_coverage/reporter.py | 73 +++++++++++++++++++++++ score_coverage/tests/reporter_test.py | 73 +++++++++++++++++++++++ 9 files changed, 198 insertions(+), 9 deletions(-) diff --git a/docs/manual/known_problems.rst b/docs/manual/known_problems.rst index d793213..aa5de8a 100644 --- a/docs/manual/known_problems.rst +++ b/docs/manual/known_problems.rst @@ -84,6 +84,13 @@ stay listed with their upstream references. category and are not findings. - Decide per file: write a test that instantiates it (it is shipped API), or remove it from the target's ``hdrs`` (it is not needed). + * - **A header reached through several targets is compiled under several + names.** Virtual-include trees of targets outside the scope are + resolved to the declared header by their path tail; if two in-scope + files share that tail the header stays unresolved. + - ``WARNING: ... matches several in-scope files`` in the reporter log; the + header appears as ``no-data``. + - Rename one of the files, or declare the header only once. * - **A source could not be staged for llvm-cov.** The reporter reads the sources from the scope's exported files; a file that is neither there nor in the workspace directory gets no HTML page (its numbers stay in diff --git a/docs/release/release_notes.rst b/docs/release/release_notes.rst index 3096537..db598ee 100644 --- a/docs/release/release_notes.rst +++ b/docs/release/release_notes.rst @@ -54,6 +54,13 @@ llvm-cov cannot show it, not even at 0 %. Until now such files were simply absent from the report and nobody noticed. The report now lists them, see :ref:`unmapped_files` below. +**Headers tested through a test-only twin target are measured.** A common +pattern declares a library's headers a second time in a test-only target with +test flags (baselibs' ``futurecpp_internal``). Tests then compile the headers +through that second target, and the report did not recognise the result as +belonging to the library: 172 futurecpp headers appeared untested. They are +now attributed to the library's declared header files. + **More untested files show their 0 %.** A library archive with one object that contains no code (typical for header-only libraries, see below) was rejected by llvm-cov as a whole, so the other files of that library lost their 0 % @@ -136,6 +143,9 @@ Details for integrators baseline. Archive members are now inspected and only those with a mapping are passed on, the way Rust rlibs were already handled (``tool_req__coverage_report_rlib_expansion`` v2). +- Fixed: a ``_virtual_includes/`` path of a target outside the scope (test-only + twin of an in-scope library) is resolved to the allowlisted header with the + same path tail instead of being excluded; ambiguous tails are warned about. - Fixed: the exclusion filter matches each out-of-scope compiled file exactly; an excluded ``foo/bar.h`` no longer suppresses an in-scope ``src/foo/bar.h``. - ``score_coverage_scope`` gained the ``path_map`` and ``source_files`` output diff --git a/docs/requirements/tool_requirements.rst b/docs/requirements/tool_requirements.rst index b763653..e2fe2d4 100644 --- a/docs/requirements/tool_requirements.rst +++ b/docs/requirements/tool_requirements.rst @@ -235,7 +235,10 @@ Report the HTML page location below ``coverage/`` and the index links. The canonical path is the allowlist path; for a header compiled through a ``_virtual_includes/`` tree it is the declared header from the scope's - path map. No absolute directory of the producing machine, no + path map, or, for the tree of a target outside the scope (a test-only twin + of a library exposing the same headers), the single allowlisted file whose + path ends with the header's path below the tree; a tail matching several + allowlisted files shall stay unresolved and be reported. No absolute directory of the producing machine, no ``/proc/self/cwd/`` prefix and no configuration-specific ``bazel-out//bin/`` prefix shall remain, so that the archived report is portable and file identity depends neither on the machine nor on the diff --git a/docs/verification/verification_report.rst b/docs/verification/verification_report.rst index b512ab7..0820bdc 100644 --- a/docs/verification/verification_report.rst +++ b/docs/verification/verification_report.rst @@ -48,7 +48,7 @@ Test inventory - 20 - merge_profraw, merge_no_data, merge_tool_error * - ``//score_coverage/tests:reporter_test`` - - 64 + - 69 - report_merged_profile, report_allowlist, report_baseline_zero, report_rlib_expansion, report_missing_baseline, report_relative_paths, report_outputs, report_unmapped, scope_transitive @@ -70,8 +70,8 @@ Test inventory * - ``//score_coverage/tests/starlark:coverage_scope_tests`` (13 analysis tests) - 13 - scope_transitive, scope_excludes, scope_baseline_objects - * - ``integration_tests/run_integration_test.sh`` (18 end-to-end checks) - - 18 + * - ``integration_tests/run_integration_test.sh`` (19 end-to-end checks) + - 19 - validation_ground_truth, report_baseline_zero, report_relative_paths, report_allowlist, report_unmapped, gate_exit_codes, gate_no_verdict, just_unknown_id, artifacts, summary_first diff --git a/integration_tests/expected_lcov.dat b/integration_tests/expected_lcov.dat index 589179f..a354f1f 100644 --- a/integration_tests/expected_lcov.dat +++ b/integration_tests/expected_lcov.dat @@ -28,10 +28,12 @@ # src/uncovered.cpp no test links against it => all lines 0, both directions # of the branch on line 18 never executed ('-'). # src/vendored/include/vendored/inline_math.h -# header-only library behind strip_include_prefix. The -# compiler records the generated _virtual_includes/ path; the -# report names the declared header (baselibs#558, -# coverage_tool#5). twice() (lines 18-20) is called once by +# header-only library behind strip_include_prefix. The test +# compiles it through the test-only twin vendored_math_internal +# (outside the scope), so the compiler records +# _virtual_includes/vendored_math_internal/...; the report +# resolves that to the declared header of the in-scope target +# (baselibs#558, coverage_tool#5, futurecpp pattern). twice() (lines 18-20) is called once by # coverable_test; never_inlined() (22-24) is never called and # clang still emits its mapping => 3 of 6 lines, no branches. # Exactly one record: the baseline variant of the same header diff --git a/integration_tests/run_integration_test.sh b/integration_tests/run_integration_test.sh index 9ad99e8..44a9e3d 100755 --- a/integration_tests/run_integration_test.sh +++ b/integration_tests/run_integration_test.sh @@ -202,6 +202,15 @@ fi rm -rf link_check echo "OK: $(echo "${LINKS}" | wc -l) index links resolve, canonical paths only, third-party code excluded" +echo "=== A header compiled only through a test-only twin target must be attributed to the declared file ===" +# Without the fallback the data sits under _virtual_includes/vendored_math_internal/ +# (a target outside the scope), gets excluded, and the header is listed as no-data. +grep -q "^SF:src/vendored/include/vendored/inline_math.h$" lcov.dat || { echo "ERROR: inline_math.h not attributed to its declared path" >&2; exit 1; } +if grep -q "vendored_math_internal" lcov.dat artifacts_dir/unmapped_files.txt 2>/dev/null; then + echo "ERROR: the test-only twin's virtual path leaked into the report" >&2; exit 1 +fi +echo "OK" + echo "=== A header nothing includes must be reported as unmapped, not invented in the LCOV ===" if grep -q "unused_api" lcov.dat; then echo "ERROR: src/unused_api.h has no compiled code and must not have an LCOV record" >&2 diff --git a/integration_tests/src/BUILD b/integration_tests/src/BUILD index 49f5082..fc1291f 100644 --- a/integration_tests/src/BUILD +++ b/integration_tests/src/BUILD @@ -50,12 +50,24 @@ cc_library( strip_include_prefix = "vendored/include", ) +# Test-only twin of vendored_math (same header, test copts), the pattern of +# baselibs' futurecpp_internal. Tests compile the header through THIS +# target's _virtual_includes/ tree, which is outside the coverage scope; the +# reporter must still attribute the data to the declared header. +cc_library( + name = "vendored_math_internal", + testonly = True, + hdrs = ["vendored/include/vendored/inline_math.h"], + defines = ["VENDORED_MATH_INTERNAL_TEST=1"], + strip_include_prefix = "vendored/include", +) + cc_test( name = "coverable_test", srcs = ["coverable_test.cpp"], deps = [ ":coverable", - ":vendored_math", + ":vendored_math_internal", "//third_party:extlib", "//third_party:vendored_ext", ], diff --git a/score_coverage/reporter.py b/score_coverage/reporter.py index a4fa993..492139e 100644 --- a/score_coverage/reporter.py +++ b/score_coverage/reporter.py @@ -231,6 +231,43 @@ def canonical_path(path: str, path_map: dict[str, str] | None = None) -> str: return stripped +_FOREIGN_VIRTUAL_RE = re.compile(r"^(.*/)?_virtual_includes/[^/]+/(?P.+)$") + + +def resolve_foreign_virtual_includes( + names: set[str], allowlist: set[str] +) -> tuple[dict[str, str], dict[str, list[str]]]: + """Map ``_virtual_includes/`` paths of targets outside the scope to the allowlisted file. + + The scope's path map only covers virtual-include trees of targets the + aspect visited. A test-only twin of a library (same ``hdrs`` behind + ``strip_include_prefix``, other copts; baselibs' ``futurecpp_internal``) + generates its own tree from the very same files, and test binaries record + that tree. Such a name is resolved by its tail: the allowlisted file that + ends with ```` at a path-component boundary. If no file matches the + full tail (an ``include_prefix`` added components), shorter tails are + tried. Returns ``{virtual name: canonical}`` and ``{virtual name: candidates}`` + for tails that match several allowlisted files (left unresolved). + """ + resolved: dict[str, str] = {} + ambiguous: dict[str, list[str]] = {} + for name in sorted(names): + match = _FOREIGN_VIRTUAL_RE.match(name) + if not match or name in allowlist: + continue + parts = match.group("tail").split("/") + for start in range(len(parts)): + suffix = "/".join(parts[start:]) + candidates = sorted(a for a in allowlist if a == suffix or a.endswith("/" + suffix)) + if len(candidates) == 1: + resolved[name] = candidates[0] + break + if len(candidates) > 1: + ambiguous[name] = candidates + break + return resolved, ambiguous + + def exclusion_regex(raw: str, roots: list[str]) -> str: """``--ignore-filename-regex`` that matches exactly one compiled file. @@ -675,6 +712,9 @@ def prepare_sources( baseline_covered = get_covered_files(llvm_bin_path, baseline_objects, None, workspace_root, path_map) print(f"INFO: Baseline archives contain {len(set(baseline_covered.values()))} files.", file=sys.stderr) + if allowlist_set is not None: + _attribute_foreign_virtual_includes(test_covered, baseline_covered, allowlist_set, path_map) + selection = select_files(test_covered, baseline_covered, allowlist_set, compiled_stems) for name, dropped in sorted(selection.duplicates.items()): print( @@ -720,6 +760,39 @@ def prepare_sources( return selection, path_map, root, regexes +def _attribute_foreign_virtual_includes( + test_covered: dict[str, str], + baseline_covered: dict[str, str], + allowlist: set[str], + path_map: dict[str, str], +) -> None: + """Rename covered files under a foreign ``_virtual_includes/`` tree to their declared file. + + Headers reached through the tree of a target outside the scope (a + test-only twin of an in-scope library) carry coverage data under a name + the path map does not know; they would be excluded as out of scope. + Updates ``test_covered`` / ``baseline_covered`` values and ``path_map`` in place. + """ + names = set(test_covered.values()) | set(baseline_covered.values()) + foreign, ambiguous = resolve_foreign_virtual_includes(names, allowlist) + if foreign: + path_map.update(foreign) + for covered in (test_covered, baseline_covered): + for raw, name in covered.items(): + if name in foreign: + covered[raw] = foreign[name] + print( + f"INFO: {len(foreign)} headers reached through virtual-include trees of targets outside the " + f"scope are reported under their declared path (e.g., {sorted(foreign.values())[:3]}).", + file=sys.stderr, + ) + for name, candidates in sorted(ambiguous.items()): + print( + f"WARNING: {name} matches several in-scope files ({candidates}); it stays out of the report.", + file=sys.stderr, + ) + + def run_llvm_cov_show( llvm_bin_path: Path, objects: list[str], diff --git a/score_coverage/tests/reporter_test.py b/score_coverage/tests/reporter_test.py index 9ad5ed6..6a92c36 100644 --- a/score_coverage/tests/reporter_test.py +++ b/score_coverage/tests/reporter_test.py @@ -331,6 +331,7 @@ def _write_tool(path: Path, body: str) -> Path: /ws/src/b.cpp 2 2 0.00% 1 1 0.00% 5 5 0.00% rust/lib.rs 3 0 100.00% 2 0 100.00% 8 0 100.00% bazel-out/k8-fastbuild/bin/src/_virtual_includes/v/api.h 2 0 100.00% 1 0 100.00% 3 0 100.00% +bazel-out/k8-fastbuild/bin/src/_virtual_includes/twin/w/w.h 2 0 100.00% 1 0 100.00% 3 0 100.00% ------------------------------------------------------------------------------------------------------- TOTAL 9 3 66.67% 4 1 75.00% 23 7 69.57% """ @@ -576,6 +577,7 @@ def test_get_covered_files_normalises_paths(self): "src/b.cpp": "src/b.cpp", "rust/lib.rs": "rust/lib.rs", "bazel-out/k8-fastbuild/bin/src/_virtual_includes/v/api.h": "src/_virtual_includes/v/api.h", + "bazel-out/k8-fastbuild/bin/src/_virtual_includes/twin/w/w.h": "src/_virtual_includes/twin/w/w.h", }, ) argv = self._last() @@ -780,6 +782,24 @@ def test_in_scope_file_without_coverage_data_is_listed(self): self.assertIn("src/never.h", err.getvalue()) self.assertIn("1 in-scope headers hold declarations only", err.getvalue()) + def test_foreign_virtual_include_is_resolved_against_the_allowlist(self): + # No path map entry for the "twin" tree (its target is outside the + # scope), but the allowlist has the declared header: resolved by tail. + self.allowlist.write_text("src/a.cpp\nsrc/w/include/w/w.h\n", encoding="utf-8") + ws = self.root / "ws" + (ws / "src" / "w" / "include" / "w").mkdir(parents=True) + (ws / "src" / "w" / "include" / "w" / "w.h").write_text("int w();\n", encoding="utf-8") + argv = self._argv() + argv[argv.index("--workspace_root") + 1] = str(ws) + err = io.StringIO() + with redirect_stderr(err): + reporter.main(argv) + self.assertIn("1 headers reached through virtual-include trees of targets outside the scope", err.getvalue()) + log = self.log.read_text(encoding="utf-8") + self.assertNotIn("twin", "\n".join(a for a in log.split() if a.startswith("--ignore-filename-regex"))) + staged = self.workdir / "sources" / "bazel-out/k8-fastbuild/bin/src/_virtual_includes/twin/w/w.h" + self.assertEqual(os.path.realpath(staged), os.path.realpath(ws / "src/w/include/w/w.h")) + def test_no_reports_writes_empty_zip(self): self.reports_file.write_text("", encoding="utf-8") with redirect_stderr(io.StringIO()): @@ -830,6 +850,59 @@ def test_unmapped_virtual_path_keeps_its_config_free_form(self): ) +@verifies("tool_req__coverage_scope_transitive", "tool_req__coverage_report_relative_paths") +class ForeignVirtualIncludesTest(unittest.TestCase): + """Virtual-include trees of targets outside the scope resolve to the declared in-scope file.""" + + ALLOW = { + "lib/include/score/apply.hpp", + "lib/include/score/private/invoke.hpp", + "other/include/score/apply.hpp", # a second apply.hpp elsewhere + "src/vendor/include/vendored/api.h", + "src/plain.cpp", + } + + def test_unique_tail_resolves(self): + resolved, ambiguous = reporter.resolve_foreign_virtual_includes( + {"lib/_virtual_includes/lib_internal/score/private/invoke.hpp", "src/plain.cpp"}, self.ALLOW + ) + self.assertEqual( + resolved, + {"lib/_virtual_includes/lib_internal/score/private/invoke.hpp": "lib/include/score/private/invoke.hpp"}, + ) + self.assertEqual(ambiguous, {}) + + def test_ambiguous_tail_is_reported_not_guessed(self): + resolved, ambiguous = reporter.resolve_foreign_virtual_includes( + {"lib/_virtual_includes/lib_internal/score/apply.hpp"}, self.ALLOW + ) + self.assertEqual(resolved, {}) + self.assertEqual( + ambiguous, + { + "lib/_virtual_includes/lib_internal/score/apply.hpp": [ + "lib/include/score/apply.hpp", + "other/include/score/apply.hpp", + ] + }, + ) + + def test_include_prefix_components_are_skipped(self): + # include_prefix = "pfx" adds a component the declared path does not have + resolved, _ = reporter.resolve_foreign_virtual_includes( + {"src/_virtual_includes/twin/pfx/vendored/api.h"}, self.ALLOW + ) + self.assertEqual( + resolved, {"src/_virtual_includes/twin/pfx/vendored/api.h": "src/vendor/include/vendored/api.h"} + ) + + def test_no_match_and_non_virtual_names_are_left_alone(self): + resolved, ambiguous = reporter.resolve_foreign_virtual_includes( + {"x/_virtual_includes/t/unknown.h", "src/plain.cpp", "lib/include/score/apply.hpp"}, self.ALLOW + ) + self.assertEqual((resolved, ambiguous), ({}, {})) + + @verifies("tool_req__coverage_report_allowlist") class ExclusionRegexTest(unittest.TestCase): """--ignore-filename-regex must hit exactly one compiled file."""