diff --git a/sydtest-test/sydtest-test.cabal b/sydtest-test/sydtest-test.cabal index 1527dfb..8e02f7a 100644 --- a/sydtest-test/sydtest-test.cabal +++ b/sydtest-test/sydtest-test.cabal @@ -88,6 +88,7 @@ test-suite sydtest-test Test.Syd.AroundSpec Test.Syd.DescriptionsSpec Test.Syd.DiffSpec + Test.Syd.ExceptionSpec Test.Syd.ExpectationSpec Test.Syd.FootgunSpec Test.Syd.GoldenSpec diff --git a/sydtest-test/test/Test/Syd/ExceptionSpec.hs b/sydtest-test/test/Test/Syd/ExceptionSpec.hs new file mode 100644 index 0000000..6835ca1 --- /dev/null +++ b/sydtest-test/test/Test/Syd/ExceptionSpec.hs @@ -0,0 +1,39 @@ +{-# LANGUAGE NumericUnderscores #-} + +module Test.Syd.ExceptionSpec where + +import Control.Concurrent (threadDelay) +import Control.Concurrent.Async +import Control.Exception (AsyncException, try) +import Control.Exception.Base (throwIO) +import Data.Time.Clock +import Test.Syd +import Test.Syd.OptParse (defaultSettings) + +spec = describe "exception handling" $ do + it "stops immediatly with async" $ do + -- Tests that when an async exception is sent to sydTest (for example, + -- ctrl-c), it behaves as expected and terminates immediately and does not + -- do anything surprising. + startTime <- liftIO getCurrentTime + + -- Runs two threads, one will be done in 1s, and the other is a test suite + -- with one test which should run for 10s. + -- When the first threads terminate, it will throw AsyncCancel in the + -- sydTest test suite, which should terminate asap. + -- This will be checked using the timer t. That's fragile, but we know that: + -- + -- t should be more than 100ms (because of the first threadDelay) + -- t should be no much more than 100ms. Especially, it should not be 10s + -- (waiting for the complete threadDelay in the test suite) or even more if + -- the exception is completly ignored (or test is retried) + race (threadDelay 100_000) $ do + sydTest $ do + it "is a dumb slow test" $ do + threadDelay 10_000_000 + + endTime <- liftIO getCurrentTime + + -- Less than 1 second + (endTime `diffUTCTime` startTime) `shouldSatisfy` (< 1) + pure () diff --git a/sydtest/CHANGELOG.md b/sydtest/CHANGELOG.md index 3cf51d8..42c359d 100644 --- a/sydtest/CHANGELOG.md +++ b/sydtest/CHANGELOG.md @@ -1,5 +1,10 @@ # Changelog +### Changed + +- sydtest won't crash anymore or behave weirdly or leak ressource in repl when + interrupted by ctrl-C. See discussion here: https://github.com/NorfairKing/sydtest/issues/80#issuecomment-2286517212 + ## [0.17.0.0] - 2024-08-04 ### Changed diff --git a/sydtest/src/Test/Syd/Run.hs b/sydtest/src/Test/Syd/Run.hs index ad86c68..5fd23c6 100644 --- a/sydtest/src/Test/Syd/Run.hs +++ b/sydtest/src/Test/Syd/Run.hs @@ -374,8 +374,13 @@ runGoldenTestWithArg createGolden TestRunSettings {..} _ wrapper = do exceptionHandlers :: [Handler (Either SomeException a)] exceptionHandlers = - [ -- Re-throw AsyncException, otherwise execution will not terminate on SIGINT (ctrl-c). - Handler (\e -> throwIO (e :: AsyncException)), + [ -- Re-throw SomeAsyncException, otherwise execution will not terminate on SIGINT (ctrl-c). + -- This is also critical for correctness, because library such as async + -- uses this signal for `concurrently`, and `race`, ..., and if we ignore + -- this exception, we can end in a context where half of the logic has + -- stopped and yet we continue. + -- See https://github.com/NorfairKing/sydtest/issues/80 + Handler (\e -> throwIO (e :: SomeAsyncException)), -- Catch all the rest Handler (\e -> return $ Left (e :: SomeException)) ]