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 fscheck3 replay #515

Merged
merged 6 commits into from
Jan 8, 2025
Merged

Fix fscheck3 replay #515

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
e91ac3e
Fix replay for fscheck3
rynoV Sep 10, 2024
667b3b8
Update docs
rynoV Sep 10, 2024
edbe859
Fix to show the final seed instead of the original seed for fscheck3
rynoV Sep 12, 2024
acc624d
Add back original seed to fscheck3 output
rynoV Oct 17, 2024
7308453
Add note about fscheck2 replay size migration to readme
rynoV Jan 6, 2025
9c2cf10
Switch to use `int option` for replay size
rynoV Jan 7, 2025
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
4 changes: 2 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 (%A, %A) \"%s\"" methodName (uint64 std) (uint64 gen) name
sprintf "Focus on error:\n\t%s (%A, %A, 0) \"%s\"" methodName (uint64 std) (uint64 gen) name
Copy link
Collaborator

Choose a reason for hiding this comment

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

What is the motivation for always printing zero here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is so the user can copy and paste. The FsCheck 2 module will just ignore the third value


sprintf "Failed after %s. %s%s\nResult:\n\t%A\n%s%s%s"
(numTests data.NumberOfTests) parameters shrunk
Expand All @@ -92,7 +92,7 @@ module ExpectoFsCheck =
MaxFail = 1000
// 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))
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
12 changes: 7 additions & 5 deletions Expecto.FsCheck3/FsCheck3.fs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ module ExpectoFsCheck =
| TestResult.Failed (_,_,_, Outcome.Failed (:? IgnoreException as e),_,_,_) ->
raise e

| TestResult.Failed (data, original, shrunk, outcome,originalSeed,_finalSeed,size) ->
| TestResult.Failed (data, original, shrunk, outcome,originalSeed,finalSeed,size) ->
let parameters =
original
|> List.map (sprintf "%A")
Expand All @@ -70,12 +70,14 @@ module ExpectoFsCheck =
| _ -> sprintf "Labels of failing property (one or more is failing): %s\n"
(String.concat " " data.Labels)

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

sprintf "Failed after %s. %s%s\nResult:\n\t%A\n%s%s%s"
sprintf "Failed after %s. %s%s\nResult:\n\t%A\n%s%s%s\n%s"
(numTests data.NumberOfTests) parameters shrunk
outcome labels (stampsToString data.Stamps) focus
outcome labels (stampsToString data.Stamps) original focus
|> FailedException
|> raise

Expand All @@ -91,7 +93,7 @@ module ExpectoFsCheck =
let config =
Config.Default
.WithMaxTest(config.maxTest)
.WithReplay(Option.map (fun (seed,gamma) -> {Rnd = Rnd(seed, gamma); Size = None}) config.replay)
.WithReplay(Option.map (fun (seed,gamma,size) -> {Rnd = Rnd(seed, gamma); Size = Some(size)}) config.replay)
.WithName(name)
.WithStartSize(config.startSize)
.WithEndSize(config.endSize)
Expand Down
26 changes: 14 additions & 12 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 (1UL,3UL) "Deliberately failing test" <|
etestProperty (1UL,3UL,50) "Deliberately failing test" <|
fun a b c ->
// wrong on purpose to test failures
a * (b + c) = a * a + a * c
Expand All @@ -87,14 +87,15 @@ let runFsCheckFocusedTests =
match (getResult ["FsCheck focused";"Deliberately failing test"]).result with
| TestResult.Failed actual ->
let expected = "
Failed after 3 tests. Parameters:
-1 1 2
Shrunk 2 times to:
-1 0 0
Failed after 1 test. Parameters:
47 43 -38
Shrunk 9 times to:
1 0 0
Result:
Failed System.Exception: Expected true, got false.
Original seed: (1UL, 3UL)
Focus on error:
etestProperty (1UL, 3UL) \"Deliberately failing test\""
etestProperty (1UL, 3UL, 50) \"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 +105,7 @@ let config =
testList "FsCheck config" [
testCase "ignore me" ignore

etestPropertyWithConfig (1UL,3UL) FsCheckConfig.defaultConfig
etestPropertyWithConfig (1UL,3UL,50) FsCheckConfig.defaultConfig
"Deliberately failing test" <|
fun a b c ->
// wrong on purpose to test failures
Expand Down Expand Up @@ -132,14 +133,15 @@ let runFsCheckConfigTests =
match (getResult ["FsCheck config";"Deliberately failing test"]).result with
| TestResult.Failed actual ->
let expected = "
Failed after 3 tests. Parameters:
-1 1 2
Shrunk 2 times to:
-1 0 0
Failed after 1 test. Parameters:
47 43 -38
Shrunk 9 times to:
1 0 0
Result:
Failed System.Exception: Expected true, got false.
Original seed: (1UL, 3UL)
Focus on error:
etestPropertyWithConfig (1UL, 3UL) \"Deliberately failing test\""
etestPropertyWithConfig (1UL, 3UL, 50) \"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 (1UL,2UL) "Deliberately failing test" <|
etestProperty (1UL,2UL,0) "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 (1UL, 2UL) \"Deliberately failing test\""
etestProperty (1UL, 2UL, 0) \"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 (1UL,2UL) FsCheckConfig.defaultConfig
etestPropertyWithConfig (1UL,2UL,0) 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 (1UL, 2UL) \"Deliberately failing test\""
etestPropertyWithConfig (1UL, 2UL, 0) \"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: (uint64 * uint64) option
replay: (uint64 * uint64 * int) option
Copy link
Collaborator

Choose a reason for hiding this comment

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

The FsCheck API exposes size as an option. Is there a reason to require it here?

Also, we should probably update the help text to mention size

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmm I suppose not. I'll update to use an option, and update the fscheck2 stuff to use None

/// 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
2 changes: 1 addition & 1 deletion README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1372,5 +1372,5 @@ This might be due to how terminals/the locking thereof work: try running your te
## 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)`.
- 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. They will also now require a third item indicating the size. E.g. from `(1865288075, 296281834)` to `(1865288075UL, 296281834UL, 3)`. With FsCheck 2 the size is ignored and any number can be used.
- FsCheck 2 is no longer supported, so we're switching Expecto.FsCheck to use FsCheck 3 by default, even though FsCheck 3 is still in release candidate state. If you still want FsCheck2, we will continue to release FsCheck2 support for the time being using a version suffix, e.g. [11.0.0-fscheck2](https://www.nuget.org/packages/Expecto.FsCheck/11.0.0-alpha1-fscheck2)
6 changes: 6 additions & 0 deletions RELEASE_NOTES.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,9 @@
### 11.0.0-alpha4 - 2025-01-06
* Breaking change: Add third item to `FsCheckConfig.replay` indicating the size
* Fixes issue where the replay seed without the size was playing all tests leading up to the failure, making debugging
more difficult
* Existing FsCheck 2 users can enter any number for the size, because it is ignored.

### 11.0.0-alpha3 - 2024-10-13
* Add testParamAsync and testParamTask (#512), thanks @1eyewonder

Expand Down
Loading