test: improve arrow-row fuzz tests#9222
Conversation
1. Make them deterministic 2. Make them produce more unique values 3. Test Boolean array as well 4. Test different null behaviors 5. test that different underlying nulls values doesn't change the output rows
| )) | ||
| } | ||
|
|
||
| fn change_underline_null_values_for_primitive<T: ArrowPrimitiveType>( |
There was a problem hiding this comment.
| fn change_underline_null_values_for_primitive<T: ArrowPrimitiveType>( | |
| fn change_underlying_null_values_for_primitive<T: ArrowPrimitiveType>( |
| #[test] | ||
| #[cfg_attr(miri, ignore)] | ||
| fn fuzz_test() { | ||
| let mut rng = StdRng::seed_from_u64(42); |
There was a problem hiding this comment.
If we're fuzzing is it better to have differing seeds so we can test more random states or its better to be deterministic instead?
There was a problem hiding this comment.
Ideally I would say more test cases, BUT the impact if it starts to fail and someone just rerun it, it will remove trust in the test and will be ignored or just rerun to make the test pass rather than fixing the issue.
for example, think about some PR that updates a comment and this fails, so the author will have less trust in this test and when that author will touch arrow-row later in the future and that test fail again, they will just rerun the CI rather than fix the bug.
Ideally you would bound the seed to the current PR so:
- rerunning it will not help except creating a new PR which is less likely
- you can still reproduce
but it will be less intuitive how to reproduce the test
…fuzz-test' into improve-tests-for-row-convertor-fuzz-test
|
Thanks @rluvaton |
2 participants
Which issue does this PR close?
N/A
Rationale for this change
Make the arrow-row more resilient to bugs
What changes are included in this PR?
Are these changes tested?
N/A
Are there any user-facing changes?
Nope
I suggest that when reviewing this code to hide whitespace changes to see actual code changes