diff --git a/lib/internal/main/test_runner.js b/lib/internal/main/test_runner.js index 064731d77bede1..4974f4ce0d338b 100644 --- a/lib/internal/main/test_runner.js +++ b/lib/internal/main/test_runner.js @@ -58,6 +58,8 @@ if (shardOption) { } run({ concurrency, inspectPort, watch: getOptionValue('--watch'), setup: setupTestReporters, shard }) -.once('test:fail', () => { - process.exitCode = kGenericUserError; +.on('test:fail', (data) => { + if (data.todo === undefined || data.todo === false) { + process.exitCode = kGenericUserError; + } }); diff --git a/lib/internal/test_runner/harness.js b/lib/internal/test_runner/harness.js index 246620f6628d88..32e188bb65a53a 100644 --- a/lib/internal/test_runner/harness.js +++ b/lib/internal/test_runner/harness.js @@ -190,8 +190,10 @@ let reportersSetup; function getGlobalRoot() { if (!globalRoot) { globalRoot = createTestTree(); - globalRoot.reporter.once('test:fail', () => { - process.exitCode = kGenericUserError; + globalRoot.reporter.on('test:fail', (data) => { + if (data.todo === undefined || data.todo === false) { + process.exitCode = kGenericUserError; + } }); reportersSetup = setupTestReporters(globalRoot); } diff --git a/lib/internal/test_runner/reporter/spec.js b/lib/internal/test_runner/reporter/spec.js index 16cbdf1d5aa901..e7ac671bf15e04 100644 --- a/lib/internal/test_runner/reporter/spec.js +++ b/lib/internal/test_runner/reporter/spec.js @@ -64,17 +64,24 @@ class SpecReporter extends Transform { ), `\n${indent} `); return `\n${indent} ${message}\n`; } - #formatTestReport(type, data, prefix = '', indent = '', hasChildren = false, skippedSubtest = false) { + #formatTestReport(type, data, prefix = '', indent = '', hasChildren = false) { let color = colors[type] ?? white; let symbol = symbols[type] ?? ' '; + const { skip, todo } = data; const duration_ms = data.details?.duration_ms ? ` ${gray}(${data.details.duration_ms}ms)${white}` : ''; - const title = `${data.name}${duration_ms}${skippedSubtest ? ' # SKIP' : ''}`; + let title = `${data.name}${duration_ms}`; + + if (skip !== undefined) { + title += ` # ${typeof skip === 'string' && skip.length ? skip : 'SKIP'}`; + } else if (todo !== undefined) { + title += ` # ${typeof todo === 'string' && todo.length ? todo : 'TODO'}`; + } if (hasChildren) { // If this test has had children - it was already reported, so slightly modify the output return `${prefix}${indent}${color}${symbols['arrow:right']}${white}${title}\n`; } const error = this.#formatError(data.details?.error, indent); - if (skippedSubtest) { + if (skip !== undefined) { color = gray; symbol = symbols['hyphen:minus']; } @@ -101,9 +108,8 @@ class SpecReporter extends Transform { ArrayPrototypeShift(this.#reported); hasChildren = true; } - const skippedSubtest = subtest && data.skip && data.skip !== undefined; const indent = this.#indent(data.nesting); - return `${this.#formatTestReport(type, data, prefix, indent, hasChildren, skippedSubtest)}\n`; + return `${this.#formatTestReport(type, data, prefix, indent, hasChildren)}\n`; } #handleEvent({ type, data }) { switch (type) { diff --git a/lib/internal/test_runner/test.js b/lib/internal/test_runner/test.js index 9c76a371462057..aaf63635f3f827 100644 --- a/lib/internal/test_runner/test.js +++ b/lib/internal/test_runner/test.js @@ -282,8 +282,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; @@ -634,7 +634,7 @@ class Test extends AsyncResource { subtest.#cancel(pendingSubtestsError); subtest.postRun(pendingSubtestsError); } - if (!subtest.passed) { + if (!subtest.passed && !subtest.isTodo) { failed++; } } diff --git a/test/fixtures/test-runner/output/describe_it.js b/test/fixtures/test-runner/output/describe_it.js index 0b89e1a11112b1..6625747d026969 100644 --- a/test/fixtures/test-runner/output/describe_it.js +++ b/test/fixtures/test-runner/output/describe_it.js @@ -13,12 +13,12 @@ it.todo('sync pass todo', () => { it('sync pass todo with message', { todo: 'this is a passing todo' }, () => { }); -it.todo('sync fail todo', () => { - throw new Error('thrown from sync fail todo'); +it.todo('sync todo', () => { + throw new Error('should not count as a failure'); }); -it('sync fail todo with message', { todo: 'this is a failing todo' }, () => { - throw new Error('thrown from sync fail todo with message'); +it('sync todo with message', { todo: 'this is a failing todo' }, () => { + throw new Error('should not count as a failure'); }); it.skip('sync skip pass', () => { diff --git a/test/fixtures/test-runner/output/describe_it.snapshot b/test/fixtures/test-runner/output/describe_it.snapshot index c5b4610d522e43..0d07851e2a1fa9 100644 --- a/test/fixtures/test-runner/output/describe_it.snapshot +++ b/test/fixtures/test-runner/output/describe_it.snapshot @@ -9,12 +9,12 @@ ok 2 - sync pass todo with message # TODO this is a passing todo --- duration_ms: * ... -# Subtest: sync fail todo -not ok 3 - sync fail todo # TODO +# Subtest: sync todo +not ok 3 - sync todo # TODO --- duration_ms: * failureType: 'testCodeFailure' - error: 'thrown from sync fail todo' + error: 'should not count as a failure' code: 'ERR_TEST_FAILURE' stack: |- * @@ -25,12 +25,12 @@ not ok 3 - sync fail todo # TODO * * ... -# Subtest: sync fail todo with message -not ok 4 - sync fail todo with message # TODO this is a failing todo +# Subtest: sync todo with message +not ok 4 - sync todo with message # TODO this is a failing todo --- duration_ms: * failureType: 'testCodeFailure' - error: 'thrown from sync fail todo with message' + error: 'should not count as a failure' code: 'ERR_TEST_FAILURE' stack: |- * diff --git a/test/fixtures/test-runner/output/spec_reporter.snapshot b/test/fixtures/test-runner/output/spec_reporter.snapshot index 768204177014af..13f3618d38c28d 100644 --- a/test/fixtures/test-runner/output/spec_reporter.snapshot +++ b/test/fixtures/test-runner/output/spec_reporter.snapshot @@ -1,6 +1,6 @@ - sync pass todo (*ms) - sync pass todo with message (*ms) - sync fail todo (*ms) + sync pass todo (*ms) # TODO + sync pass todo with message (*ms) # this is a passing todo + sync fail todo (*ms) # TODO Error: thrown from sync fail todo * * @@ -10,7 +10,7 @@ * * - sync fail todo with message (*ms) + sync fail todo with message (*ms) # this is a failing todo Error: thrown from sync fail todo with message * * @@ -21,7 +21,7 @@ * sync skip pass (*ms) # SKIP - sync skip pass with message (*ms) # SKIP + sync skip pass with message (*ms) # this is skipped sync pass (*ms) this test should pass sync throw fail (*ms) @@ -130,7 +130,7 @@ invalid subtest - pass but subtest fails (*ms) sync skip option (*ms) # SKIP - sync skip option with message (*ms) # SKIP + sync skip option with message (*ms) # this is skipped sync skip option is false fail (*ms) Error: this should be executed * @@ -151,8 +151,8 @@ functionAndOptions (*ms) # SKIP escaped description \ # \#\  (*ms) - escaped skip message (*ms) # SKIP - escaped todo message (*ms) + escaped skip message (*ms) # #skip + escaped todo message (*ms) # #todo escaped diagnostic (*ms) #diagnostic callback pass (*ms) @@ -307,7 +307,7 @@ failing tests: - sync fail todo (*ms) + sync fail todo (*ms) # TODO Error: thrown from sync fail todo * * @@ -317,7 +317,7 @@ * * - sync fail todo with message (*ms) + sync fail todo with message (*ms) # this is a failing todo Error: thrown from sync fail todo with message * * @@ -347,7 +347,7 @@ * * - async skip fail (*ms) + async skip fail (*ms) # SKIP Error: thrown from async throw fail * * diff --git a/test/fixtures/test-runner/output/spec_reporter_cli.snapshot b/test/fixtures/test-runner/output/spec_reporter_cli.snapshot index e4e08764fd4925..22c9a9174574a1 100644 --- a/test/fixtures/test-runner/output/spec_reporter_cli.snapshot +++ b/test/fixtures/test-runner/output/spec_reporter_cli.snapshot @@ -1,6 +1,6 @@ - sync pass todo (*ms) - sync pass todo with message (*ms) - sync fail todo (*ms) + sync pass todo (*ms) # TODO + sync pass todo with message (*ms) # this is a passing todo + sync fail todo (*ms) # TODO Error: thrown from sync fail todo * * @@ -10,7 +10,7 @@ * * - sync fail todo with message (*ms) + sync fail todo with message (*ms) # this is a failing todo Error: thrown from sync fail todo with message * * @@ -21,7 +21,7 @@ * sync skip pass (*ms) # SKIP - sync skip pass with message (*ms) # SKIP + sync skip pass with message (*ms) # this is skipped sync pass (*ms) this test should pass sync throw fail (*ms) @@ -130,7 +130,7 @@ invalid subtest - pass but subtest fails (*ms) sync skip option (*ms) # SKIP - sync skip option with message (*ms) # SKIP + sync skip option with message (*ms) # this is skipped sync skip option is false fail (*ms) Error: this should be executed * @@ -151,8 +151,8 @@ functionAndOptions (*ms) # SKIP escaped description \ # \#\  (*ms) - escaped skip message (*ms) # SKIP - escaped todo message (*ms) + escaped skip message (*ms) # #skip + escaped todo message (*ms) # #todo escaped diagnostic (*ms) #diagnostic callback pass (*ms) @@ -307,7 +307,7 @@ failing tests: - sync fail todo (*ms) + sync fail todo (*ms) # TODO Error: thrown from sync fail todo * * @@ -317,7 +317,7 @@ * * - sync fail todo with message (*ms) + sync fail todo with message (*ms) # this is a failing todo Error: thrown from sync fail todo with message * * @@ -347,7 +347,7 @@ * * - async skip fail (*ms) + async skip fail (*ms) # SKIP Error: thrown from async throw fail * * diff --git a/test/fixtures/test-runner/todo_exit_code.js b/test/fixtures/test-runner/todo_exit_code.js new file mode 100644 index 00000000000000..6577eefe52f7dc --- /dev/null +++ b/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/test/parallel/test-runner-exit-code.js b/test/parallel/test-runner-exit-code.js index 8eeebc21d31753..700480386d5b4a 100644 --- a/test/parallel/test-runner-exit-code.js +++ b/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);