Skip to content
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.

Sign up for GitHub

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

Fix unusable replay seed with FsCheck3 #501

Merged
merged 4 commits into from
Aug 15, 2024
Merged

Fix unusable replay seed with FsCheck3 #501

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
5302857
Fix unusable replay seed with FsCheck3
rynoV Aug 9, 2024
f6a7408
Documentation for replay seed type change
rynoV Aug 12, 2024
e85dc44
Test for replay seed backward compatibility
rynoV Aug 12, 2024
4806f1a
Remove unused import
rynoV Aug 12, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions Expecto.FsCheck/FsCheck.fs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ module ExpectoFsCheck =
(String.concat " " data.Labels)

let focus =
sprintf "Focus on error:\n\t%s (%i, %i) \"%s\"" methodName std gen name
sprintf "Focus on error:\n\t%s (%A, %A) \"%s\"" methodName (uint64 std) (uint64 gen) name

sprintf "Failed after %s. %s%s\nResult:\n\t%A\n%s%s%s"
(numTests data.NumberOfTests) parameters shrunk
Expand All @@ -90,7 +90,9 @@ module ExpectoFsCheck =
let config =
{ MaxTest = config.maxTest
MaxFail = 1000
Replay = Option.map Random.StdGen config.replay
// We're converting uint64s to a smaller type, but it shouldn't be an issue because users are only using the
// values given in the test output, which are only ints when running FsCheck 2
Replay = Option.map Random.StdGen (config.replay |> Option.map (fun (seed, gamma) -> int seed, int gamma))
Name = name
StartSize = config.startSize
EndSize = config.endSize
Expand Down
4 changes: 2 additions & 2 deletions Expecto.FsCheck3/FsCheck3.fs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ module ExpectoFsCheck =
(String.concat " " data.Labels)

let focus =
sprintf "Focus on error:\n\t%s (%i, %i) \"%s\"" methodName originalSeed.Seed originalSeed.Gamma name
sprintf "Focus on error:\n\t%s (%A, %A) \"%s\"" methodName originalSeed.Seed originalSeed.Gamma name

sprintf "Failed after %s. %s%s\nResult:\n\t%A\n%s%s%s"
(numTests data.NumberOfTests) parameters shrunk
Expand All @@ -91,7 +91,7 @@ module ExpectoFsCheck =
let config =
Config.Default
.WithMaxTest(config.maxTest)
.WithReplay(Option.map (fun (seed,gamma) -> {Rnd = Rnd(uint64 seed, uint64 gamma); Size = None}) config.replay)
.WithReplay(Option.map (fun (seed,gamma) -> {Rnd = Rnd(seed, gamma); Size = None}) config.replay)
.WithName(name)
.WithStartSize(config.startSize)
.WithEndSize(config.endSize)
Expand Down
8 changes: 4 additions & 4 deletions Expecto.Tests.FsCheck3/FsCheck3Tests.fs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ let focused =
testList "FsCheck focused" [
testCase "ignore me" <| ignore

etestProperty (1,3) "Deliberately failing test" <|
etestProperty (1UL,3UL) "Deliberately failing test" <|
fun a b c ->
// wrong on purpose to test failures
a * (b + c) = a * a + a * c
Expand Down Expand Up @@ -94,7 +94,7 @@ Shrunk 2 times to:
Result:
Failed System.Exception: Expected true, got false.
Focus on error:
etestProperty (1, 3) \"Deliberately failing test\""
etestProperty (1UL, 3UL) \"Deliberately failing test\""
Expect.equal actual expected "It should fail with the right message"
| x ->
failtestf "Expected Failed, actual was: %A" x
Expand All @@ -104,7 +104,7 @@ let config =
testList "FsCheck config" [
testCase "ignore me" ignore

etestPropertyWithConfig (1,3) FsCheckConfig.defaultConfig
etestPropertyWithConfig (1UL,3UL) FsCheckConfig.defaultConfig
"Deliberately failing test" <|
fun a b c ->
// wrong on purpose to test failures
Expand Down Expand Up @@ -139,7 +139,7 @@ Shrunk 2 times to:
Result:
Failed System.Exception: Expected true, got false.
Focus on error:
etestPropertyWithConfig (1, 3) \"Deliberately failing test\""
etestPropertyWithConfig (1UL, 3UL) \"Deliberately failing test\""
Expect.equal actual expected "It should fail with the right message."

| x ->
Expand Down
8 changes: 4 additions & 4 deletions Expecto.Tests/FsCheckTests.fs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ let focused =
testList "FsCheck focused" [
testCase "ignore me" <| ignore

etestProperty (1,2) "Deliberately failing test" <|
etestProperty (1UL,2UL) "Deliberately failing test" <|
fun a b c ->
// wrong on purpose to test failures
a * (b + c) = a * a + a * c
Expand Down Expand Up @@ -94,7 +94,7 @@ Shrunk 4 times to:
Result:
False
Focus on error:
etestProperty (1, 2) \"Deliberately failing test\""
etestProperty (1UL, 2UL) \"Deliberately failing test\""
Expect.equal actual expected "It should fail with the right message"
| x ->
failtestf "Expected Failed, actual was: %A" x
Expand All @@ -104,7 +104,7 @@ let config =
testList "FsCheck config" [
testCase "ignore me" ignore

etestPropertyWithConfig (1,2) FsCheckConfig.defaultConfig
etestPropertyWithConfig (1UL,2UL) FsCheckConfig.defaultConfig
"Deliberately failing test" <|
fun a b c ->
// wrong on purpose to test failures
Expand Down Expand Up @@ -139,7 +139,7 @@ Shrunk 4 times to:
Result:
False
Focus on error:
etestPropertyWithConfig (1, 2) \"Deliberately failing test\""
etestPropertyWithConfig (1UL, 2UL) \"Deliberately failing test\""
Expect.equal actual expected "It should fail with the right message."

| x ->
Expand Down
2 changes: 1 addition & 1 deletion Expecto/Model.fs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ type FsCheckConfig =
/// The size to use for the last test, when all the tests are passing. The size increases linearly between Start- and EndSize.
endSize: int
/// If set, the seed to use to start testing. Allows reproduction of previous runs.
replay: (int * int) option
replay: (uint64 * uint64) option
/// The Arbitrary instances on this class will be merged in back to front order, i.e. instances for the same generated type at the front
/// of the list will override those at the back. The instances on Arb.Default are always known, and are at the back (so they can always be
/// overridden)
Expand Down
3 changes: 3 additions & 0 deletions RELEASE_NOTES.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
### 11.0.0 - 2024-08-09
* Fix unusable replay seed with FsCheck3
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since this is a breaking change, the release notes should be more descriptive of the breaking change.
Perhaps something like

Breaking Change: move FsCheckConfig.replay from int to uint64. 
Fixes issue where many FsCheck3 runs could not be replayed since the random seed is too large. 
Existing FsCheck 2 users should be able to use the same seeds values, but converted to uint64. 
Uint64 literals can be defined like `let iAm5 = 5UL`

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh. The ReadMe also needs updated with the new replay type. Probably also with update guidance

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated thanks, let me know if there's anything I missed


### 10.2.1 - 2024-03-15
* Fix bug where testTask and testCaseTask allow the tasks to start immediately when the test is defined, breaking backward compatibility with testTask.

Expand Down
Loading