Skip to content

Commit

Permalink
Fix unusable replay seed with FsCheck3 (#501)
Browse files Browse the repository at this point in the history
* Fix unusable replay seed with FsCheck3

* Documentation for replay seed type change

* Test for replay seed backward compatibility

* Remove unused import
  • Loading branch information
rynoV authored Aug 15, 2024
1 parent f359e21 commit bc14bbb
Show file tree
Hide file tree
Showing 7 changed files with 32 additions and 16 deletions.
6 changes: 4 additions & 2 deletions Expecto.FsCheck/FsCheck.fs
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
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
14 changes: 9 additions & 5 deletions Expecto.Tests.FsCheck3/FsCheck3Tests.fs
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,9 +139,13 @@ 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 ->
failtestf "Expected Failed, actual %A" x
}
}

[<Tests>]
let runFsCheckReplaySeedMigrationTests =
testProperty "int32/uint64 roundtrip" <| fun (FsCheck.DoNotSize(n)) -> n = int32 (uint64 n)
8 changes: 4 additions & 4 deletions Expecto.Tests/FsCheckTests.fs
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
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
8 changes: 6 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -723,7 +723,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 Expand Up @@ -860,7 +860,7 @@ If a property fails, the output could look like this.
Result:
False
Focus on error:
etestProperty (1865288075, 296281834) "addition is not commutative (should fail)"
etestProperty (1865288075UL, 296281834UL) "addition is not commutative (should fail)"

The output that Expecto gives you, lets you recreate the exact test (that's from the 18..., 29... seed numbers). It's
also a good idea to lift inputs and the test-case/parameter combination that failed into its *own* test (which isn't a
Expand Down Expand Up @@ -1366,3 +1366,7 @@ This might be due to how terminals/the locking thereof work: try running your te


[logary]: https://github.com/logary/logary#using-logary-in-a-library

## Migration notes

- 11.0.0: Any usages of the `replay` (a.k.a `stdGen` with `etestProperty*` functions) config with FsCheck tests will need to be updated to use `uint64` by appending `UL` to the literals, e.g. from `(1865288075, 296281834)` to `(1865288075UL, 296281834UL)`.
6 changes: 6 additions & 0 deletions RELEASE_NOTES.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,9 @@
### 11.0.0 - 2024-08-12
* 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`

### 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

0 comments on commit bc14bbb

Please sign in to comment.