Avoid running earlier successes with --quickcheck-replay #410

facundominguez · 2024-03-18T18:53:08Z

On failure, tasty-quickcheck now suggests string seeds (with an implementation-dependent structure). Integer seeds are still accepted as input though.

Fixes #383

quickcheck/Test/Tasty/QuickCheck.hs

facundominguez · 2024-03-18T23:15:45Z

quickcheck/Test/Tasty/QuickCheck.hs

-  in printf "Use --quickcheck-replay=%d%s to reproduce." seed sizeStr
+makeReplayMsg :: (QCGen, Int) -> String
+makeReplayMsg seedSz =
+  printf "Use --quickcheck-single-replay=\"%s\" to reproduce." (show seedSz)


Probably we should display both the value of --quickcheck-replay and --quickcheck-single-replay here. Otherwise, the user of --quickcheck-replay has no way to get a seed.

The value printed for --quickcheck-replay would be only meaningful when not setting --quickcheck-single-replay. Otherwise, the result of the run is unrelated to the value of --quickcheck-replay.

What are the remaining use cases for --quickcheck-replay now? I guess you can just reuse it without introducing a new command line flag.

I don't know who is relying on the old behavior. I'm happy to break that if nobody cares.

I find it hard to imagine that someone would consciously prefer old behaviour to a new, but it seems you can support both without too much trouble. If it's --quickcheck-replay=(QCGen ..., 42) do a new thing, if it's --quichckeck-replay=QCGen... do the old thing.

And when printing, should we show both forms when sensible?

Showing one form would be enough IMO.

It can be done as you suggest. But note that the old form is unusable if the user has no way to discover the seeds that go with it. What would be the purpose of supporting the old form as input then?

Just an escape hatch. I do not see why a user might want the old behavior back; but someone could have a regression script still using an old format. Not crashing in such scenario would be useful.

Pushed. Also added a note to the change log. Thanks for your review @Bodigrim!

On failure, tasty-quickcheck now suggests string seeds (no integer seeds anymore). Integer seeds are still accepted as input though.

facundominguez · 2024-03-25T18:00:14Z

I was wondering, is this PR good to merge? Or does it need more reviews?

Bodigrim

A few more wibbles, but overall looks good to me, thanks!

CC @andreasabel @martijnbastiaan @VictorCMiraldo to review.

quickcheck/CHANGELOG.md

Bodigrim · 2024-03-25T23:46:02Z

quickcheck/Test/Tasty/QuickCheck.hs

+-- | Replay seed. The @Left int@ form is kept for legacy purposes.
+-- The form @Right (qcgen, intSize)@ holds both the seed and the size
+-- to run QuickCheck tests.
+newtype QuickCheckReplay = QuickCheckReplay (Maybe (Either Int (QCGen, Int)))


Could it be

Suggested change

newtype QuickCheckReplay = QuickCheckReplay (Maybe (Either Int (QCGen, Int)))

newtype QuickCheckReplay = QuickCheckReplay (Maybe (Int, Maybe QCGen))

?

I find the code easier to approach as it is.

The two Ints in the Either type have different meanings, whether the meaning of the Int in the Maybe form depends on the value of the second component of the pair.

The quickcheck API can take the pair (QCGen, Int) as is without further ado.

But let me know if you still think the Maybe form is to be preferred.

Another option is to use a custom type:

data Seed = OldSeed Int | NewSeed (QCGen, Int)

or

data QuickcheckReplay = QuickcheckReplay Int | QuickcheckReplayNew (QCGen, Int)

OK, unless someone else feels strongly about it, I'm fine to keep it as is.

quickcheck/Test/Tasty/QuickCheck.hs

Co-authored-by: ˌbodʲɪˈɡrʲim <[email protected]>

martijnbastiaan

LGTM.

I do like a custom data type better (Seed/QuickCheckReplay), but I'm fine with keeping it as it is now too.

facundominguez · 2024-04-05T11:04:55Z

Thanks @martijnbastiaan

I do like a custom data type better (Seed/QuickCheckReplay),

I contributed an implementation of this idea in facundominguez#1.
I could make it a follow up PR to UnkindPartition/tasty or maintainers can cherry-pick it if they wish.

Bodigrim · 2024-04-09T20:56:22Z

