-
Notifications
You must be signed in to change notification settings - Fork 93
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Print property seeds and add docs about reproducing test failures #90
Conversation
val seed = scalaCheckTestParameters.initialSeed.getOrElse(Seed.random()) | ||
val result = check(scalaCheckTestParameters, prop.useSeed(test.name, seed)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The ScalaCheck runner uses prop.viewSeed(name, seed)
but that causes the prop to use an hardcoded println
to output the seed, which would look quite messy since it appears before the test failure in the output.
Instead, we determine the seed in the same way ScalaCheck does (getting it from the parameters or falling back to a random one) and inject it in the property using useSeed
(which is what propertyWithSeed
does in ScalaCheck)
Now we're free to print the seed ourselves, which looks much better and more consistent with the rest of the failure message.
implicit class XtensionScalaCheckTestParameters( | ||
params: ScalaCheckTest.Parameters | ||
) { | ||
def withInitialSeed(encodedSeed: String): ScalaCheckTest.Parameters = | ||
params.withInitialSeed(Seed.fromBase64(encodedSeed).get) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ScalaCheck does something analogous in propertyWithSeed
, and I think it makes sense (since we print the Base64 encoded form in test failures)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems like a regression to declare the seed as a string instead of int, why do we need to use base64 encoded strings?
ScalaCheck itself uses base64 encoded seeds in the output. See an example here https://gist.github.com/non/aeef5824b3f681b9cfc141437b16b014 You can still use Long if you prefer, but I preferred to stay consistent with the default ScalaCheck runner |
Using a Base64 encoded string is fine. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 👍
docs/integrations/scalacheck.md
Outdated
|
||
To reproduce the failure you can follow the suggestion to fix the seed: | ||
|
||
```scala |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In these cases I normally use the diff
language to highlight what's changed
class MySuite extends ScalaCheckSuite {
+ override val scalaCheckInitialSeed = "CTH6hXj8ViScMmsO78-k4_RytXHPK_wSJYNH2h4dCpB="
// ...
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nice idea 👍 done!
Previously we would not print seeds when a property would fail, so it would be impossible to deterministically reproduce a failure.
Now we print the seed when a property fails and we added docs to explain how to reproduce failures