Simplify fuzzing coverage (CI) scripts somewhat#3925
Simplify fuzzing coverage (CI) scripts somewhat#3925TheBlueMatt merged 3 commits intolightningdevkit:mainfrom
Conversation
|
👋 Thanks for assigning @joostjager as a reviewer! |
![graphite-app[bot]](https://avatars.githubusercontent.com/in/158384?s=60&v=4){kind=small}
.github/workflows/build.yml
Outdated
| ./contrib/generate_fuzz_coverage.sh --output-dir `pwd` | ||
| # Could you use this to fake the coverage report for your PR? Sure. | ||
| # Will anyone be impressed by your amazing coverage? No | ||
| # Maybe if codecov wasn't broken we wouldn't need to do this... | ||
| bash <(curl -s https://codecov.io/bash) -f fuzz-codecov.json -t "f421b687-4dc2-4387-ac3d-dc3b2528af57" |
There was a problem hiding this comment.
There appears to be a path mismatch in the coverage file handling. The script is called with --output-dir \pwd`which will create the coverage file at`pwd`/fuzz-codecov.json, but the codecov upload command is looking for fuzz-codecov.json` in the current directory without a path prefix.
To fix this, either:
- Add the
--output-codecov-jsonflag to the script call, or - Update the file path in the bash command to
\pwd`/fuzz-codecov.json`
This will ensure the codecov upload can find the generated coverage file.
| ./contrib/generate_fuzz_coverage.sh --output-dir `pwd` | |
| # Could you use this to fake the coverage report for your PR? Sure. | |
| # Will anyone be impressed by your amazing coverage? No | |
| # Maybe if codecov wasn't broken we wouldn't need to do this... | |
| bash <(curl -s https://codecov.io/bash) -f fuzz-codecov.json -t "f421b687-4dc2-4387-ac3d-dc3b2528af57" | |
| ./contrib/generate_fuzz_coverage.sh --output-dir `pwd` | |
| # Could you use this to fake the coverage report for your PR? Sure. | |
| # Will anyone be impressed by your amazing coverage? No | |
| # Maybe if codecov wasn't broken we wouldn't need to do this... | |
| bash <(curl -s https://codecov.io/bash) -f `pwd`/fuzz-codecov.json -t "f421b687-4dc2-4387-ac3d-dc3b2528af57" |
Spotted by Diamond
Is this helpful? React 👍 or 👎 to let us know.
joostjager
left a comment
joostjager
left a comment
There was a problem hiding this comment.
Small PR, but still difficult to review without much explanation. Commit message just says 'simplify', but adding self-comments in the PR explaining why this is all equivalent would be helpful.
|
👋 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. |
| ;; | ||
| *) | ||
| echo "Unknown option: $1" | ||
| echo "Usage: $0 [--output-dir OUTPUT_DIRECTORY]" |
There was a problem hiding this comment.
The usage information is missing the newly added --output-codecov-json flag. Please update the help text to include this option:
echo "Usage: $0 [--output-dir OUTPUT_DIRECTORY] [--output-codecov-json]"This will help users understand all available command-line options.
| echo "Usage: $0 [--output-dir OUTPUT_DIRECTORY]" | |
| echo "Usage: $0 [--output-dir OUTPUT_DIRECTORY] [--output-codecov-json]" |
Spotted by Diamond
Is this helpful? React 👍 or 👎 to let us know.
There was a problem hiding this comment.
Its a CI-only flag so not really worth documenting.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #3925 +/- ##
=========================================
Coverage 89.01% 89.02%
=========================================
Files 166 167 +1
Lines 119692 121800 +2108
Branches 119692 121800 +2108
=========================================
+ Hits 106546 108427 +1881
- Misses 10740 10956 +216
- Partials 2406 2417 +11
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:
|
TheBlueMatt
force-pushed
the
2025-07-3718-followups
branch
from
95df8a0 to
427d984
Compare
We recently introduced uploading coverage from no-corpus fuzzing runs into codecov in CI. Here, we simplify some of the scripts that do so, especially removing the second `ci-fuzz.sh` file we added to the repo.
codecov recommends using the new CLI uploader rather than their (deprecated) bash uploader (which we use). It also appears to have a fail-on-error mode which we'd prefer over the bash one which silently swallows errors.
TheBlueMatt
force-pushed
the
2025-07-3718-followups
branch
from
427d984 to
08795f2
Compare
|
Okay, this seems to work, and codecov's report includes coverage for lines that are |
joostjager
left a comment
joostjager
left a comment
There was a problem hiding this comment.
If I understand it correctly, all the coverage now ends up in the same report?
Its useful to be able to identify what kind of coverage we have for a line - test coverage is very different in nature from fuzzing coverage. Here we pass separate "job names" to codecov uploads to do so.
TheBlueMatt
force-pushed
the
2025-07-3718-followups
branch
from
059c0cf to
37cd32d
Compare
joostjager
left a comment
joostjager
left a comment
There was a problem hiding this comment.
One unclarified issue around fuzz coverage (process_onion_failure), but seems unlikely to have anything to do with this PR.
|
Landing as CI-only. |
3 participants
We recently introduced uploading coverage from no-corpus fuzzing runs into codecov in CI. Here, we simplify some of the scripts that do so, especially removing the second
ci-fuzz.shfile we added to the repo.