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

QuickCheck swallows UserInterrupt in callbacks #32

Closed
sol opened this issue Nov 16, 2014 · 7 comments
Closed

QuickCheck swallows UserInterrupt in callbacks #32

sol opened this issue Nov 16, 2014 · 7 comments

Comments

@sol
Copy link
Contributor

sol commented Nov 16, 2014

Steps to reproduce

-- file Main.hs
import Test.QuickCheck
import Test.QuickCheck.State
import Test.QuickCheck.Property

main :: IO ()
main = quickCheckWithResult args (callback progress prop) >>= print
  where
    prop :: [Int] -> Bool
    prop xs = (reverse . reverse) xs == xs

    args = stdArgs {maxSuccess = 100000, chatty = False}

    progress = PostTest NotCounterexample $ \st _ -> (print $ numSuccessTests st)
$ ghci Main.hs
*Main> :main

While the test is running, press ctrl-c.

Expected result:

  • The program is terminated immediately (the UserInterrupt exception is propagated, so that the default exception handler kicks in)

Actual result:

With high probability

  • the test keeps running
  • the result is Success, with output set to "*** Exception in callback: user interrupt\n+++ OK, passed 100000 tests.\n"
@sol
Copy link
Contributor Author

sol commented Nov 16, 2014

This was added with b8570ad. @nick8325 do you remember the reasoning for it?

sol added a commit to sol/quickcheck that referenced this issue Nov 16, 2014
@sol
Copy link
Contributor Author

sol commented Nov 16, 2014

(related Hspec issue: hspec/hspec#208)

I use callbacks for progress reporting in Hspec. Not being able to interrupt a test is a show stopper for me. If we don't get that fixed soon, I have to disable progress reporting for quickcheck properties.

I consider this a quick fix. A proper solution can be done with async.

import Control.Concurrent.Async

-- | @safeTry@ evaluates given action and returns its result.  If an exception
-- occurs, the exception is returned instead.  Unlike `try` it is agnostic to
-- asynchronous exceptions.
safeTry :: IO a -> IO (Either SomeException a)
safeTry action = withAsync (action >>= evaluate) waitCatch

I haven't tried, but most likely using timeout in IO properties has issues, too (see https://www.fpcomplete.com/user/snoyberg/general-haskell/exceptions/catching-all-exceptions for details).

@sol
Copy link
Contributor Author

sol commented Jan 16, 2015

@nick8325 Can we do something about this? This is a major problem for Hspec (and anybody else who uses callbacks). Swallowing async exceptions is almost never ok.

@nick8325
Copy link
Owner

Hi Simon,

In fact QuickCheck is swallowing all exceptions in callbacks, which is clearly a bug even for synchronous exceptions. If a callback throws an exception, QuickCheck prints a warning but the test case still passes instead of failing. I'll fix this.

I'm not sure what the story should be for asynchronous exceptions. One problem is that we do want to catch ctrl-C, so that if the property loops the user can interrupt it and get a counterexample. However, I agree that it's fishy to catch other asynchronous exceptions. I think the principled thing would be to evaluate each test case in its own thread so as to isolate them as far as possible (e.g. in case a test case spawns a thread which throws an exception after a delay, and that exception ends up killing a later test) - I'll have a think about how to fit this into the test loop.

@nick8325
Copy link
Owner

nick8325 commented Feb 9, 2015

This should be fixed now!

@nick8325 nick8325 closed this as completed Feb 9, 2015
@nick8325
Copy link
Owner

Rather, all exceptions thrown in callbacks are turned into property failures, just like happens with exceptions thrown outside callbacks. I haven't yet made QuickCheck run in a separate thread to avoid catching asynchronous exceptions, so I'll reopen this ticket for the time being. But the behaviour you reported is fixed!

@nick8325
Copy link
Owner

As of b02aad9, QuickCheck no longer catches asynchronous exceptions.

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

2 participants