-
Notifications
You must be signed in to change notification settings - Fork 9
Add fuzz subcommand #46
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
Conversation
cargo-rbmt/src/fuzz.rs
Outdated
| use xshell::Shell; | ||
|
|
||
| /// Default package name for fuzz targets. | ||
| const FUZZ_PACKAGE: &str = "bitcoin-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.
I don't see why we have to use bitcoin-fuzz as the package name in rust-bitcoin, would it be easier (in future perhaps) to change it and use fuzz as the default here?
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.
Yea, probably. We are the only users so easy to roll out. I was just taking the path of least resistance for now.
tcharding
left a comment
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.
ACK 743fab9
|
I acke'd because if it works it works. But did you try it out on E.g [[bin]]
name = "bitcoin_arbitrary_block"
path = "fuzz_targets/bitcoin/arbitrary_block.rs" |
Yea, I am running it on rust-bitcoin and it prints out the targets (e.g. |
|
No I didn't test this one, was lazy and just asked you. 'trust don't verify', right? |
743fab9 to
2ef2083
Compare
|
2ef2083: rebased |
|
BTW do we expect this crate to be formatted? It's pretty close to being so. |
I am planning on adding the same lint and fmt rules as rust-bitcoin with #54 |
tcharding
left a comment
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.
ACK 2ef2083
Initially only supporting the dynamic listing of targets.
2ef2083 to
9a24e67
Compare
|
9a24e67: rebased and switched the default package name to just |
tcharding
left a comment
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.
ACK 9a24e67
|
utACK 9a24e67 |
Initially only supporting the dynamic listing of targets, but that might still be super helpful for the CI jobs. The command is following a similar style of
integrationwhere it is assuming some sort offuzzcrate in the workspace.Related to #5