search

hedgehogqa / haskell-hedgehog-classes

Hedgehog will eat your typeclass bugs
BSD 3-Clause "New" or "Revised" License
56 stars 17 forks source link

Add tests for nearly all laws #19

Closed ag-eitilt closed 5 years ago

ag-eitilt commented 5 years ago

After the issues with Ord and Storable, it seemed like a good idea to be sure there weren't any other bugs in the tests themselves, and that can't really be done with a half-null test suite. Good news is, those seem to be the only problems! At least, unless something's hiding in Arrow or Category...

I wound up splitting some of the laws lists (see, eg., Semigroup.hs) on the basis that the sublaws shouldn't be mashed into the overriding ones, leaving their top-level test*Laws empty, but it does seem like a bit of an unwieldy layout with the output being divided by both law category and instance type. In my mind, the best would be to have a single group for testShowLaws, say, containing one list of concat [lawsE, lawsInt*, lawsWord*], but as there's not any way to specify a prefix for the individual laws themselves, having a big block of identical "Commutative Monoid: passed" statements doesn't help anything. Regardless, I see the laws* lists as being in this case more for if one of them needs testing with multiple generators in the future than for testing different sets of laws.

chessai commented 5 years ago

Oh man, this is great! Yeah, I was kicking myself over both of those issues and have been meaning to finish the test suite in this manner. Also, yeah, unfortunately Arrow/Category are paper generators and it's hard to actually test concrete values of them, without some unsafe hacks.

chessai commented 5 years ago

Thanks so much!

chessai commented 5 years ago

Btw, it wasn't the monad instance. It was the MonadPlus instance. some bad copy/pasting had left the tcName as "Monad" instead of "MonadPlus".

ag-eitilt commented 5 years ago

No problem! I've been relying on hedgehog-classes to kickstart the test suites for my own projects, and didn't want to wind up with too twisty a chain of #if MIN_VERSION-hedgehog-classes directives, so figured I'd just take the time to be sure things were reasonably stable continuing on.

Like you were saying in #12, there's nothing horrible about unsafe hacks in the test code if you're careful, and as you've seen the TestIO does definitely make use of them; just need to figure out how to extend that to the other two. Glad you found the copy-paste! I was having some slight trouble tracing the code. That solves the exception in lawsIO as well as plusLawsIO?

chessai commented 5 years ago

lawsIO does not encounter an exception, plusLawsIO still does. But, this is expected, since the monad plus laws require:

mzero >>= f  =  mzero
v >> mzero   =  mzero

and, mzero is failIO "mzero", which raises a user error exception.

chessai commented 5 years ago

see http://hackage.haskell.org/package/base-4.12.0.0/docs/src/GHC.IO.html#failIO

chessai commented 5 years ago

Note that MonadPlus has these laws, but Alternative does not test these two (since we're not necessarily in a Monad), so the alternative laws work just fine

chessai commented 5 years ago

BTW i just did a release to 0.2.2 that contains the correctness fixes for storable and the improved test suite

ag-eitilt commented 5 years ago

Ah, yeah, I must have been looking at the wrong surroundings when figuring out the error message, sorry! It did seem a bit strange that Monad would have been complaining about something from MonadPlus, but not have any problems with MonadIO. And thanks for the release!

Edit: Ah, I see what you were talking about with the copy-paste. The rest of this still holds.