I contributed an implementation of this idea in facundominguez#1.
I could make it a follow up PR to UnkindPartition/tasty or maintainers can cherry-pick it if they wish.

This looks nice. Please add it to this PR + mention the change to QuickCheckReplay in the changlog + bump major version of tasty-quickcheck - and I think we'll be good to merge.

Bodigrim · 2024-04-09T20:57:10Z

please ensure that everything has correct @since pragmas.

facundominguez · 2024-04-09T22:01:17Z

add it to this PR

mention the change to QuickCheckReplay in the changelog

bump major version of tasty-quickcheck

please ensure that everything has correct @since pragmas.

I added @since 0.11 tags to the data constructors of the QuickCheckReplay data type. Let me know if more tags are needed.

Bodigrim · 2024-04-09T22:17:41Z

@facundominguez thanks! README.md can use an update too, if you are up to it.

@amesgen as an original author of #383, are you happy with this solution?

facundominguez · 2024-04-10T00:52:24Z

README.md can use an update too, if you are up to it.

I'm a bit embarrassed to admit that I cannot discover on my own what needs updating there. 🙈

amesgen

This looks excellent!

Bodigrim · 2024-04-10T19:01:59Z

I'm a bit embarrassed to admit that I cannot discover on my own what needs updating there. 🙈

@facundominguez the output of --help could be updated, but I'm not sure it was kept up-to-date before anyway.

Thanks for your contribution, merging.

