testscript: interrupted background process should not cause a test to fail #260
Assignees
Labels
bug
Something isn't working
Comments
|
I'm currently looking into this, e.g. #261 to also test Windows. |
mvdan
added a commit
to mvdan/go-internal
that referenced
this issue
Jul 9, 2024
When an entire script runs and the end is reached, any background processes begun with a '&' command get interrupted or killed, depending on the platform and timeout, and we wait for them to finish. We also checked their resulting status code and failed if they didn't exit with a status code of 0. However, as explained in the comment, this would always fail on Windows, given that it doesn't have interrupt signals so we would kill directly, causing a "signal: killed" error. Worse, any failures here caused a `panic: fail now!` as that is how we bubble up errors when a script command is being run, but such panics were not being recovered once we reached the end of a script. Now that we don't check the result anymore here, the panics are gone. Fixes rogpeppe#228. Fixes rogpeppe#260.
mvdan
added a commit
to mvdan/go-internal
that referenced
this issue
Jul 9, 2024
When an entire script runs and the end is reached, any background processes begun with a '&' command get interrupted or killed, depending on the platform and timeout, and we wait for them to finish. We also checked their resulting status code and failed if they didn't exit with a status code of 0. However, as explained in the comment, this would always fail on Windows, given that it doesn't have interrupt signals so we would kill directly, causing a "signal: killed" error. Worse, any failures here caused a `panic: fail now!` as that is how we bubble up errors when a script command is being run, but such panics were not being recovered once we reached the end of a script. Now that we don't check the result anymore here, the panics are gone. Fixes rogpeppe#228. Fixes rogpeppe#260.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
It is usual and expected for processes to return a non-zero exit code when they
are terminated with a signal. At the end of a test, testscript kills all background
processes and then fails if any of them terminate with a non-zero exit code.
This doesn't seem right. (It's even more wrong that it panics with an error code
that should be internal-only - see issue #228).
Instead, at the end of a test, testscript should check if any background processes
have already exited with a non-zero exit code and fail then, but otherwise
it should interrupt them and ignore any non-zero exit codes from processes
killed as a result.
The text was updated successfully, but these errors were encountered: