Emit warning when using --shard-count (> 1) with --order rand - #3186
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## devel #3186 +/- ##
==========================================
- Coverage 91.25% 91.24% -0.01%
==========================================
Files 204 204
Lines 8965 8973 +8
==========================================
+ Hits 8181 8187 +6
- Misses 784 786 +2 🚀 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
|
0749a82 to
6ac51cd
Compare
6ac51cd to
126c88a
Compare
Sorry for the delay, updated |
|
Thanks |
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.