From e8d4aab143661ca3b394284fa7c7047d8296c97e Mon Sep 17 00:00:00 2001
From: Sergey Chernyshev <schernyshev@openjdk.org>
Date: Thu, 25 Apr 2024 04:21:53 +0200
Subject: [PATCH] test_runner: dont set exit code on todo tests

PR-URL: https://github.com/nodejs/node/pull/48929
Reviewed-By: Chemi Atlow <chemi@atlow.co.il>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
---
 graal-nodejs/lib/internal/main/test_runner.js     |  6 ++++--
 graal-nodejs/lib/internal/test_runner/harness.js  |  6 ++++--
 graal-nodejs/lib/internal/test_runner/test.js     |  6 +++---
 .../test/fixtures/test-runner/todo_exit_code.js   | 15 +++++++++++++++
 .../test/parallel/test-runner-exit-code.js        | 13 +++++++++++++
 5 files changed, 39 insertions(+), 7 deletions(-)
 create mode 100644 graal-nodejs/test/fixtures/test-runner/todo_exit_code.js

diff --git a/graal-nodejs/lib/internal/main/test_runner.js b/graal-nodejs/lib/internal/main/test_runner.js
index f3d0a56884b..9413c8c93c7 100644
--- a/graal-nodejs/lib/internal/main/test_runner.js
+++ b/graal-nodejs/lib/internal/main/test_runner.js
@@ -57,6 +57,8 @@ if (shardOption) {
 }
 
 run({ concurrency, inspectPort, watch: getOptionValue('--watch'), setup: setupTestReporters, shard })
-.once('test:fail', () => {
-  process.exitCode = 1;
+.on('test:fail', (data) => {
+  if (data.todo === undefined || data.todo === false) {
+    process.exitCode = 1;
+  }
 });
diff --git a/graal-nodejs/lib/internal/test_runner/harness.js b/graal-nodejs/lib/internal/test_runner/harness.js
index d0a71735afe..f85217bec35 100644
--- a/graal-nodejs/lib/internal/test_runner/harness.js
+++ b/graal-nodejs/lib/internal/test_runner/harness.js
@@ -189,8 +189,10 @@ let reportersSetup;
 function getGlobalRoot() {
   if (!globalRoot) {
     globalRoot = createTestTree();
-    globalRoot.reporter.once('test:fail', () => {
-      process.exitCode = 1;
+    globalRoot.reporter.on('test:fail', (data) => {
+      if (data.todo === undefined || data.todo === false) {
+        process.exitCode = 1;
+      }
     });
     reportersSetup = setupTestReporters(globalRoot);
   }
diff --git a/graal-nodejs/lib/internal/test_runner/test.js b/graal-nodejs/lib/internal/test_runner/test.js
index aef307ac0cb..cc7c81cad88 100644
--- a/graal-nodejs/lib/internal/test_runner/test.js
+++ b/graal-nodejs/lib/internal/test_runner/test.js
@@ -311,8 +311,8 @@ class Test extends AsyncResource {
     this.harness = null; // Configured on the root test by the test harness.
     this.mock = null;
     this.cancelled = false;
-    this.skipped = !!skip;
-    this.isTodo = !!todo;
+    this.skipped = skip !== undefined && skip !== false;
+    this.isTodo = todo !== undefined && todo !== false;
     this.startTime = null;
     this.endTime = null;
     this.passed = false;
@@ -667,7 +667,7 @@ class Test extends AsyncResource {
         subtest.#cancel(pendingSubtestsError);
         subtest.postRun(pendingSubtestsError);
       }
-      if (!subtest.passed) {
+      if (!subtest.passed && !subtest.isTodo) {
         failed++;
       }
     }
diff --git a/graal-nodejs/test/fixtures/test-runner/todo_exit_code.js b/graal-nodejs/test/fixtures/test-runner/todo_exit_code.js
new file mode 100644
index 00000000000..6577eefe52f
--- /dev/null
+++ b/graal-nodejs/test/fixtures/test-runner/todo_exit_code.js
@@ -0,0 +1,15 @@
+const { describe, test } = require('node:test');
+
+describe('suite should pass', () => {
+  test.todo('should fail without harming suite', () => {
+    throw new Error('Fail but not badly')
+  });
+});
+
+test.todo('should fail without effecting exit code', () => {
+  throw new Error('Fail but not badly')
+});
+
+test('empty string todo', { todo: '' }, () => {
+  throw new Error('Fail but not badly')
+});
diff --git a/graal-nodejs/test/parallel/test-runner-exit-code.js b/graal-nodejs/test/parallel/test-runner-exit-code.js
index 8eeebc21d31..700480386d5 100644
--- a/graal-nodejs/test/parallel/test-runner-exit-code.js
+++ b/graal-nodejs/test/parallel/test-runner-exit-code.js
@@ -50,6 +50,19 @@ if (process.argv[2] === 'child') {
   assert.strictEqual(child.status, 0);
   assert.strictEqual(child.signal, null);
 
+
+  child = spawnSync(process.execPath, [
+    '--test',
+    fixtures.path('test-runner', 'todo_exit_code.js'),
+  ]);
+  assert.strictEqual(child.status, 0);
+  assert.strictEqual(child.signal, null);
+  const stdout = child.stdout.toString();
+  assert.match(stdout, /# tests 3/);
+  assert.match(stdout, /# pass 0/);
+  assert.match(stdout, /# fail 0/);
+  assert.match(stdout, /# todo 3/);
+
   child = spawnSync(process.execPath, [__filename, 'child', 'fail']);
   assert.strictEqual(child.status, 1);
   assert.strictEqual(child.signal, null);