Emit warning when using --shard-count (> 1) with --order rand - #3186
Emit warning when using --shard-count (> 1) with --order rand#3186mattyrazz7 wants to merge 1 commit into
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## devel #3186 +/- ##
==========================================
- Coverage 91.14% 91.14% -0.00%
==========================================
Files 204 204
Lines 8942 8950 +8
==========================================
+ Hits 8150 8157 +7
- Misses 792 793 +1 🚀 New features to boost your workflow:
|
44dc3cc to
0aad6b3
Compare
| ) | ||
| set_tests_properties(TestSharding::NoWarningOnRandomOrderWithFixedSeed | ||
| PROPERTIES | ||
| FAIL_REGULAR_EXPRESSION "Warning: using sharding \\(--shard-count\\) with random order" |
There was a problem hiding this comment.
This should be more generic (less specific), so that small rewording of the err. message doesn't break it.
Or have it be a CMake variable that is used by this & and the test above, so that the two tests are guaranteed to be kept in sync.
| set_tests_properties(TestSharding::NoWarningOnRandomOrderWithFixedSeed | ||
| PROPERTIES | ||
| FAIL_REGULAR_EXPRESSION "Warning: using sharding \\(--shard-count\\) with random order" | ||
| ) |
There was a problem hiding this comment.
There should also be a test that the warning doesn't fire without rng-seed, but with declaration/alphabetical order.
|
Thanks, this will be useful. |
0aad6b3 to
b9283fd
Compare
| PROPERTIES | ||
| FAIL_REGULAR_EXPRESSION "${CATCH_SHARDING_WARNING_REGEX}" | ||
| ) | ||
| endforeach() |
There was a problem hiding this comment.
Used a loop to test both lex and decl, but let me know if you want me to do it another way
| PASS_REGULAR_EXPRESSION "Randomness seeded to: 17171717" | ||
| ) | ||
|
|
||
| set(CATCH_SHARDING_WARNING_REGEX "Warning: using sharding .* with random order") |
There was a problem hiding this comment.
Went with this to give us more flexibility while still getting the gist of the message
2 participants
Description
Using sharding with the default test ordering (
--order rand) doesn't behave properly since each--shard-indexinvocation obtains a freshly reordered set, resulting in duplication and omitted tests. This can be fixed by changing the test ordering or using a fixed random seed between invocations, but it's surprising default behavior.Change
Mitigate by emitting a warning when the following conditions are met:
--shard-count> 1--orderisrand--rng-seedis set totimeorrandom-deviceThe warning points users to either set
--order decl,--order lex, or pass a fixed value to--rng-seed.