Add a script to produce combined fuzzing coverage profiles. #24
Conversation
cov_profiles.sh
Outdated
|
|
||
| # Collect coverage profiles. | ||
| for p in ${packages[@]}; do | ||
| cd $PWD/$p/testdata/fuzz |
There was a problem hiding this comment.
It seems BASE_DIR and PWD are being used interchangeably, but it is possible the user runs cov_profiles.sh from a directory other than lnd-fuzz.
We should only use BASE_DIR.
There was a problem hiding this comment.
that is true in the above case but not in for f in $(ls $PWD); do
Crypt-iQ
force-pushed
the
cov_profiles_10_07_2024
branch
from
82d98b2 to
6b373e3
Compare
gijswijs
left a comment
gijswijs
left a comment
There was a problem hiding this comment.
This is a very nice improvement. Concept ACK
Made some remarks throughout.
cov_profiles.sh
Outdated
| for p in ${packages[@]}; do | ||
| cd "${BASE_DIR}/${p}/testdata/fuzz" | ||
|
|
||
| for f in $(ls $PWD); do |
There was a problem hiding this comment.
You can use for with a glob pattern. ./* is safer than * but both work in this case.
| for f in $(ls $PWD); do | |
| for f in ./*; do |
There was a problem hiding this comment.
For some reason this did not work for me
Crypt-iQ
force-pushed
the
cov_profiles_10_07_2024
branch
from
6b373e3 to
9054ca1
Compare
morehouse
left a comment
morehouse
left a comment
There was a problem hiding this comment.
Tried to test the script, but it's broken for me. See inline comments.
gijswijs
left a comment
gijswijs
left a comment
There was a problem hiding this comment.
Apart from @morehouse remarks, there's nothing left to do afaict. Once the script is fixed I'm confident Matt can give it the final stamp of approval, so I'm giving it mine right now. 🎉
Co-authored-by: Matt Morehouse <[email protected]>
Co-authored-by: Matt Morehouse <[email protected]>
Co-authored-by: Matt Morehouse <[email protected]>
Co-authored-by: Matt Morehouse <[email protected]>
|
|
||
| # Collect coverage profiles. | ||
| for p in ${PACKAGES[@]}; do | ||
| pushd "${BASE_DIR}/${p}/testdata/fuzz" |
There was a problem hiding this comment.
AFAICT this pushd and the matching popd are useless and should be removed.
|
|
||
| coverage_dirs+=("coverage/${f}") | ||
|
|
||
| popd |
There was a problem hiding this comment.
At this point I think we should do rm -r "${CACHE_DIR}/${f}" to ensure that fuzz targets with the same name will never cross-contaminate each others' corpus.
|
@Crypt-iQ, remember to re-request review from reviewers when ready |
|
!lightninglabs-deploy mute |
6 participants
Pretty much a copy of
corpus_merge.sh. After running the script, go to yourlnddirectory then rungo tool cover -html ../lnd-fuzz/coverage/profileto see coverage in HTML.Addresses part of #10