From 6dd77b3c140dcba30289766354076facb3c0bc6d Mon Sep 17 00:00:00 2001 From: Tim van der Meij Date: Sun, 21 Jan 2024 18:49:55 +0100 Subject: [PATCH] Don't ignore `test.mjs` child process exit codes in the Gulpfile 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. --- gulpfile.mjs | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/gulpfile.mjs b/gulpfile.mjs index b421c6ceab300..4ee0a0f136afd 100644 --- a/gulpfile.mjs +++ b/gulpfile.mjs @@ -691,6 +691,9 @@ function createTestSource(testsName, { bot = false, xfaOnly = false } = {}) { const testProcess = startNode(args, { cwd: TEST_DIR, stdio: "inherit" }); testProcess.on("close", function (code) { + if (code !== 0) { + throw new Error(`Running ${testsName} tests failed.`); + } source.push(null); }); return undefined; @@ -722,6 +725,10 @@ function makeRef(done, bot) { const testProcess = startNode(args, { cwd: TEST_DIR, stdio: "inherit" }); testProcess.on("close", function (code) { + if (code !== 0) { + done(new Error("Creating reference images failed.")); + return; + } done(); }); }