Don't ignore test.mjs child process exit codes in the Gulpfile
#17555
Conversation
In the Gulpfile only the exit codes of `test.mjs` child processes erroneously aren't checked. This causes failures in `test.mjs` to be logged but not propagated to the master process, which in turn causes test runners such as GitHub Actions to succeed because they only monitor the master process. This is easy to reproduce by throwing an error at the top of `test.mjs` and running `gulp makeref` or `gulp unittest`: the error is logged, but the task that spawned the child process succeeds and the master process exits with exit code 0. This is problematic because it can easily cause errors to go by unnoticed. This commit fixes the issue by making sure that the `test.mjs` invocations are handled in the same way as the other child processes in the file, i.e., if the child process exits with a non-zero exit code then the master process also exits with a non-zero exit code. After this patch the error is still logged, but the task now also fails and the master process exits with exit code 1 to properly signal failure.
|
Coming to think of it, this might also be why the bot scripts check the output string instead of the exit code of the process: the exit code is always 0 before this patch, so it wasn't the reliable indicator of success or failure it should have been. This means it's also relevant for #11851 because the GitHub Actions pipelines rely on the exit code. /botio unittest |
From: Bot.io (Linux m4)ReceivedCommand cmd_unittest from @timvandermeij received. Current queue size: 0 Live output at: http://54.241.84.105:8877/028bb669e218297/output.txt |
From: Bot.io (Windows)ReceivedCommand cmd_unittest from @timvandermeij received. Current queue size: 1 Live output at: http://54.193.163.58:8877/12ee678ef0d7e3d/output.txt |
From: Bot.io (Linux m4)SuccessFull output at http://54.241.84.105:8877/028bb669e218297/output.txt Total script time: 2.55 mins
|
From: Bot.io (Windows)SuccessFull output at http://54.193.163.58:8877/12ee678ef0d7e3d/output.txt Total script time: 9.24 mins
|
In the Gulpfile only the exit codes of
test.mjschild processes erroneously aren't checked. This causes failures intest.mjsto be logged but not propagated to the master process, which in turn causes test runners such as GitHub Actions to succeed because they only monitor the master process. This is easy to reproduce by throwing an error at the top oftest.mjsand runninggulp makereforgulp unittest: the error is logged, but the task that spawned the child process succeeds and the master process exits with exit code 0. This is problematic because it can easily cause errors to go by unnoticed.This commit fixes the issue by making sure that the
test.mjsinvocations are handled in the same way as the other child processes in the file, i.e., if the child process exits with a non-zero exit code then the master process also exits with a non-zero exit code. After this patch the error is still logged, but the task now also fails and the master process exits with exit code 1 to properly signal failure.Fixes #17396.