Fail property test when discard count exceeds maximumDiscarded
#112
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
majk-p
approved these changes
Nov 8, 2024
| dogfood.runSuite(Meta.ConfigOverrideChecks)) | ||
| } | ||
|
|
||
| test("Discarded counts should be accurate") { dogfood => | ||
| expectErrorMessage( | ||
| s"Discarded more inputs (${Meta.DiscardedChecks.checkConfig.maximumDiscarded}) than allowed", | ||
| s"Discarded more inputs (${Meta.DiscardedChecks.checkConfig.maximumDiscarded + 1}) than allowed", |
There was a problem hiding this comment.
I'd be surprised to see a number different than what's in the config. How about a slightly redacted message?
Suggested change
| s"Discarded more inputs (${Meta.DiscardedChecks.checkConfig.maximumDiscarded + 1}) than allowed", | |
| s"Discarded more inputs than allowed. Input discard limit is ${Meta.DiscardedChecks.checkConfig.maximumDiscarded}.", |
There was a problem hiding this comment.
Definitely! I've gone with the slightly more terse:
Discarded more inputs (2) than allowed (1)
Baccata
approved these changes
Nov 8, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Property tests with a
maximumDiscardedvalue of zero fail without running any tests. This is a bug introduced by the stream refactors in #39.This is because the status stream now outputs an initial empty
Status.startvalue which satisfies thediscard <= maximumDiscardedstop condition.Prior to the refactor, the stream wouldn't output the
Status.startvalue, so a single test would be run before the condition was checked. This behaviour was also odd: even if no values were actually discarded, the property test could fail with a discard error.This PR alters the discard behaviour. The discard count must exceed the
maximumDiscardedvalue, not be equal to it. This means that if amaximumDiscardedvalue of0is set, then the test will fail if1value is discarded.