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

Allow to disable the recheckAt-message #519

Merged
merged 1 commit into from
May 4, 2024

Conversation

sol
Copy link
Contributor

@sol sol commented May 2, 2024

This is not useful in the context of hspec-hedgehog. Hspec uses its global seed when running hedgehog properties and it already prints instruction on how to re-run with the same seed when a test fails.

@sol
Copy link
Contributor Author

sol commented May 2, 2024

@moodmosaic can you hold off with a new release for just a little bit longer? I may have more changes coming.

@sol sol force-pushed the disable-recheckAt-message branch from f6333b2 to 2bdf341 Compare May 2, 2024 12:08
@sol sol force-pushed the disable-recheckAt-message branch from 2bdf341 to f51874d Compare May 2, 2024 12:20
@moodmosaic
Copy link
Member

@sol, sure 👍

Copy link
Member

@moodmosaic moodmosaic left a comment

Choose a reason for hiding this comment

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

Perhaps a user would want to re-run the test this time using vanilla hedgehog? What happens then?

@sol
Copy link
Contributor Author

sol commented May 3, 2024

Perhaps a user would want to re-run the test this time using vanilla hedgehog? What happens then?

I don't need it myself, but if somebody requests it I'll add an option to Hspec that allows the user to explicitly request it (say --print-hedgehog-recheck-at-message). Currently a user gets two messages that tell them how to recheck with the same seed, one from Hspec and one from Hedgehog, which not only takes up screen estate but is also confusing. See this screenshot:

screen

Note that all Hspec options can be set through environment variable and config files (https://hspec.github.io/options.html#specifying-options-in-config-files), so it is easy for a user to specify what they want globally / per project.

@moodmosaic
Copy link
Member

This is great, then. 👍

@moodmosaic moodmosaic merged commit 0f342a7 into hedgehogqa:master May 4, 2024
25 checks passed
@sol sol deleted the disable-recheckAt-message branch May 4, 2024 08:48
@sol sol changed the title renderResultWith: Allow to disable the recheckAt-message Allow to disable the recheckAt-message May 4, 2024
@sol
Copy link
Contributor Author

sol commented May 4, 2024

Thanks @moodmosaic 🙏

@ChickenProp
Copy link
Contributor

This is not useful in the context of hspec-hedgehog. Hspec uses its global seed when running hedgehog properties and it already prints instruction on how to re-run with the same seed when a test fails.

Note that the shrink path ("11:b" in your screenshot) is still useful, but admittedly only if you know how to use it. If you're running with hspec (which I do) you need to do something like

HEDGEHOG_SKIP=11:b cabal test -- --match ... --seed ...

Combining part of hedgehog's recheckAt line with hspec's To rerun line.

So I'd be sad about losing this, but as long as I can get it back with an environment variable I'm happy. I think the ideal would be that hspec gets a --skip option, and outputs something like

To rerun use: --match ... --seed ... --skip 11:b

but I don't know how likely that is. We'd need hspec to recognize a new command-line option; my memory from last I checked is that there's no way for downstream projects to add support for options that hspec itself doesn't explicitly recognize. And we'd need it to allow hedgehog to change the "to rerun" line, which I don't remember checking if that's currently possible but my guess is not.

There's probably less ideal things that would still be an improvement on status quo for hspec users.

To check, do I understand right that this would need a PR to hspec-hedgehog to take effect for hspec users?

@sol
Copy link
Contributor Author

sol commented May 11, 2024

@ChickenProp can we move this discussion to #522?

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.

None yet

3 participants