Skip to content

Conversation

@HuwCampbell
Copy link
Member

As discussed in #233.

@HuwCampbell HuwCampbell force-pushed the topic/discarding-fail branch from abd765b to 4121279 Compare March 20, 2019 11:12
@jacobstanley jacobstanley added the next breaking Merge on the next breaking change / major version label Mar 20, 2019
@fosskers
Copy link

fosskers commented May 3, 2019

A similar issue exists for PropertyT.

@moodmosaic
Copy link
Member

moodmosaic commented May 3, 2019

See this conversation regarding PropertyT https://github.com/hedgehogqa/haskell-hedgehog/pull/233/files#r234163334

@jacobstanley
Copy link
Member

jacobstanley commented May 13, 2019

I've spent some time thinking about this and the problem I have with it is that ideally one should aim to write tests / generators that don't ever discard.

Discarding slows things down and I would consider its use a bit of an anti-pattern. I think it should only really be used as a last resort. You're better of using the just / filter functions which locally retry the generation so that your "discard" is limited in scope, rather than throwing away the entire test because some deeply nested generator has failed.

just :: MonadGen m => m (Maybe a) -> m a
filter :: MonadGen m => (a -> Bool) -> m a -> m a

So I'm just a bit unsure about making it syntactically convenient to discard, as often there are more appropriate options. I actually wonder if it would be better to remove the MonadFail instance for Gen and have it be a compile error. This would mean you can't use it for the "this should never happen" scenario though, which you could argue isn't going to cause any huge issues (like discarding does).

@moodmosaic
Copy link
Member

I actually wonder if it would be better to remove the MonadFail instance for Gen and have it be a compile error.

This aligns pretty well with @sol's comment on the same topic (QC): nick8325/quickcheck#228 (comment)

If we decide to get rid of MonadFail perhaps we better do now, before next major release is out.

@jacobstanley
Copy link
Member

If we decide to get rid of MonadFail perhaps we better do now, before next major release is out.

Too late 😅

I think this topic has too many trade-offs to rush the decision, so I'm not that concerned. What we ended up releasing is the same as what it has always been.

@jacobstanley jacobstanley force-pushed the master branch 2 times, most recently from 4139585 to c228279 Compare May 22, 2022 14:42
@HuwCampbell
Copy link
Member Author

I'm going to bump this, as I still think this is a better behaviour.

@moodmosaic
Copy link
Member

@HuwCampbell, in which scenario(s) do you rely more on discards?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

next breaking Merge on the next breaking change / major version

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants