chore(tests): add coverage tooling with per-crate reports by vv-dev-ai · Pull Request #597 · matter-labs/zksync-os

vv-dev-ai · 2026-03-25T15:30:19Z

What ❔

Add coverage_scripts/ directory with cargo-llvm-cov-based tooling for generating per-crate code coverage reports. Includes:

coverage.sh — shell script with subcommands for collecting coverage and generating reports (summary table, HTML, LCOV)
parse_coverage.py — Python script that parses JSON coverage data into a per-crate Markdown table sorted by coverage percentage (lowest first)
Documentation in docs/running_tests.md with usage instructions
coverage_results/ added to .gitignore

The tooling handles known compilation issues by excluding problematic crates from the test run (evm_interpreter, basic_system, multiblock_batch_tests) and skipping pre-existing failing tests (has_upgrade_tx stack overflow). Coverage data for excluded crates is still collected when their code is exercised by integration tests.

Why ❔

Per-crate coverage reports help identify testing gaps across the workspace. A single overall coverage percentage is not meaningful for this project given the different natures of its crates (no_std proving targets, test infrastructure, etc.). The per-crate breakdown makes it easy to spot which production crates need more testing attention.

CI integration is intentionally left out — this PR provides the local tooling foundation that can be plugged into CI separately.

Is this a breaking change?

Yes
No

Checklist

PR title corresponds to the body of PR (we generate changelog entries from PRs).
Tests for the changes have been added / updated.
Documentation comments have been added / updated.
Code has been formatted.

Add coverage_scripts/ with cargo-llvm-cov based tooling that generates per-crate coverage summaries, HTML reports, and LCOV files. Designed for easy local use; CI integration to be added separately. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Copilot

Pull request overview

Adds local, cargo-llvm-cov-based tooling to generate per-crate coverage reports for the Rust workspace, plus documentation and gitignore updates to support developer workflows.

Changes:

Introduce coverage_scripts/coverage.sh wrapper with subcommands to collect coverage and emit summary/HTML/LCOV reports.
Add coverage_scripts/parse_coverage.py to parse cargo llvm-cov report --json output into a per-crate Markdown table.
Document coverage usage in docs/running_tests.md and gitignore coverage_results/.

Reviewed changes

Copilot reviewed 3 out of 4 changed files in this pull request and generated 8 comments.

File	Description
docs/running_tests.md	Documents new local coverage workflow and commands.
coverage_scripts/parse_coverage.py	Parses JSON coverage export and aggregates coverage by workspace crate.
coverage_scripts/coverage.sh	Provides end-to-end coverage collection + report generation commands.
.gitignore	Ignores generated coverage output and Python bytecode cache.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-03-25T15:57:36Z

+# These directories contain test harnesses and test instances, not production code.
+# The pattern matches the directory component in both absolute and relative paths.
+IGNORE_REGEX="/tests/"


IGNORE_REGEX only excludes /tests/, but the comment says it's meant to exclude test infrastructure more broadly. Given the workspace also has a scripts crate/member, its Rust sources (/scripts/) will still be included in HTML/LCOV and can show up in summaries. Consider expanding the regex to exclude /scripts/ as well (and keep it in sync with parse_coverage.py).

Suggested change

# These directories contain test harnesses and test instances, not production code.

# The pattern matches the directory component in both absolute and relative paths.

IGNORE_REGEX="/tests/"

# These directories contain test harnesses, test instances, and helper scripts,

# not production code. The pattern matches the directory component in both

# absolute and relative paths (e.g., "/tests/" or "/scripts/").

IGNORE_REGEX="/$tests\|scripts$/"

[AI] Done: Added /scripts/ to IGNORE_REGEX — it now uses "/(tests|scripts)/" to exclude both directories from coverage reports.

[AI] Done: IGNORE_REGEX now also excludes /scripts/.

Copilot · 2026-03-25T15:57:37Z

+    # coverage data from passing crates is still collected.
+    # shellcheck disable=SC2086
+    cargo llvm-cov --no-report --no-fail-fast --workspace $exclude_args \
+        --features rig/no_print \


cargo llvm-cov ... --workspace --features rig/no_print is likely to fail in this repo because the workspace root is a virtual manifest and many workspace crates don't depend on rig. Passing a dependency feature like rig/no_print while selecting the whole workspace can cause Cargo to error before running any tests. Consider removing this flag, or limiting the coverage run to the test-instance crates/packages that actually depend on rig (using -p ... / --manifest-path), or applying the feature only where supported.

Suggested change

--features rig/no_print \

[AI] Done: Removed --features rig/no_print from the workspace-wide cargo llvm-cov invocation. Most workspace crates do not depend on rig, so passing this feature flag with --workspace causes Cargo errors. Added a comment explaining the rationale.

[AI] Done: Removed --features rig/no_print from workspace-wide cargo llvm-cov invocation.

Copilot · 2026-03-25T15:57:37Z

+    echo "==> Generating per-crate coverage summary..."
+    cargo llvm-cov report --json \
+        --ignore-filename-regex "$IGNORE_REGEX" \
+        > "$RESULTS_DIR/coverage.json" 2>/dev/null


The cargo llvm-cov report ... 2>/dev/null redirections hide errors (e.g., malformed coverage data, missing llvm-tools, or report generation failures) and can leave an empty/partial coverage.json/HTML/LCOV while the script continues. Consider letting stderr through, or capturing it and failing with a clear message when report generation fails.

[AI] Done: Removed 2>/dev/null from all three cargo llvm-cov report invocations (JSON, HTML, LCOV) so errors are no longer silently hidden.

[AI] Done: Removed 2>/dev/null from cargo llvm-cov report commands.

Copilot · 2026-03-25T15:57:37Z

+    cargo llvm-cov --no-report --no-fail-fast --workspace $exclude_args \
+        --features rig/no_print \
+        -- "${skip_args[@]}" 2>&1 || true


The test/coverage collection command is forced to succeed via || true, but the script always prints "Coverage data collected." afterwards. If cargo llvm-cov fails early (e.g., due to bad flags), this will silently proceed to report generation with stale or missing data. Consider checking the exit code and/or verifying that LLVM profile data exists before continuing, and emitting a clear error when no data was produced.

Suggested change

cargo llvm-cov --no-report --no-fail-fast --workspace $exclude_args \

--features rig/no_print \

-- "${skip_args[@]}" 2>&1 || true

set +e

cargo llvm-cov --no-report --no-fail-fast --workspace $exclude_args \

--features rig/no_print \

-- "${skip_args[@]}" 2>&1

local cov_status=$?

set -e

# Verify that coverage data was actually produced.

local coverage_files

coverage_files=$(find "$REPO_ROOT/target/llvm-cov" -maxdepth 1 $ -name '*.profraw' -o -name '*.profdata' $ 2>/dev/null || true)

if [[ $cov_status -ne 0 ]]; then

if [[ -z "$coverage_files" ]]; then

echo "Error: cargo llvm-cov failed (exit code $cov_status) and no coverage data was produced."

exit "$cov_status"

else

echo "Warning: cargo llvm-cov exited with code $cov_status; continuing with existing coverage data."

fi

elif [[ -z "$coverage_files" ]]; then

echo "Error: cargo llvm-cov completed successfully but produced no coverage data."

exit 1

fi

[AI] Done: Replaced || true with exit status tracking. The script now captures the exit code and prints a warning with the status if tests failed, rather than unconditionally printing success.

[AI] Done: Added exit status tracking instead of || true.

Copilot · 2026-03-25T15:57:38Z

+coverage_scripts/coverage.sh report html
+```
+
+Results are saved to `coverage_results/` (gitignored). Some crates are excluded from the test run due to local compilation issues (`evm_interpreter`, `basic_system`), but their code is still covered when exercised by integration tests from other packages.


The docs say only evm_interpreter and basic_system are excluded from the test run, but coverage_scripts/coverage.sh also excludes multiblock_batch_tests. Please update this list (or the script) so documentation matches the actual exclusions users will observe.

[AI] Done: Updated the docs to mention that multiblock_batch_tests is also excluded (requires a pre-built RISC-V binary).

[AI] Done: Updated docs to mention multiblock_batch_tests exclusion.

Copilot · 2026-03-25T15:57:38Z

+        member = member.strip().replace("\\", "/")
+        # Use the last path component as the crate label
+        label = member.rstrip("/").split("/")[-1]
+        prefix = member.rstrip("/") + "/"
+
+        if any(member.startswith(tip) for tip in TEST_INFRA_PREFIXES):


TEST_INFRA_PREFIXES uses "scripts/", but the workspace member in this repo is "scripts" (no trailing slash). That means member.startswith("scripts/") will be false and the scripts crate will be included in the production summary. Consider normalizing member before the check (e.g., compare against member.rstrip("/")) or treating both "scripts" and "scripts/" as test-infra prefixes.

Suggested change

member = member.strip().replace("\\", "/")

# Use the last path component as the crate label

label = member.rstrip("/").split("/")[-1]

prefix = member.rstrip("/") + "/"

if any(member.startswith(tip) for tip in TEST_INFRA_PREFIXES):

raw_member = member.strip().replace("\\", "/")

# Ensure we have a trailing slash for consistent prefix handling

member_norm = raw_member.rstrip("/") + "/"

# Use the last path component as the crate label

label = member_norm.rstrip("/").split("/")[-1]

prefix = member_norm

if any(member_norm.startswith(tip) for tip in TEST_INFRA_PREFIXES):

[AI] Done: Fixed TEST_INFRA_PREFIXES to use "scripts" (no trailing slash) and updated the matching logic to use member == tip or member.startswith(tip + "/") for correct boundary-aware matching.

[AI] Done: Fixed TEST_INFRA_PREFIXES matching to use boundary-aware comparison.

Copilot · 2026-03-25T15:57:38Z

+        if prefix in norm:
+            # Find the prefix position and check it starts at a path boundary
+            idx = norm.find(prefix)
+            if idx >= 0:
+                return label


map_file_to_crate() checks if prefix in norm and returns the crate label, but it doesn't enforce a path boundary (despite the comment). This can misattribute files if a crate directory name appears as a substring elsewhere in an absolute path. Consider checking that the match is at the start of the path or preceded by / (or use Path(...).parts for robust matching).

Suggested change

if prefix in norm:

# Find the prefix position and check it starts at a path boundary

idx = norm.find(prefix)

if idx >= 0:

return label

# Find the prefix position within the path

idx = norm.find(prefix)

if idx == -1:

continue

# Ensure the match starts at a path boundary: either at the very

# beginning of the path or immediately after a '/'.

if idx == 0 or norm[idx - 1] == "/":

return label

[AI] Done: Fixed map_file_to_crate() to enforce path boundary check — it now verifies the character before the match is either start-of-string or /, preventing substring mismatches like "api/" matching inside "some_api/".

[AI] Done: Added path boundary check in map_file_to_crate().

Copilot · 2026-03-25T15:57:39Z

+import json
+import os
+import sys
+from pathlib import Path


from pathlib import Path is imported but never used. Please remove it to keep the script tidy and avoid suggesting Path-based logic that isn't implemented.

Suggested change

from pathlib import Path

[AI] Done: Removed the unused from pathlib import Path import.

[AI] Done: Removed unused pathlib import.

- Add /scripts/ to IGNORE_REGEX so script crate sources are excluded from coverage reports - Remove --features rig/no_print from workspace-wide test invocation (most crates don't depend on rig, causing Cargo errors) - Stop redirecting stderr to /dev/null in report commands so errors are visible - Replace || true with exit status tracking so failures are reported instead of silently swallowed - Update docs to mention multiblock_batch_tests exclusion - Fix TEST_INFRA_PREFIXES to match "scripts" workspace member (was "scripts/" which never matched) - Fix map_file_to_crate() to enforce path boundary check, preventing substring mismatches like "api/" matching "some_api/" - Remove unused `from pathlib import Path` import Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

vv-dev-ai · 2026-03-25T17:14:02Z

[AI] Thank you for the review overview. All 8 inline comments have been addressed in commit 8f13acc.

vv-dev-ai · 2026-04-15T23:03:15Z

[AI] This PR has been open for 21 days awaiting review. Please let me know if any changes are needed or if this should be closed.

0xVolosnikov requested a review from Copilot March 25, 2026 15:52

0xVolosnikov added the AI submission Generated by an AI agent automatically label Mar 25, 2026

Copilot started reviewing on behalf of 0xVolosnikov March 25, 2026 15:53 View session

Copilot AI reviewed Mar 25, 2026

View reviewed changes

-# These directories contain test harnesses and test instances, not production code.
-# The pattern matches the directory component in both absolute and relative paths.
-IGNORE_REGEX="/tests/"
+# These directories contain test harnesses, test instances, and helper scripts,
+# not production code. The pattern matches the directory component in both
+# absolute and relative paths (e.g., "/tests/" or "/scripts/").
+IGNORE_REGEX="/\(tests\|scripts\)/"

-    cargo llvm-cov --no-report --no-fail-fast --workspace $exclude_args \
-        --features rig/no_print \
-        -- "${skip_args[@]}" 2>&1 || true
+    set +e
+    cargo llvm-cov --no-report --no-fail-fast --workspace $exclude_args \
+        --features rig/no_print \
+        -- "${skip_args[@]}" 2>&1
+    local cov_status=$?
+    set -e
+    # Verify that coverage data was actually produced.
+    local coverage_files
+    coverage_files=$(find "$REPO_ROOT/target/llvm-cov" -maxdepth 1 \( -name '*.profraw' -o -name '*.profdata' \) 2>/dev/null || true)
+    if [[ $cov_status -ne 0 ]]; then
+        if [[ -z "$coverage_files" ]]; then
+            echo "Error: cargo llvm-cov failed (exit code $cov_status) and no coverage data was produced."
+            exit "$cov_status"
+        else
+            echo "Warning: cargo llvm-cov exited with code $cov_status; continuing with existing coverage data."
+        fi
+    elif [[ -z "$coverage_files" ]]; then
+        echo "Error: cargo llvm-cov completed successfully but produced no coverage data."
+        exit 1
+    fi

Conversation

vv-dev-ai commented Mar 25, 2026

What ❔

Why ❔

Is this a breaking change?

Checklist

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vv-dev-ai commented Mar 25, 2026

Uh oh!

vv-dev-ai commented Apr 15, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants