ci: fix fuzz coverage report#4522
Conversation
|
👋 Thanks for assigning @TheBlueMatt as a reviewer! |
30be0ed to
798d4ea
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #4522 +/- ##
==========================================
+ Coverage 86.14% 87.09% +0.94%
==========================================
Files 160 162 +2
Lines 108046 108539 +493
Branches 108046 108539 +493
==========================================
+ Hits 93080 94531 +1451
+ Misses 12346 11524 -822
+ Partials 2620 2484 -136
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Since cargo-llvm-cov 0.7.0, only workspace members are instrumented by default. Since the fuzz crate is a standalone workspace, library crates like lightning were not instrumented, and the coverage report was empty. Add --dep-coverage to instrument the library path dependencies. This alone is not sufficient for the report: --dep-coverage's report filtering only supports crates.io deps, not path deps (per a TODO in cargo-llvm-cov source). Add --no-default-ignore-filename-regex to include all instrumented code, then use a custom --ignore-filename-regex to exclude unwanted paths (cargo registry, rustup toolchains, fuzz harness). AI tools were used in preparing this commit.
798d4ea to
922d9f1
Compare
|
Ugh, can't get a coverage report with fuzz failing. |
|
Fuzz passed and subsequent fuzzing coverage now gets uploaded again. The fuzzer hits 48% of channelmanager. |
| if [ "$OUTPUT_CODECOV_JSON" = "0" ]; then | ||
| cargo llvm-cov --html --ignore-filename-regex "fuzz/" --output-dir "$OUTPUT_DIR" | ||
| cargo llvm-cov --html \ | ||
| --dep-coverage lightning,lightning-invoice,lightning-liquidity,lightning-rapid-gossip-sync,lightning-persister \ |
There was a problem hiding this comment.
lightning-types is a transitive dependency (used by lightning, lightning-invoice, and lightning-liquidity) and contains non-trivial code (features.rs, payment.rs, routing.rs, string.rs). It should likely be included in the --dep-coverage list so that coverage of its code is tracked.
Similarly, possiblyrandom is a dependency of lightning and may warrant inclusion depending on whether its coverage is of interest.
| echo "Replaying imported corpus (if found) via tests to generate coverage..." | ||
| cargo llvm-cov -j8 --codecov --ignore-filename-regex "fuzz/" \ | ||
| cargo llvm-cov -j8 --codecov \ | ||
| --dep-coverage lightning,lightning-invoice,lightning-liquidity,lightning-rapid-gossip-sync,lightning-persister \ |
There was a problem hiding this comment.
The --dep-coverage list and the --ignore-filename-regex are now duplicated between the HTML path (lines 60-64) and the codecov JSON path (lines 85-88). If a new dependency is added to fuzz/Cargo.toml, both places must be updated in lockstep. Consider extracting these common flags into a shell variable (e.g., LLVM_COV_FLAGS) to avoid divergence.
Review SummaryIssues found: 2
|
|
Coverage looks to be there now, but there is a different problem now: https://app.codecov.io/github/lightningdevkit/rust-lightning/commit/5330f9f4802be749ed46420690497c54f658fdd5/blob/lightning/src/ln/channelmanager.rs?flags%5B0%5D=fuzzing&dropdown=coverage When you navigate to a source file:
|
Fix fuzz coverage reporting