feat: graceful termination on --test only

PR-URL: nodejs/node#43977 Reviewed-By: Antoine du Hamel <[email protected]> Reviewed-By: Nitzan Uziely <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]> (cherry picked from commit 26e27424ad91c60a44d3d4c58b62a39b555ba75d)
nodejs · Jul 27, 2022 · 4071052 · 4071052
1 parent f875da2
commit 4071052
Show file tree

Hide file tree

Showing 5 changed files with 50 additions and 26 deletions.
diff --git 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: '<root>' })
 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
@@ -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
@@ -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
@@ -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
@@ -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())
 }