Created Started Cancelled
Fix the asynchronous runner hanging on exceptions that escape a test Fixes #110 ## The bug A test that throws an exception classified as asynchronous makes the asynchronous runner hang. The minimal repro from @bristermitten: ```haskell data Boom = Boom deriving (Show) instance Exception Boom where toException = asyncExceptionToException fromException = asyncExceptionFromException main = sydTest $ it "boom" (throwIO Boom :: IO ()) ``` Before this change, with the asynchronous runner: ``` repro: thread blocked indefinitely in an MVar operation ``` After, matching what the synchronous runner has always done: ``` repro: Boom ``` This shape is not exotic: `effectful`'s `Error` effect uses exactly the same `asyncExceptionToException` design, which is how @bristermitten ran into it. ## The regression test `sydtest-test/test/Test/Syd/Runner/AsynchronousSpec.hs` defines an exception whose `toException` is `asyncExceptionToException`, runs a one-test forest that throws it, and asserts that the runner re-throws it instead of hanging. One test per asynchronous entry point (`runSpecForestAsynchronously` and `runSpecForestInterleavedWithOutputAsynchronously`), each wrapped in a 10 second `System.Timeout.timeout` so a regression shows up as a failure rather than a stuck suite. Both tests fail by timing out on `development` and pass with the fix. ## The root cause `Test.Syd.Run.exceptionHandlers` re-throws `SomeAsyncException` instead of turning it into a test result. That is deliberate (#80): swallowing it would break `race`/`concurrently` and Ctrl-C. In the asynchronous runner, that re-thrown exception escapes `job` before it reaches `putMVar var result`, so: - the worker thread dies, and `withJobQueueWorkers` uses `withAsync` without `link`, so nothing notices; - the runner's `blockUntilDone` sees the worker's count released and carries on, so the runner finishes normally; - the printer/waiter thread is left in `race (readMVar failFastVar) (takeMVar var)` on a result nobody will ever put, so `concurrently` waits forever on it. ## The fix The fail-fast `MVar ()` becomes an `MVar Abort` that says *why* the suite is stopping: `AbortFailFast` or `AbortException SomeException`. - A worker records an exception escaping the test in that var before re-throwing it. - The printer and waiter now share `awaitTestResult`, which races the abort var against the result var: fail-fast still means "no result for this test", an exception is re-thrown. - The re-throw makes `concurrently` cancel the runner, so the exception leaves `runSpecForest*Asynchronously` the way it leaves the synchronous runners. I kept the existing behaviour of letting the exception propagate rather than reporting it as a test failure, so this does not walk back #80 and the two runners now agree. If you would rather see such a test reported as a failure, that is a separate decision and a bigger change. ### One extra hang fixed along the way `putMVar failFastVar ()` is now `tryPutMVar`. With `--fail-fast` and two tests failing at the same time, the second worker blocked on an already full `MVar`, which left it counted as working forever and hung the runner at its next `waitForWorkersDone`. There is no regression test for this one: it needs two workers to fail inside the same window, which I could not make deterministic without adding hooks to the runner. ## Things to check manually - `nix flake check` passes here. During one run the `sydtest-test` derivation failed, but the kernel log showed a global OOM kill in that window and the same derivation built clean afterwards; worth a second look if you see it too. - The `Abort`/`abortVar` naming: the var is no longer only about fail-fast, so I renamed it, which is most of the diff's line count. - Whether re-throwing rather than reporting is the behaviour you want long term (see above). - Fail-fast behaviour in the asynchronous runner in general, since `tryPutMVar` changes which worker wins the race to set it (nothing reads the payload differently, but it is the one behavioural change outside the bug).