Upload CI generated fuzz corpus coverage to codecov #4153
Conversation
|
👋 Thanks for assigning @TheBlueMatt as a reviewer! |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #4153 +/- ##
==========================================
+ Coverage 88.63% 89.18% +0.54%
==========================================
Files 180 180
Lines 135230 136622 +1392
Branches 135230 136622 +1392
==========================================
+ Hits 119865 121846 +1981
+ Misses 12594 12171 -423
+ Partials 2771 2605 -166
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:
|
Because each CI job runs on a fresh runner and can't share data between jobs. We rely on Github Actions upload-artifact and download-artifact to share the CI generated fuzz corpus, then replay them in the `contrib/generate_fuzz_coverage.sh` script to generate the coverage report.
Anyitechs
force-pushed
the
upload-fuzz-coverage
branch
from
dc493c2 to
fdf6799
Compare
contrib/generate_fuzz_coverage.sh
Outdated
| for target_dir in hfuzz_workspace/*; do | ||
| [ -d "$target_dir" ] || continue | ||
| src_name="$(basename "$target_dir")" | ||
| for dest in "$src_name" "${src_name%_target}"; do |
There was a problem hiding this comment.
I don't think you need to copy into $src_name.
contrib/generate_fuzz_coverage.sh
Outdated
| mkdir -p "test_cases/$dest" | ||
| # Copy corpus files into the test_cases directory | ||
| find "$target_dir" -maxdepth 2 -type f \ | ||
| \( -path "$target_dir/CORPUS/*" -o -path "$target_dir/INPUT/*" -o -path "$target_dir/NEW/*" -o -path "$target_dir/input/*" \) \ |
There was a problem hiding this comment.
Because we're just looking in hfuzz_workspace, I believe we only need to look in input, not CORPUS, INPUT, or NEW.
| cargo clean | ||
| - name: Run fuzzers | ||
| run: cd fuzz && ./ci-fuzz.sh && cd .. | ||
| - name: Upload honggfuzz corpus |
There was a problem hiding this comment.
Rather than only uploading, is there a way to make this directory persistent so that we can keep it between fuzz jobs?
There was a problem hiding this comment.
I'm not sure if we really need to persist the directory here. My understanding is that the fuzz job runs on the latest code changes on every PR, so the generated corpus is tailored to the code changes on that PR. If we persist the corpus from a previous run and use that on a new run, won't that produce incorrect/misleading coverage data?
There was a problem hiding this comment.
I don't think the point of the fuzz job is only to generate coverage data, but rather test the code :). Having a bit more coverage data from fuzzing than we "deserve" is okay, at least now that we split the coverage data out so that codecov shows fuzzing separately, and having persistent fuzzing corpus means our fuzzing is much more likely to catch issues.
There was a problem hiding this comment.
Right, how long do you think we can have this directory persisted? The upload-artifact action have a retention-days input that can be used to persist the artifact for a while. The default is 90 days but can be adjusted (https://github.com/actions/upload-artifact?tab=readme-ov-file#retention-period).
contrib/generate_fuzz_coverage.sh
Outdated
| # Copy corpus files into the test_cases directory | ||
| find "$target_dir" -maxdepth 2 -type f \ | ||
| \( -path "$target_dir/CORPUS/*" -o -path "$target_dir/INPUT/*" -o -path "$target_dir/NEW/*" -o -path "$target_dir/input/*" \) \ | ||
| -print0 | xargs -0 -I{} cp -n {} "test_cases/$dest/" 2>/dev/null || true |
There was a problem hiding this comment.
| -print0 | xargs -0 -I{} cp -n {} "test_cases/$dest/" 2>/dev/null || true | |
| -print0 | xargs -0 -I{} cp -n {} "test_cases/$dest/" |
contrib/generate_fuzz_coverage.sh
Outdated
| done | ||
| # Check if any files were actually imported | ||
| if [ -n "$(find test_cases -type f -print -quit 2>/dev/null)" ]; then | ||
| imported=1 |
There was a problem hiding this comment.
Not sure its worth the extra effort just to print differently.
|
👋 The first review has been submitted! Do you think this PR is ready for a second reviewer? If so, click here to assign a second reviewer. |
|
Thank you for the review. I've addressed all feedbacks and pushed a fixup here 1e4a7c5 |
3 participants
Following the work (#3718 and #3925) that introduced uploading coverage from no-corpus fuzzing runs into codecov in CI. This PR focuses on uploading the CI-generated fuzz corpus coverage into codecov in CI.
Closes #3926