haskell-hedgehog

haskell-hedgehog copied to clipboard

Having this only work on lists is more restrictive than it needs to be. This commit generalizes element to instead work on any Foldable.

I want this because in state machine testing I quite often do things like this:

    commandGen :: State Symbolic -> Maybe (Gen (RegisterUser Symbolic))
    commandGen state = do
      knownUsernames <- nonEmpty $ state ^.. #users . folded . #username

      return do
        RegisterUser{ email, password } <- genRegisterUser
        username                        <- Gen.element $ toList knownUsernames

        return RegisterUser{ username, .. }

It's annoying to have to use toList here - it would be nice if I could just say:

        username                        <- Gen.element knownUsernames

@jacobstanley this now actually builds :laughing:

@moodmosaic any thoughts?

Thank you @ocharles 👍 🚀

From the API consumer's point of view, Gen.element $ knownUsernames looks indeed betten than Gen.element $ toList knownUsernames.

A couple of notes:

• Types that are instances of Foldable include not only lists but also other data structures like trees, maps, sets, and more. Those aren't going to work in this case though...
• Perhaps accepting NonEmpty a would be best(?)
• Postel's law isn't violated by taking on Foldable, so that's a good thing

I've never seen a QC-style elements combinator accepting Foldable, that's why I added those notes above.

Types that are instances of Foldable include not only lists but also other data structures like trees, maps, sets, and more. Those aren't going to work in this case though...

Why?

Perhaps accepting NonEmpty a would be best(?)

It would definitely be an improvement. Having tests crash due to an empty list is really annoying.

Perhaps accepting NonEmpty a would be best(?)

It would definitely be an improvement.

I don't see this as a clear-cut improvement. elements is frequently used with static lists.

For static list you don't benefit much from the additional safety that NonEmpty provides, but it complicates your code.

(you would then need to use NonEmpty.fromList + add the corresponding import)

Thank you, @ocharles. Also, thank you @tmcgilchrist and @sol for the comments.