UnkindPartition / tasty

Modern and extensible testing framework for Haskell
640 stars 110 forks source link

Tests can catch timeouts #349

Closed ocharles closed 2 years ago

ocharles commented 2 years ago

The following test-suite will run forever, despite a timeout being requested:

module Main (main) where

import Control.Exception
import Test.Tasty
import Test.Tasty.HUnit

main :: IO ()
main = defaultMain $
  testCase "Swallow timeout" $ do
    res <- try (length [1..] @?= 0)
    case res of
      Left e@SomeException{} -> do
        print e
        length [1..] @?= 0
      Right ok -> putStrLn "Err"

I believe this could be fixed by using race (threadDelay timeout), but that would incur a dependency on async. If people agree that's a fix and are OK with the dependency, I can look at creating a PR.

This is a problem because currently tests using tasty-hedgehog can hang: https://github.com/qfpl/tasty-hedgehog/issues/64

Bodigrim commented 2 years ago

async is way too fat for a dependency, because it depends on hashable. tasty vendors in necessary parts of async as https://github.com/UnkindPartition/tasty/blob/master/core/Control/Concurrent/Async.hs. Maybe you can extend it with a copy of race combinator?..

ocharles commented 2 years ago

Sure, I can give that a try

ocharles commented 2 years ago

I actually have a feeling this is impossible. My reasoning is that any type of timeout in Haskell is going to require using threads, and cancelling the user-supplied IO action thread with an asynchronous exception. However, as we have no control over what this IO is, there's nothing stopping it from just swallowing that cancellation request and carrying on. Even if we use race, when the timeout happens the other thread will be cancelled, but as there's an implicit call to waitCatch, that will never terminate (because the child thread will refuse to die). I have a feeling the best we can do is what we currently do, and for people to be aware that catching asynchronous exceptions may well bite them. In this case Hedgehog needs to be fixed, because it has a tryAny in its main property runner (https://github.com/hedgehogqa/haskell-hedgehog/blob/master/hedgehog/src/Hedgehog/Internal/Runner.hs#L229).

ocharles commented 2 years ago

I'm now of the opinion that there's nothing tasty can do here beyond what it's currently doing. I'll fix hedgehog and we'll just have to play whack-a-mole with any other libraries that are catching async exceptions.