From the [changelog](https://hackage.haskell.org/package/tasty-quickcheck-0.11/changelog): > Produce seeds that run a single failing tests instead of reproducing all the earlier successes ([#410](UnkindPartition/tasty#410)). > > Seeds are now pairs instead of single integers, e.g. `--quickcheck-replay="(SMGen 2909028190965759779 12330386376379709109,0)"` > > Single integer seeds are still accepted as input, but they do run through earlier successes. > > The `QuickCheckReplay` type used as a tasty option has three data constructors now. `QuickCheckReplayNone` is the default value and provides no seed. `QuickCheckReplayLegacy` takes an integer as before. The `QuickCheckReplay` data constructor takes the new seed form. This is very useful both for slow tests, as well as for the WIP nightly tests.

Version 0.11.1 -------------- * Add timeouts for individual tests within a property ([#425](UnkindPartition/tasty#425)). * Define `showDefaultValue` for `QuickCheckTests`, `QuickCheckMaxSize` and `QuickCheckMaxRatio` ([#428](UnkindPartition/tasty#428)). * Print the number of QuickCheck shrinks in the progress message ([#431](UnkindPartition/tasty#431)). * Drop support for GHC 7.10. Version 0.11 -------------- * Fix issues with QuickCheck progress reporting in the presence of `withMaxSuccess` ([#419](UnkindPartition/tasty#419)). * Produce seeds that run a single failing tests instead of reproducing all the earlier successes ([#410](UnkindPartition/tasty#410)). Seeds are now pairs instead of single integers, e.g. `--quickcheck-replay="(SMGen 2909028190965759779 12330386376379709109,0)"` Single integer seeds are still accepted as input, but they do run through earlier successes. The `QuickCheckReplay` type used as a tasty option has three data constructors now. `QuickCheckReplayNone` is the default value and provides no seed. `QuickCheckReplayLegacy` takes an integer as before. The `QuickCheckReplay` data constructor takes the new seed form.

facundominguez commented Mar 18, 2024

View reviewed changes

quickcheck/Test/Tasty/QuickCheck.hs Outdated Show resolved Hide resolved

facundominguez force-pushed the fd/single-replay branch from 475d8e9 to 14e6ce9 Compare March 18, 2024 21:27

facundominguez marked this pull request as ready for review March 18, 2024 21:29

facundominguez force-pushed the fd/single-replay branch from 14e6ce9 to 1f6ed71 Compare March 18, 2024 21:45

facundominguez commented Mar 18, 2024

View reviewed changes

facundominguez force-pushed the fd/single-replay branch 3 times, most recently from b87ef3a to f5206e2 Compare March 19, 2024 01:55

Avoid running earlier successes with --quickcheck-replay

363da1a

On failure, tasty-quickcheck now suggests string seeds (no integer seeds anymore). Integer seeds are still accepted as input though.

facundominguez force-pushed the fd/single-replay branch from f5206e2 to 363da1a Compare March 19, 2024 10:30

facundominguez changed the title ~~Implement --quickcheck-single-replay for tasty-quickcheck~~ Avoid running earlier successes with --quickcheck-replay Mar 19, 2024

Bodigrim reviewed Mar 25, 2024

View reviewed changes

Comment edits

69bf87e

Co-authored-by: ˌbodʲɪˈɡrʲim <[email protected]>

Bodigrim approved these changes Mar 26, 2024

View reviewed changes

martijnbastiaan approved these changes Apr 5, 2024

View reviewed changes

facundominguez mentioned this pull request Apr 5, 2024

Name the different cases of QuickCheckReplay facundominguez/tasty#1

Open

facundominguez added 3 commits April 9, 2024 21:49

Name the different cases of QuickCheckReplay

732e99e

Describe QuickCheckReplay changes

304e371

Bump tasty-quickcheck version to 0.11 and add 'since' tags

7dc69b6

facundominguez force-pushed the fd/single-replay branch from 921dc11 to 7dc69b6 Compare April 9, 2024 21:53

amesgen approved these changes Apr 10, 2024

View reviewed changes

Bodigrim merged commit 83b3a53 into UnkindPartition:master Apr 10, 2024
14 checks passed

amesgen mentioned this pull request Jun 25, 2024

Use tasty-quickcheck-0.11 IntersectMBO/ouroboros-consensus#1161

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Avoid running earlier successes with --quickcheck-replay #410

Avoid running earlier successes with --quickcheck-replay #410

facundominguez commented Mar 18, 2024 •

edited

Loading

facundominguez Mar 18, 2024 •

edited

Loading

Bodigrim Mar 19, 2024

facundominguez Mar 19, 2024

Bodigrim Mar 19, 2024

facundominguez Mar 19, 2024

Bodigrim Mar 19, 2024

facundominguez Mar 19, 2024 •

edited

Loading

Bodigrim Mar 19, 2024

facundominguez Mar 19, 2024

facundominguez commented Mar 25, 2024

Bodigrim left a comment

Bodigrim Mar 25, 2024

facundominguez Mar 26, 2024 •

edited

Loading

Bodigrim Mar 26, 2024

martijnbastiaan left a comment

facundominguez commented Apr 5, 2024 •

edited

Loading

Bodigrim commented Apr 9, 2024

Bodigrim commented Apr 9, 2024

facundominguez commented Apr 9, 2024 •

edited

Loading

Bodigrim commented Apr 9, 2024

facundominguez commented Apr 10, 2024 •

edited

Loading

amesgen left a comment

Bodigrim commented Apr 10, 2024

	newtype QuickCheckReplay = QuickCheckReplay (Maybe (Either Int (QCGen, Int)))
	newtype QuickCheckReplay = QuickCheckReplay (Maybe (Int, Maybe QCGen))

Avoid running earlier successes with --quickcheck-replay #410

Avoid running earlier successes with --quickcheck-replay #410

Conversation

facundominguez commented Mar 18, 2024 • edited Loading

facundominguez Mar 18, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

facundominguez Mar 19, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

facundominguez commented Mar 25, 2024

Bodigrim left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

facundominguez Mar 26, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

martijnbastiaan left a comment

Choose a reason for hiding this comment

facundominguez commented Apr 5, 2024 • edited Loading

Bodigrim commented Apr 9, 2024

Bodigrim commented Apr 9, 2024

facundominguez commented Apr 9, 2024 • edited Loading

Bodigrim commented Apr 9, 2024

facundominguez commented Apr 10, 2024 • edited Loading

amesgen left a comment

Choose a reason for hiding this comment

Bodigrim commented Apr 10, 2024

facundominguez commented Mar 18, 2024 •

edited

Loading

facundominguez Mar 18, 2024 •

edited

Loading

facundominguez Mar 19, 2024 •

edited

Loading

facundominguez Mar 26, 2024 •

edited

Loading

facundominguez commented Apr 5, 2024 •

edited

Loading

facundominguez commented Apr 9, 2024 •

edited

Loading

facundominguez commented Apr 10, 2024 •

edited

Loading