-
-
Notifications
You must be signed in to change notification settings - Fork 25
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
Sydtest exception handling: crash / lock / ... #80
Comments
Wow thank you for the detailed report!
I think you got that exactly right. Yes, PR please. |
Additionally, I observed the following behaviors related to exceptions:
E.g. the folliwng testsuite: import Test.Syd
import Control.Concurrent
import Control.Exception
main = sydTest $ do
it "is slow" $ do
print "is slow"
e <- try $ threadDelay 10_000_000
print "end of delay"
print (e :: Either SomeException ())
case e of
Left e -> throwIO e
Right () -> pure ()
pure () The test is supposed to wait for 10s. If
However, if
(That's a documented GHC rts behavior, but I'm unable to find the documentation). I suppose that sydtest is catching ALL possible exception and retries. From my PoV, it should ignore the asyrc ones and terminate the test runner immediatly. Note that it is surprising that if the test suite survives one retry, it does not display the final results:
|
Wow cool finds! The retry logic was added after the async exception handling so I'm not surprised it doesn't work well together. |
I found the problem with import Test.Syd
data Cheval = Cheval String deriving (Eq)
instance Show Cheval where
show x = error "NO"
main = sydTest $ do
it "fails" $ do
Cheval "troll" `shouldBe` Cheval "troll2"
Here, the
(No final report). |
So, my plans in a MR about exceptions will be to: a) Add an exception handler when sydtest displays the result of the tests and uses values from the "user side", which may contain hidden exceptions |
I've wrote the following test suite:
Which shows the problems and should hopefully be fine once the problems are fixed. Note that I also observed that:
However, I'm surprised, because tests are run in
So as a first observation, it seems that the problem was already anticipated and taken care of. I need to read the precise spec of
|
This fix is trivial, |
@guibou I'm impressed that you're run into so many of these issues already. I'm open to any improvements, especially when you have such a nice test suite along with them. |
Ensures that the diff computed when `shouldBe` test are failing is not taking more than 2s, otherwise do not display a diff. This fix NorfairKing#92 in which we observed that diff can take an arbitrary amount of time, up to a few minutes or hours. This implementations changes the API of a few internal functions (see the `Changelog` entries), but no change on the "common" API. - Diff time is taken into account in the time of the test. Note that the diff can be computed multiples time during flaky detection. - This also fix an issue discussed in NorfairKing#80, if the values being diffed contains lazy exception, it won't crash the test suite anymore, but instead, the exception will be raised in the test context.
Ensures that the diff computed when `shouldBe` test are failing is not taking more than 2s, otherwise do not display a diff. This fix NorfairKing#92 in which we observed that diff can take an arbitrary amount of time, up to a few minutes or hours. This implementations changes the API of a few internal functions (see the `Changelog` entries), but no change on the "common" API. - Diff time is taken into account in the time of the test. Note that the diff can be computed multiples time during flaky detection. - This also fix an issue discussed in NorfairKing#80, if the values being diffed contains lazy exception, it won't crash the test suite anymore, but instead, the exception will be raised in the test context.
Ensures that the diff computed when `shouldBe` test are failing is not taking more than 2s, otherwise do not display a diff. This fix NorfairKing#92 in which we observed that diff can take an arbitrary amount of time, up to a few minutes or hours. This implementations changes the API of a few internal functions (see the `Changelog` entries), but no change on the "common" API. - Diff time is taken into account in the time of the test. Note that the diff can be computed multiples time during flaky detection. - This also fix an issue discussed in NorfairKing#80, if the values being diffed contains lazy exception, it won't crash the test suite anymore, but instead, the exception will be raised in the test context.
Ensures that the diff computed when `shouldBe` test are failing is not taking more than 2s, otherwise do not display a diff. This fix NorfairKing#92 in which we observed that diff can take an arbitrary amount of time, up to a few minutes or hours. This implementations changes the API of a few internal functions (see the `Changelog` entries), but no change on the "common" API. - Diff time is taken into account in the time of the test. Note that the diff can be computed multiples time during flaky detection. - This also fix an issue discussed in NorfairKing#80, if the values being diffed contains lazy exception, it won't crash the test suite anymore, but instead, the exception will be raised in the test context.
This is related to NorfairKing#80 The current exception handler for test was incorrectly catching async exception (the handler for `AsyncException` was not enough, the "parent" of the async exception hierarchy is `SomeAsyncException`). This lead to surprising behaviors with ctrl-C. when receiving a `ctrl-c` (e.g. an interruption of the test-suite by the user), the test runner was cancelled, cancelling the print loop of the test and cancelling all workers. However, the exception was caught in workers as a "test-failure", leading to it being ignored and the different tests retrieved and meaning that the test runner survived the interruption. They will hence continue their life (e.g. purging the job queue). It was sometime leading to a lot of delay before ending the process, or even sometime other errors related to the different involved `MVar` (when no producer / consumer detection detects the error. Note that a second ctrl-c would cancels the program execution completely (Because that's how the RTS behaves). HOWEVER, the behavior is completely different in the repl, because the repl won't terminate itself on second ctrl-C. Hence the worker threads are leaked and continues running, and even survives the repl reloads. In the best case, that's a waste of resources and CPU time, but if you happen to rerun new sydtest, it can stacks and some tests may run concurrently with the "ghost" version of themself, which may or may not work ;)
This is related to NorfairKing#80 The current exception handler for test was incorrectly catching async exception (the handler for `AsyncException` was not enough, the "parent" of the async exception hierarchy is `SomeAsyncException`). This lead to surprising behaviors with ctrl-C. when receiving a `ctrl-c` (e.g. an interruption of the test-suite by the user), the test runner was cancelled, cancelling the print loop of the test and cancelling all workers. However, the exception was caught in workers as a "test-failure", leading to it being ignored and the different tests retrieved and meaning that the test runner survived the interruption. They will hence continue their life (e.g. purging the job queue). It was sometime leading to a lot of delay before ending the process, or even sometime other errors related to the different involved `MVar` (when no producer / consumer detection detects the error. Note that a second ctrl-c would cancels the program execution completely (Because that's how the RTS behaves). HOWEVER, the behavior is completely different in the repl, because the repl won't terminate itself on second ctrl-C. Hence the worker threads are leaked and continues running, and even survives the repl reloads. In the best case, that's a waste of resources and CPU time, but if you happen to rerun new sydtest, it can stacks and some tests may run concurrently with the "ghost" version of themself, which may or may not work ;)
This is related to NorfairKing#80 The current exception handler for test was incorrectly catching async exception (the handler for `AsyncException` was not enough, the "parent" of the async exception hierarchy is `SomeAsyncException`). This lead to surprising behaviors with ctrl-C. when receiving a `ctrl-c` (e.g. an interruption of the test-suite by the user), the test runner was cancelled, cancelling the print loop of the test and cancelling all workers. However, the exception was caught in workers as a "test-failure", leading to it being ignored and the different tests retrieved and meaning that the test runner survived the interruption. They will hence continue their life (e.g. purging the job queue). It was sometime leading to a lot of delay before ending the process, or even sometime other errors related to the different involved `MVar` (when no producer / consumer detection detects the error. Note that a second ctrl-c would cancels the program execution completely (Because that's how the RTS behaves). HOWEVER, the behavior is completely different in the repl, because the repl won't terminate itself on second ctrl-C. Hence the worker threads are leaked and continues running, and even survives the repl reloads. In the best case, that's a waste of resources and CPU time, but if you happen to rerun new sydtest, it can stacks and some tests may run concurrently with the "ghost" version of themself, which may or may not work ;)
This is related to NorfairKing#80 The current exception handler for test was incorrectly catching async exception (the handler for `AsyncException` was not enough, the "parent" of the async exception hierarchy is `SomeAsyncException`). This lead to surprising behaviors with ctrl-C. when receiving a `ctrl-c` (e.g. an interruption of the test-suite by the user), the test runner was cancelled, cancelling the print loop of the test and cancelling all workers. However, the exception was caught in workers as a "test-failure", leading to it being ignored and the different tests retrieved and meaning that the test runner survived the interruption. They will hence continue their life (e.g. purging the job queue). It was sometime leading to a lot of delay before ending the process, or even sometime other errors related to the different involved `MVar` (when no producer / consumer detection detects the error. Note that a second ctrl-c would cancels the program execution completely (Because that's how the RTS behaves). HOWEVER, the behavior is completely different in the repl, because the repl won't terminate itself on second ctrl-C. Hence the worker threads are leaked and continues running, and even survives the repl reloads. In the best case, that's a waste of resources and CPU time, but if you happen to rerun new sydtest, it can stacks and some tests may run concurrently with the "ghost" version of themself, which may or may not work ;)
This is related to NorfairKing#80 The current exception handler for test was incorrectly catching async exception (the handler for `AsyncException` was not enough, the "parent" of the async exception hierarchy is `SomeAsyncException`). This lead to surprising behaviors with ctrl-C. when receiving a `ctrl-c` (e.g. an interruption of the test-suite by the user), the test runner was cancelled, cancelling the print loop of the test and cancelling all workers. However, the exception was caught in workers as a "test-failure", leading to it being ignored and the different tests retrieved and meaning that the test runner survived the interruption. They will hence continue their life (e.g. purging the job queue). It was sometime leading to a lot of delay before ending the process, or even sometime other errors related to the different involved `MVar` (when no producer / consumer detection detects the error. Note that a second ctrl-c would cancels the program execution completely (Because that's how the RTS behaves). HOWEVER, the behavior is completely different in the repl, because the repl won't terminate itself on second ctrl-C. Hence the worker threads are leaked and continues running, and even survives the repl reloads. In the best case, that's a waste of resources and CPU time, but if you happen to rerun new sydtest, it can stacks and some tests may run concurrently with the "ghost" version of themself, which may or may not work ;)
This is related to NorfairKing#80 The current exception handler for test was incorrectly catching async exception (the handler for `AsyncException` was not enough, the "parent" of the async exception hierarchy is `SomeAsyncException`). This lead to surprising behaviors with ctrl-C. when receiving a `ctrl-c` (e.g. an interruption of the test-suite by the user), the test runner was cancelled, cancelling the print loop of the test and cancelling all workers. However, the exception was caught in workers as a "test-failure", leading to it being ignored and the different tests retrieved and meaning that the test runner survived the interruption. They will hence continue their life (e.g. purging the job queue). It was sometime leading to a lot of delay before ending the process, or even sometime other errors related to the different involved `MVar` (when no producer / consumer detection detects the error. Note that a second ctrl-c would cancels the program execution completely (Because that's how the RTS behaves). HOWEVER, the behavior is completely different in the repl, because the repl won't terminate itself on second ctrl-C. Hence the worker threads are leaked and continues running, and even survives the repl reloads. In the best case, that's a waste of resources and CPU time, but if you happen to rerun new sydtest, it can stacks and some tests may run concurrently with the "ghost" version of themself, which may or may not work ;)
This is related to #80 The current exception handler for test was incorrectly catching async exception (the handler for `AsyncException` was not enough, the "parent" of the async exception hierarchy is `SomeAsyncException`). This lead to surprising behaviors with ctrl-C. when receiving a `ctrl-c` (e.g. an interruption of the test-suite by the user), the test runner was cancelled, cancelling the print loop of the test and cancelling all workers. However, the exception was caught in workers as a "test-failure", leading to it being ignored and the different tests retrieved and meaning that the test runner survived the interruption. They will hence continue their life (e.g. purging the job queue). It was sometime leading to a lot of delay before ending the process, or even sometime other errors related to the different involved `MVar` (when no producer / consumer detection detects the error. Note that a second ctrl-c would cancels the program execution completely (Because that's how the RTS behaves). HOWEVER, the behavior is completely different in the repl, because the repl won't terminate itself on second ctrl-C. Hence the worker threads are leaked and continues running, and even survives the repl reloads. In the best case, that's a waste of resources and CPU time, but if you happen to rerun new sydtest, it can stacks and some tests may run concurrently with the "ghost" version of themself, which may or may not work ;)
This is related to #80 The current exception handler for test was incorrectly catching async exception (the handler for `AsyncException` was not enough, the "parent" of the async exception hierarchy is `SomeAsyncException`). This lead to surprising behaviors with ctrl-C. when receiving a `ctrl-c` (e.g. an interruption of the test-suite by the user), the test runner was cancelled, cancelling the print loop of the test and cancelling all workers. However, the exception was caught in workers as a "test-failure", leading to it being ignored and the different tests retrieved and meaning that the test runner survived the interruption. They will hence continue their life (e.g. purging the job queue). It was sometime leading to a lot of delay before ending the process, or even sometime other errors related to the different involved `MVar` (when no producer / consumer detection detects the error. Note that a second ctrl-c would cancels the program execution completely (Because that's how the RTS behaves). HOWEVER, the behavior is completely different in the repl, because the repl won't terminate itself on second ctrl-C. Hence the worker threads are leaked and continues running, and even survives the repl reloads. In the best case, that's a waste of resources and CPU time, but if you happen to rerun new sydtest, it can stacks and some tests may run concurrently with the "ghost" version of themself, which may or may not work ;)
Ensures that the diff computed when `shouldBe` test are failing is not taking more than 2s, otherwise do not display a diff. This fix #92 in which we observed that diff can take an arbitrary amount of time, up to a few minutes or hours. This implementations changes the API of a few internal functions (see the `Changelog` entries), but no change on the "common" API. - Diff time is taken into account in the time of the test. Note that the diff can be computed multiples time during flaky detection. - This also fix an issue discussed in #80, if the values being diffed contains lazy exception, it won't crash the test suite anymore, but instead, the exception will be raised in the test context.
The following test suite:
Will crash the complete test suite:
Replace
beforeAll
bybefore
and you'll get:Using the following changes:
The following test suites now pass:
With the following result:
A few notes:
before
andbeforeAll
is that the code forbefore
is executed during theit
execution, when the code forbeforeAll
is executed during the tree walking.SomeException
for proper excption handling.So my question is:
Thank you.
The text was updated successfully, but these errors were encountered: