Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bug? Importing Tasty breaks SmallCheck's testing of higher order functions #71

Closed
rudymatela opened this issue Jun 24, 2014 · 9 comments

Comments

@rudymatela
Copy link
Contributor

I think I found a bug on Tasty, importing it will export the show instance for functions (a->b) on Text.Show.Functions. Making it impossible to test higher order properties for custom types on SmallCheck.

Here's a minimal example with a simple boolean type and the ill-conceived property that all operations on booleans are associative.

{-# LANGUAGE FlexibleInstances, MultiParamTypeClasses #-}

import Test.Tasty             -- Comment this for it to work
import Test.SmallCheck
import Test.SmallCheck.Series

data Boolean = T | F deriving (Show, Eq)

instance (Monad m) => Serial m Boolean where
  series = cons0 T \/ cons0 F

instance Monad m => CoSerial m Boolean where
  coseries rs =
    rs >>- \r1 ->
    rs >>- \r2 ->
    return $ \x -> case x of
                     T -> r1
                     F -> r2

prop_assoc :: (Boolean -> Boolean -> Boolean) -> Boolean -> Boolean -> Boolean -> Bool
prop_assoc op = \x y z -> (x `op` y) `op` z == x `op` (y `op` z)

main = smallCheck 2 prop_assoc

When running the program above, I get the following error:

example.hs:23:8:
    Overlapping instances for Show (Boolean -> Boolean -> Boolean)
      arising from a use of ‘smallCheck’
    Matching instances:
      instance [safe] Show (a -> b) -- Defined in ‘Text.Show.Functions’
      instance [overlap ok] (Serial Data.Functor.Identity.Identity a,
                             Show a, Show b) =>
                            Show (a -> b)
        -- Defined in ‘Test.SmallCheck.Series’
    In the expression: smallCheck 2 prop_assoc
    In an equation for ‘main’: main = smallCheck 2 prop_assoc

If I comment the line where it says import Test.Tasty, the program works, and shows the correct counterexample:

Failed test no. 10.
there exist {T->{T->F;F->T};F->{T->T;F->T}} T T F such that
  condition is false

Is this a bug, or I am using Tasty with SmallCheck in some wrong way?

@UnkindPartition
Copy link
Owner

This probably means that some of Tasty's transitive dependencies imported Text.Show.Functions.

Which, unfortunately, breaks smallcheck.

I plan to get rid of that show instance in smallcheck eventually, but probably won't have time for it anytime soon. You could try to find what package uses Text.Show.Functions and file a bug against it,

@UnkindPartition
Copy link
Owner

The offending package was regex-tdfa. This is now fixed, by depending on my fork of regex-tdfa and disabling that instance there.

UnkindPartition added a commit that referenced this issue Jul 5, 2014
@nomeata
Copy link
Contributor

nomeata commented Aug 3, 2014

Unfortunately, such a fork prevents an upgrade of tasty in Debian. Is there a plan to make tasty and regex-tdfa compatible in the near future again?

@UnkindPartition
Copy link
Owner

Well, they are API-compatible, so you could compile tasty against regex-tdfa.

Except you'll re-introduce this bug for your users.

As I said above, I plan to get rid of that show instance in smallcheck eventually, but probably won't have time for it anytime soon.

Alternatively, you could ship my fork of regex-tdfa in place of the original version in Debian.
I believe my changes don't break anything, just remove some debugging output, but I'd advise you to review them if you decide to go down this path.

I'm sorry that this makes your life harder.

@nomeata
Copy link
Contributor

nomeata commented Aug 3, 2014

Thanks for the overview! I’ll go with tasty 0.8.1.2 for now and revisit this some other time.

Are the regex-tdfa maintainers aware of your improvements?

@UnkindPartition
Copy link
Owner

Probably not; I haven't contacted them.

@nomeata
Copy link
Contributor

nomeata commented Aug 3, 2014

Sigh. You seriously put forks on hackage without even bothing to send a quick message to the maintainers?

Anyways, I sent them a mail, maybe there can be one regex-tdfa soon again.

@UnkindPartition
Copy link
Owner

The fork had been made before this issue arose. It was when regex-tdfa remained broken with 7.8 for a significant amount of time, despite many emails sent to the maintainer. After that incident, the maintainer didn't take any steps to prevent such things from happening in the future (such as appointing a backup maintainer). Unless that happens, there's little incentive for me to switch back to regex-tdfa.

Of course, how you package it in Debian is an orthogonal question. You enjoy a possibility of applying quick fixes anytime you need them. Hackage allows it only through forking.

@nomeata
Copy link
Contributor

nomeata commented Aug 4, 2014

The fork had been made before this issue arose. It was when regex-tdfa remained broken with 7.8 for a significant amount of time, despite many emails sent to the maintainer.

Fair enough.

neongreen pushed a commit to ChrisKuklewicz/regex-tdfa that referenced this issue Apr 28, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants