Skip to content

Fix for: Tests in fuzz directory [issue #91] - #96

Closed
kiminuo wants to merge 2 commits into
rust-bitcoin:masterfrom
kiminuo:feature/2020-06-extend-from-vec-issue-91
Closed

Fix for: Tests in fuzz directory [issue #91]#96
kiminuo wants to merge 2 commits into
rust-bitcoin:masterfrom
kiminuo:feature/2020-06-extend-from-vec-issue-91

Conversation

@kiminuo

@kiminuo kiminuo commented Jun 6, 2020

Copy link
Copy Markdown
Contributor
  • Move duplicate_crash_2 test to semantic.rs
  • Move extend_vec_from_hex to lib.rs (de-duplicate)

Fixes #91

@kiminuo
kiminuo force-pushed the feature/2020-06-extend-from-vec-issue-91 branch 2 times, most recently from d545f6d to 82069e8 Compare June 6, 2020 12:48
@kiminuo
kiminuo force-pushed the feature/2020-06-extend-from-vec-issue-91 branch from 82069e8 to 4a24ab2 Compare June 6, 2020 12:49
@kiminuo kiminuo changed the title Tests in fuzz directory (fixes #91) Fix for: Tests in fuzz directory [issue #91] Jun 6, 2020
@kiminuo

kiminuo commented Jun 6, 2020

Copy link
Copy Markdown
Contributor Author

https://github.com/apoelstra/rust-miniscript/pull/96/files#diff-4374b9b87c971ea1a2293a875f41efabR4 is not that nice. It would be nicer to put it somehow to https://github.com/apoelstra/rust-miniscript/blob/master/fuzz/Cargo.toml. Not sure how. Rust gurus are welcomed here :-)

@kiminuo kiminuo mentioned this pull request Jun 6, 2020
@sanket1729

Copy link
Copy Markdown
Member

Hi @kiminuo , even I am not sure what would be the best way to address this. I also made an attempt for this in #95 .

My idea was to simply put those in README. My understanding was that code should not be a part of fuzz repo at all. We had it because it was easy to reproduce crashes. So, whenever some contributor wants to reproduce a crash, they can paste the code from README, debug the crash, remove the crash code before we merge the PR.
We can wait for @apoelstra's opinion on how to address this.

@kiminuo

kiminuo commented Jun 7, 2020

Copy link
Copy Markdown
Contributor Author

Hi @kiminuo , even I am not sure what would be the best way to address this. I also made an attempt for this in #95 .

Your PR looks very good. A lot of goodies.

My idea was to simply put those in README. My understanding was that code should not be a part of fuzz repo at all. We had it because it was easy to reproduce crashes. So, whenever some contributor wants to reproduce a crash, they can paste the code from README, debug the crash, remove the crash code before we merge the PR.

To be honest, I don't find that really user friendly. But it might be what @apoelstra had in mind, I don't know.

@kiminuo kiminuo closed this Jun 17, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Tests in fuzz directory

2 participants