From bc14bbb9129816f246cbed105f447079303627dc Mon Sep 17 00:00:00 2001 From: Calum Sieppert Date: Wed, 14 Aug 2024 18:20:00 -0600 Subject: [PATCH] Fix unusable replay seed with FsCheck3 (#501) * Fix unusable replay seed with FsCheck3 * Documentation for replay seed type change * Test for replay seed backward compatibility * Remove unused import --- Expecto.FsCheck/FsCheck.fs | 6 ++++-- Expecto.FsCheck3/FsCheck3.fs | 4 ++-- Expecto.Tests.FsCheck3/FsCheck3Tests.fs | 14 +++++++++----- Expecto.Tests/FsCheckTests.fs | 8 ++++---- Expecto/Model.fs | 2 +- README.md | 8 ++++++-- RELEASE_NOTES.md | 6 ++++++ 7 files changed, 32 insertions(+), 16 deletions(-) diff --git a/Expecto.FsCheck/FsCheck.fs b/Expecto.FsCheck/FsCheck.fs index d0c99bf0..3758cd20 100644 --- a/Expecto.FsCheck/FsCheck.fs +++ b/Expecto.FsCheck/FsCheck.fs @@ -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 @@ -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 diff --git a/Expecto.FsCheck3/FsCheck3.fs b/Expecto.FsCheck3/FsCheck3.fs index eebe56e6..55c8e4c4 100644 --- a/Expecto.FsCheck3/FsCheck3.fs +++ b/Expecto.FsCheck3/FsCheck3.fs @@ -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 @@ -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) diff --git a/Expecto.Tests.FsCheck3/FsCheck3Tests.fs b/Expecto.Tests.FsCheck3/FsCheck3Tests.fs index 0c234271..f1bbeb2b 100644 --- a/Expecto.Tests.FsCheck3/FsCheck3Tests.fs +++ b/Expecto.Tests.FsCheck3/FsCheck3Tests.fs @@ -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 @@ -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 @@ -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 @@ -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 - } \ No newline at end of file + } + +[] +let runFsCheckReplaySeedMigrationTests = + testProperty "int32/uint64 roundtrip" <| fun (FsCheck.DoNotSize(n)) -> n = int32 (uint64 n) diff --git a/Expecto.Tests/FsCheckTests.fs b/Expecto.Tests/FsCheckTests.fs index e4f3f8e6..a6059aca 100644 --- a/Expecto.Tests/FsCheckTests.fs +++ b/Expecto.Tests/FsCheckTests.fs @@ -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 @@ -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 @@ -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 @@ -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 -> diff --git a/Expecto/Model.fs b/Expecto/Model.fs index 4e2b96dd..aeb3c7dd 100644 --- a/Expecto/Model.fs +++ b/Expecto/Model.fs @@ -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) diff --git a/README.md b/README.md index 3280bc79..6fab015f 100644 --- a/README.md +++ b/README.md @@ -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) @@ -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 @@ -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)`. \ No newline at end of file diff --git a/RELEASE_NOTES.md b/RELEASE_NOTES.md index c6e77c07..d26a2fa1 100644 --- a/RELEASE_NOTES.md +++ b/RELEASE_NOTES.md @@ -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.