hedgehogqa / haskell-hedgehog

Release with confidence, state-of-the-art property testing for Haskell.
675 stars 107 forks source link

Add `Semigroup` instance to the `Group` data type #261

Open chshersh opened 5 years ago

chshersh commented 5 years ago

Currently if I want to run all property-based tests, I need to put them all into single group. But I would like to separate my test groups into different modules. For example, I have basic roundtrip properties for core data types and functions and in separate directory I have property-based tests for checking consistency of my SQL database. If Group had Semigroup instance, I could just combine all my groups of tests into single one.

moodmosaic commented 5 years ago

Would something like this work? https://github.com/hedgehogqa/haskell-hedgehog/pull/173#pullrequestreview-158446660

chshersh commented 5 years ago

@moodmosaic That's a valid alternative to me! Maybe hedgehog could have helper function like:

checkGroups :: [Group] -> IO Bool
jacobstanley commented 4 years ago

I'm coming around to this idea, we'd just have to choose whether it's left or right biased for where the title comes from.

Data.Map is left biased so that is an obvious choice:

The Semigroup operation for Map is union, which prefers values from the left operand

chshersh commented 4 years ago

@jacobstanley Thanks for considering the issue! On the one hand, it doesn't sound good to ignore group names, on the other hand, I don't care too much. I was using a workaround proposed by @moodmosaic and it works for me so far. However, as a user of the library, I probably still would like to see all group names. Maybe something like newtype Groups = Groups { unGroups :: [Group] } with checkGroups could work better, where I can have:

propertyTests :: IO ()
propertyTests = checkGroups $ mconcat
    [ roundtripTests
    , parsingTests
    , validationTests
    , stateTests
    , timeConversionTests
    ]

with some helper constructor

roundtripTests :: Groups
roundtripTests = singleGroup "Roundtrip property tests"
    [ ... 
    ]
jacobstanley commented 4 years ago

Yeah a tree like structure would certainly be optimal, I have had request to be able to have a way to check in parallel across an entire project, and having a tree structure group Group could be a step in that direction.

I think I may have looked at doing this in the past but it wasn't so easy to do without breaking lots of stuff. Adding a Semigroup instance otoh is easily backwards compatible.

I can imagine a use case where you do $$(discover) <> Group "" [.. to discover most properties and just add a few manual ones that need an argument to run.

Maybe we should look at doing both? The Semigroup is ofc something that could be on Hackage within days.