-
Notifications
You must be signed in to change notification settings - Fork 5
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add a script to produce combined fuzzing coverage profiles. #24
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Concept ACK
cov_profiles.sh
Outdated
|
||
# Collect coverage profiles. | ||
for p in ${packages[@]}; do | ||
cd $PWD/$p/testdata/fuzz |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
that is true in the above case but not in for f in $(ls $PWD); do
82d98b2
to
6b373e3
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a very nice improvement. Concept ACK
Made some remarks throughout.
for p in ${packages[@]}; do | ||
cd "${BASE_DIR}/${p}/testdata/fuzz" | ||
|
||
for f in $(ls $PWD); do |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For some reason this did not work for me
@Crypt-iQ, remember to re-request review from reviewers when ready |
6b373e3
to
9054ca1
Compare
Pretty much a copy of
corpus_merge.sh
. After running the script, go to yourlnd
directory then rungo tool cover -html ../lnd-fuzz/coverage/profile
to see coverage in HTML.Addresses part of #10