Expand input coverage in clarity-serialization #3
Conversation
Co-authored-by: BowTiedRadone <[email protected]>
Co-authored-by: Nikos Baxevanis <[email protected]>
Co-authored-by: Nikos Baxevanis <[email protected]>
Co-authored-by: Nikos Baxevanis <[email protected]>
Co-authored-by: Nikos Baxevanis <[email protected]>
Co-authored-by: Nikos Baxevanis <[email protected]>
Co-authored-by: Nikos Baxevanis <[email protected]>
Co-authored-by: Nikos Baxevanis <[email protected]>
There was a problem hiding this comment.
Always nice to see some prop tests! This is a huge win for improving our code's coverage (since it didn't have any unit tests previously!). I'm also thinking about the larger goal of preventing consensus-breaking changes, which eventually prompted this testing effort. Do you see this PR as a step in that direction?
| // 90% regular names, 10% the special "__transient" contract name. | ||
| prop_oneof
There was a problem hiding this comment.
prop_oneof! weights set probabilities, not counts. So 9 => regular_names, 1 => "__transient" gives "__transient" a 10% chance each time(run). Over 256 cases, it will almost always show up at least once. If we change it to 255/1, then "__transient" drops to a 1/256 chance per case, making it practically impossible to hit in a default test run. I suggest keeping it as is, since this distribution ensures the special case still has a reasonable chance of being covered.
@Jiloc Yes — this PR is part of stacks-network#6355 and represents one step in a larger “recipe” for expanding input coverage, with the other parts tracked in the issue. |
There was a problem hiding this comment.
Everything looks good to me, but would it make sense if we move this to a separate PR that targets develop directly (and the clarity crate, not clarity-serialization)? Mostly because we don't know when the merge of this branch with clarity-serialization will actually happen and any addition in here will make the diff of the clarity-serialization PR bigger for the reviewers. The test code will still be the same fortunately. Moving it to clarity and merging it first will have the added value of proving that the tests pass before and after the clarity crate will change to consume the clarity-serialization one. What do you think? @moodmosaic @BowTiedRadone
|
@Jiloc, yes, this makes perfect sense. Thank you. |
This PR broadens the range of tested inputs (as per stacks-network#6355) to include boundaries and edge cases. It validates that serialization, deserialization, and regex checks behave consistently.