phile314 / tasty-silver

A fancy test runner for tasty and support for golden tests.
MIT License
9 stars 3 forks source link

Abort gracefully on ctrl-c #2

Closed phile314 closed 4 years ago

phile314 commented 9 years ago

Should abort all currently running tests, but make sure everything is cleaned up.

andreasabel commented 4 years ago

I realized some oddity when interrupting a test with Ctrl-c: goldenTestIO1 seems to call the supplied differ (of type Text -> Text -> IO GDiff), but the interaction does not actually present a diff to the user (instead, as expected, the test run is aborted).

The fact that the differ is called in case of Ctrl-c is inconvenient for me, and the call seems superfluous when anyway the test run is aborted. Why inconvenient? I placed a side effect into the differ to touch the input file when a diff has occurred, to freshen the test so it will be run earlier (I order tests by time, most recently touched first). However, I do not want to move up a test only because the test suite was interrupted.

Do you have an idea where this behavior comes from, @phil314, and how it could be fixed or worked around?

phile314 commented 4 years ago

I strongly suspect that this could be related to threading. The differ itself is called when executing the test itself ( https://github.com/phile314/tasty-silver/blob/master/Test/Tasty/Silver/Internal.hs#L105 ), whereas showing the diff is done in the UI code ( https://github.com/phile314/tasty-silver/blob/master/Test/Tasty/Silver/Interactive.hs#L382 ). Also, the tests are run in different threads using async ( http://hackage.haskell.org/package/tasty-1.2.3/docs/src/Test.Tasty.Parallel.html#runInParallel ). The tests are also run as fast as possible in the background even if the UI is still waiting for user input.

Can you maybe clarify when exactly you press ctrl-c? Is it at a random point while tests are being executed, while a diff is being shown or while it waits for user input?

andreasabel commented 4 years ago

I am pressing ctrl-c during a sequence of successful tests (thus, there is no user interaction or showing of diffs). I.e., if I hadn't interrupted, the differ would return "no diff".

I suppose getActual is running the actual test in: https://github.com/phile314/tasty-silver/blob/b39f3afead39933e0753522c53572ba769dc27aa/Test/Tasty/Silver/Internal.hs#L103 If getActual is interrupted, cmp should not be called. Maybe this would fix my problem.

phile314 commented 4 years ago

Can you show the exact code of your differ?

What is the output if you add a putStrLn to the differ? In what order is it called?

andreasabel commented 4 years ago

I now replaced the call to touchFile by a putStrLn:


  resDiff :: T.Text -> T.Text -> IO GDiff
  resDiff t1 t2
    | stripConsecutiveWhiteSpace t1 == stripConsecutiveWhiteSpace t2 = return Equal
    | otherwise = do
        -- Andreas, 2020-06-09, issue #4736
        -- If the output has changed, the test case is "interesting"
        -- regardless of whether the golden value is updated or not.
        -- Thus, we touch the agdaFile to have it sorted up in the next
        -- test run.
        putStrLn $ "TOUCHING " ++ agdaFile
        -- touchFile agdaFile
        return $ DiffText Nothing t1 t2

If I run the testsuite with,

this is the output:

$ make BUILD_DIR=./dist-2.6.2-fast fail
Using JS node binary at /usr/local/bin/node
all
  Fail
    AbsurdPatternNotUnused:                            TOUCHING test/Fail/AbsurdPatternNotUnused.agda
Golden value differs from actual value.
                                                       Accept actual value as new golden value? [yn] y
                                                       OK
                                                       Updated golden value.
    InferRecordTypes-2:                                OK (0.42s)
    Issue1015OnlyRecord:                               OK (0.64s)
    TacticFail:                                        ^CTO[C?H2I5NhGmake: *** [Makefile:337: fail] Interrupt: 2

One can see at the end that the printing of TOUCHING is interleaved with some other printing.

In some other run, I did not see a print of TOUCHING... when I interrupt.

phile314 commented 4 years ago

Hm, this looks to be a bit a can of worms....

Apparently on CTRL-C a SIGINT signal is sent to all process in the shell process group. This includes both the tasty test runner, but also any Agda process that has been spawned by the tests. Now I suspect it could be that the Agda process is killed first, but the tasty test runner is left running slightly longer. In that case, the test runner would observe the terminated Agda process and will try to update the golden result.

Maybe putting the agda processes in a new process group using the create_group option of CreateProcess could work. On the other hand, we then need to kill the Agda process ourselves or else they could run indefinitely.

(Hm, maybe just setting the flag and bracketing the agda process with a kill call could work?)

andreasabel commented 4 years ago

I create PR https://github.com/agda/agda/pull/4744 to make the code available to you. See test/{Fail,Succeed}/Tests.hs and look for touchFile.