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

Divergent counterexamples in GHCi #144

Closed
thumphries opened this issue Jan 5, 2017 · 9 comments · Fixed by #148
Closed

Divergent counterexamples in GHCi #144

thumphries opened this issue Jan 5, 2017 · 9 comments · Fixed by #148

Comments

@thumphries
Copy link

Hi,

Just sharing a common weird behaviour when using QuickCheck in GHCi. This hits me very frequently, usually when a property takes a really long time to complete, or when the function being tested diverges (e.g. when working with parser combinators).

import Test.QuickCheck as QC

badFun :: String -> String
badFun s =
  case s of
    [] ->
      badFun "foo"
    _:xs ->
      reverse (badFun (xs ++ "foo"))

badProp :: QC.Property
badProp = QC.forAll QC.arbitrary (\s -> badFun s QC.=== s)

If we run badProp in GHCi, it doesn't terminate nor does it reach WHNF any time soon. We hit C-c to send UserInterrupt, which is caught and used to fail the property. QuickCheck tries to print the counterexample, which also doesn't reach WHNF any time soon, so we hit C-c again. The subsequent exception seems to be caught by GHCi, halting IO and restoring our control over the session:

λ> QC.quickCheck badProp
^C*** Failed! Exception: 'user interrupt' (after 1 test):
""
^CInterrupted.

... but in the background, the thunk continues to spin, pinning the CPU and racking up unbound gigabytes of memory.

In less contrived cases (this happens often to me when working with large generated structures and inefficient algorithms, or exceedingly slow IO operations), the test might eventually terminate, flooding stdout.

(Thanks for maintaining QuickCheck, I use it daily.)

@thumphries
Copy link
Author

I'm not sure why it keeps spinning in the background after being interrupted.

I've been trying to come up with an Async wrapper to catch a second C-c, but the ThreadKilled exception also gets caught and seems to have no effect.

@phadej
Copy link
Contributor

phadej commented Jan 5, 2017

I can reproduce this on linux with ghc8. If you run badFun "" == "" directly, C-c aborts the computation properly.

@phadej
Copy link
Contributor

phadej commented Jan 5, 2017

with

--- a/Test/QuickCheck/Exception.hs
+++ b/Test/QuickCheck/Exception.hs
@@ -56,7 +56,13 @@ tryEvaluate :: a -> IO (Either AnException a)
 tryEvaluate x = tryEvaluateIO (return x)
 
 tryEvaluateIO :: IO a -> IO (Either AnException a)
-tryEvaluateIO m = E.try (m >>= E.evaluate)
+tryEvaluateIO m = E.tryJust notAsync (m >>= E.evaluate)
+  where
+    notAsync :: E.SomeException -> Maybe AnException
+    notAsync e = case E.fromException e of
+        Just (E.SomeAsyncException _) -> Nothing
+        Nothing                       -> Just e
+
 --tryEvaluateIO m = Right `fmap` m

one can remove interactivity handling from QuickCheck (which imho is good idea), and then C-c interrupts quickCheck badProp properly.

@thumphries
Copy link
Author

thumphries commented Jan 5, 2017 via email

@phadej
Copy link
Contributor

phadej commented Jan 5, 2017

@thumphries I see.

Wasn't it better to have some limits on how much shrinked values / shrinks QuickCheck tries (and assume that shrink is somewhat productive, IMHO quite fair, why one would wtrite shrink x = x : shrink x)?

@thumphries
Copy link
Author

Wasn't it better to have some limits on how much shrinked values / shrinks QuickCheck tries

An upper bound on shrink attempts is a very good idea, especially if configurable via Args.

IMHO quite fair, why one would wtrite shrink x = x : shrink x?

I mostly use Jack, which can lead to some pretty large shrink trees if you hold it wrong. It doesn't make much sense to shrink more than 10,000 or 100,000 times.

@nick8325
Copy link
Owner

Handling ctrl-C is the only reason QuickCheck catches asynchronous exceptions as far as I remember. Previously it was also useful to catch StackOverflow but now GHC has no stack limit by default, I think.

I do find it useful, if my program hangs during testing, to be able to press ctrl-C and get a test case. On the other hand, there are some problems with it: shrinking is disabled, so you don't get a minimal test case, and if it's the generator that hangs then QuickCheck will hang printing the test case and you have to press ctrl-C a second time. So maybe it makes more sense to use timeout to find hanging bugs.

Anyway, I'm quite inclined to just remove the ctrl-C handling code from QuickCheck and only catch synchronous exceptions. This would make the code a bit simpler, and there are definitely problems with the way it works today.

@phadej
Copy link
Contributor

phadej commented Jan 11, 2017

@nick8325 I can make a PR out of the snippet above, and remove conditional code from shrinking?

@nick8325
Copy link
Owner

Yes, go ahead!

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

Successfully merging a pull request may close this issue.

3 participants