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.

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

Conversation

rynoV
Copy link
Contributor

@rynoV rynoV commented Jan 6, 2025

Fixes the replay config to use the size parameter so fscheck 3 will skip directly to the failing case. Also updates test output to show the "final seed" which will directly run the failing case when used, instead of the "original seed"

Separated out from #511

rynoV added 5 commits January 6, 2025 10:46
(cherry picked from commit f7d1690)
(cherry picked from commit e5943f6)
Using the original seed has the same problem

(cherry picked from commit e69fb64)
@@ -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

Expecto/Model.fs Outdated
@@ -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

@farlee2121
Copy link
Collaborator

Thanks for splitting these up!

This looks pretty good, I just had two questions that I left as comments.

To allow more flexibility and match fscheck's type
@farlee2121
Copy link
Collaborator

Looks great. I'll get a preview package released soon.
Thanks for the contribution!

@farlee2121 farlee2121 merged commit 2d364cd into haf:main Jan 8, 2025
2 checks passed
@farlee2121
Copy link
Collaborator

The alpha4 package is up

@rynoV rynoV deleted the fix-fscheck3-replay branch January 9, 2025 15:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants