Add a proper test suite

[treeowl]

I just went to try to add QuickCheck or SmallCheck properties to the test suite, and discovered that there isn't a proper test suite. Ack! No wonder the Foldable instance for Array managed to be wrong for so long! I'll be happy to write some properties, but I don't have the time right now to set up the framework.

[RyanGlScott]

Alas, this is not as easy to accomplish as it sounds. QuickCheck in particular depends on tf-random, which in turn depends on primitive itself, so you quickly run into haskell/cabal#5200 if you try to make primitive's test suite depend on it.

That being said, there is a relatively serviceable workaround: turn the test suite into its own .cabal file and make it its own project, distinct from primitive itself. That way, build tools can first build primitive and then build tf-random et al. using the locally built primitive, avoiding any dependency cycles. I've used this hack before to make a test suite for transformers-compat, and it seems to work okay.

[RyanGlScott]

I've opened #90 to implement the suggestion in #89 (comment). Once that is merged (pending Travis giving a green check mark), that should make setting up QuickCheck tests far, far easier.

[treeowl]

Great! Have you had a chance to look at my folds and traversals? On Mar 19, 2018 10:02 AM, "Ryan Scott" <[email protected]> wrote: I've opened #90 <#90> to implement the suggestion in #89 (comment) <#89 (comment)>. Once that is merged (pending Travis giving a green check mark), that should make setting up QuickCheck tests far, far easier. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#89 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABzi_cjSqtw0ZCpwiNickjoOADr4_uY_ks5tf7qPgaJpZM4SvpGw> .

[andrewthad]

@RyanGlScott Thanks for moving the test suite into its own project. I'm going to work on adding some property tests.

[treeowl]

Andrew, please let me know if/when you start, and with what. I'll do the same.

…

On Mar 20, 2018 1:40 PM, "Andrew Martin" ***@***.***> wrote: @RyanGlScott <https://github.com/ryanglscott> Thanks for moving the test suite into its own project. I'm going to work on adding some property tests. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#89 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABzi_dR0p0grV5A3WHvVtZEO304c7KtAks5tgT8DgaJpZM4SvpGw> .

[andrewthad]

I'm working on it now. I'm going to use tasty + quickcheck + quickcheck-classes just as an easy way to get access to some of the typeclass property tests we're interested in.

[RyanGlScott]

Thanks @andrewthad!

Do be aware that primitive has a support window back to GHC 7.4, so using quickcheck-classes in its current state would shorten that window somewhat (as it only supports GHC 7.8 and later at the moment).

[treeowl]

Sounds great! I'll be happy to help expand it once you get it going.

…

On Mar 20, 2018 1:52 PM, "Andrew Martin" ***@***.***> wrote: I'm working on it now. I'm going to use tasty + quickcheck + quickcheck-classes just as an easy way to get access to some of the typeclass property tests we're interested in. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#89 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABzi_e1EAc5dwWaq9pvocfWMfy_0v2ELks5tgUHSgaJpZM4SvpGw> .

[andrewthad]

@RyanGlScott Good point. Let me see if I can get quickcheck-classes to support older GHCs. I don't think there's any fundamental reason that it can't.

[andrewthad]

It was more difficult than I expected, but I now have quickcheck-classes building with all GHCs back to 7.4. I'll get the tests together now.

[treeowl]

Huzzah! Unfortunately, quickcheck-classes is not yet really up to the job. It's missing Traversable altogether (which I have a PR to reimplement). It's also missing the foldl1 and foldr1 methods; we had some trouble with foldl1! I also don't really understand its tests. I would think the Monad laws, for instance, would be tested using Test.QuickCheck.Function, but they're not. Why not something roughly like this:

monadAssoc ::  ... => m A -> Fun A (m B) -> Fun B (m C) -> Property
monadAssoc m f g = (m >>= \x -> apply f x >>= apply g) `equivalent`
                         ((m >>= apply f) >>= apply g)

For Traversable, I suggest you consider the answers to my long-ago SO question on the topic.

[andrewthad]

I've got a start on it here: #99. Already, we can see that it fails the laws for Functor, which is expected.

@treeowl I agree that it's currently missing several things. I had never done Traverable since those are normally derived. I had only done Foldable (also normally easy to derive) as an exercise since the strict variants of the fold functions are tricky to test correctly. I've never used Test.QuickCheck.Function, and it's almost certainly a better way to test some of the monad properties than what I came up with.

[cartazio]

I’m nearly done with getting random updated in shape. But I could pull out split mix so we have a copy in primitive too. That would address circular deps perhaps ?

…

On Tue, Mar 20, 2018 at 5:22 PM Andrew Martin ***@***.***> wrote: I've got a start on it here: #99 <#99>. Already, we can see that it fails the laws for Functor, which is expected. @treeowl <https://github.com/treeowl> I agree that it's currently missing several things. I had never done Traverable since those are normally derived. I had only done Foldable (also normally easy to derive) as an exercise since the strict variants of the fold functions are tricky to test correctly. I've never used Test.QuickCheck.Function, and it's almost certainly a better way to test some of the monad properties than what I came up with. — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#89 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAAQwmFg9U6o8ZPS0FpycgEYhM2czOdwks5tgXMRgaJpZM4SvpGw> .

[treeowl]

For Traversable, you need to test the identity law, which should be easy, and the composition law, which will be harder. Also, like checkers, you'll want to verify that foldMapDefault and fmapDefault match foldMap and fmap. If you're adding detailed strictness checks as well (see ChasingBottoms), you'll probably want to check the naturality law too, but that sort of detail can surely wait.

[treeowl]

@cartazio, I don't see how that could help. QuickCheck has loads of dependencies. We could try a different test framework, I suppose.

[treeowl]

@andrewthad, are we ready to close this, or are there still non-trivial functions that lack adequate tests?

[andrewthad]

We still aren't testing the following typeclass instances: MonadZip, Alternative, MonadPlus, and MonadFix. Of those, only MonadFix has what I would describe as a trivial implementation. All the others are at least as complicated as fmap. However, I would prefer closing out this issue, since the objective "Add a proper test suite" is complete, and opening a new one instead to track progress on those.

[treeowl]

Done! I opened up an issue to track test suite expansion.