From 40710523a76bf6cc182120e82763ddbdb3ceb1be Mon Sep 17 00:00:00 2001 From: Moshe Atlow Date: Wed, 27 Jul 2022 10:02:07 +0300 Subject: [PATCH] feat: graceful termination on `--test` only PR-URL: https://github.com/nodejs/node/pull/43977 Reviewed-By: Antoine du Hamel Reviewed-By: Nitzan Uziely Reviewed-By: Benjamin Gruenbaum (cherry picked from commit 26e27424ad91c60a44d3d4c58b62a39b555ba75d) --- lib/internal/test_runner/harness.js | 11 +++-- test/common/index.js | 4 +- .../test-runner/never_ending_async.js | 7 +++ .../fixtures/test-runner/never_ending_sync.js | 6 +++ test/parallel/test-runner-exit-code.js | 48 ++++++++++--------- 5 files changed, 50 insertions(+), 26 deletions(-) create mode 100644 test/fixtures/test-runner/never_ending_async.js create mode 100644 test/fixtures/test-runner/never_ending_sync.js diff --git a/lib/internal/test_runner/harness.js b/lib/internal/test_runner/harness.js index 8c5dcc9..1780178 100644 --- a/lib/internal/test_runner/harness.js +++ b/lib/internal/test_runner/harness.js @@ -1,4 +1,4 @@ -// https://github.com/nodejs/node/blob/1523a1817ed9b06fb51c0149451f9ea31bd2756e/lib/internal/test_runner/harness.js +// https://github.com/nodejs/node/blob/26e27424ad91c60a44d3d4c58b62a39b555ba75d/lib/internal/test_runner/harness.js 'use strict' const { ArrayPrototypeForEach, @@ -14,8 +14,10 @@ const { ERR_TEST_FAILURE } } = require('#internal/errors') +const { getOptionValue } = require('#internal/options') const { Test, ItTest, Suite } = require('#internal/test_runner/test') +const isTestRunner = getOptionValue('--test') const testResources = new SafeMap() const root = new Test({ __proto__: null, name: '' }) let wasRootSetup = false @@ -136,8 +138,11 @@ function setup (root) { process.on('uncaughtException', exceptionHandler) process.on('unhandledRejection', rejectionHandler) process.on('beforeExit', exitHandler) - process.on('SIGINT', terminationHandler) - process.on('SIGTERM', terminationHandler) + // TODO(MoLow): Make it configurable to hook when isTestRunner === false. + if (isTestRunner) { + process.on('SIGINT', terminationHandler) + process.on('SIGTERM', terminationHandler) + } root.reporter.pipe(process.stdout) root.reporter.version() diff --git a/test/common/index.js b/test/common/index.js index d656ded..ae531e6 100644 --- a/test/common/index.js +++ b/test/common/index.js @@ -143,5 +143,7 @@ if (process.version.startsWith('v14.') || process.version.startsWith('v16.')) { } module.exports = { - expectsError + expectsError, + isWindow: process.platform === 'win32', + mustCall } diff --git a/test/fixtures/test-runner/never_ending_async.js b/test/fixtures/test-runner/never_ending_async.js new file mode 100644 index 0000000..a47e8b9 --- /dev/null +++ b/test/fixtures/test-runner/never_ending_async.js @@ -0,0 +1,7 @@ +// https://github.com/nodejs/node/blob/26e27424ad91c60a44d3d4c58b62a39b555ba75d/test/fixtures/test-runner/never_ending_async.js +const test = require('#node:test') +const { setTimeout } = require('#timers/promises') + +// We are using a very large timeout value to ensure that the parent process +// will have time to send a SIGINT signal to cancel the test. +test('never ending test', () => setTimeout(100_000_000)) diff --git a/test/fixtures/test-runner/never_ending_sync.js b/test/fixtures/test-runner/never_ending_sync.js new file mode 100644 index 0000000..2bc80a5 --- /dev/null +++ b/test/fixtures/test-runner/never_ending_sync.js @@ -0,0 +1,6 @@ +// https://github.com/nodejs/node/blob/26e27424ad91c60a44d3d4c58b62a39b555ba75d/test/fixtures/test-runner/never_ending_sync.js +const test = require('#node:test') + +test('never ending test', () => { + while (true); +}) diff --git a/test/parallel/test-runner-exit-code.js b/test/parallel/test-runner-exit-code.js index 7e407ee..014d381 100644 --- a/test/parallel/test-runner-exit-code.js +++ b/test/parallel/test-runner-exit-code.js @@ -1,13 +1,31 @@ -// https://github.com/nodejs/node/blob/2fd4c013c221653da2a7921d08fe1aa96aaba504/test/parallel/test-runner-exit-code.js - +// https://github.com/nodejs/node/blob/26e27424ad91c60a44d3d4c58b62a39b555ba75d/test/parallel/test-runner-exit-code.js 'use strict' - const common = require('../common') const fixtures = require('../common/fixtures') const assert = require('assert') -const { spawnSync } = require('child_process') -const { promisify } = require('util') -const setTimeout = promisify(require('timers').setTimeout) +const { spawnSync, spawn } = require('child_process') +const { once } = require('events') +const finished = require('util').promisify(require('stream').finished) + +async function runAndKill (file) { + if (common.isWindows) { + common.printSkipMessage(`signals are not supported in windows, skipping ${file}`) + return + } + let stdout = '' + const child = spawn(process.execPath, ['--test', file]) + child.stdout.setEncoding('utf8') + child.stdout.on('data', (chunk) => { + if (!stdout.length) child.kill('SIGINT') + stdout += chunk + }) + const [code, signal] = await once(child, 'exit') + await finished(child.stdout) + assert.match(stdout, /not ok 1/) + assert.match(stdout, /# cancelled 1\n/) + assert.strictEqual(signal, null) + assert.strictEqual(code, 1) +} if (process.argv[2] === 'child') { const test = require('#node:test') @@ -21,12 +39,6 @@ if (process.argv[2] === 'child') { test('failing test', () => { assert.strictEqual(true, false) }) - } else if (process.argv[3] === 'never_ends') { - assert.strictEqual(process.argv[3], 'never_ends') - test('never ending test', () => { - return setTimeout(100_000_000) - }) - process.kill(process.pid, 'SIGINT') } else assert.fail('unreachable') } else { let child = spawnSync(process.execPath, [__filename, 'child', 'pass']) @@ -41,14 +53,6 @@ if (process.argv[2] === 'child') { assert.strictEqual(child.status, 1) assert.strictEqual(child.signal, null) - child = spawnSync(process.execPath, [__filename, 'child', 'never_ends']) - assert.strictEqual(child.status, 1) - assert.strictEqual(child.signal, null) - if (common.isWindows) { - common.printSkipMessage('signals are not supported in windows') - } else { - const stdout = child.stdout.toString() - assert.match(stdout, /not ok 1 - never ending test/) - assert.match(stdout, /# cancelled 1/) - } + runAndKill(fixtures.path('test-runner', 'never_ending_sync.js')).then(common.mustCall()) + runAndKill(fixtures.path('test-runner', 'never_ending_async.js')).then(common.mustCall()) }