From 67487fe6265a34d26e8a9cd9e4d2e85dfb2eeb31 Mon Sep 17 00:00:00 2001 From: Joyee Cheung Date: Wed, 16 Aug 2023 15:27:08 +0200 Subject: [PATCH] test: add spawnSyncAndExit() and spawnSyncAndExitWithoutError() Replaces expectSyncExit() and expectSyncExitWithoutError(). Since we usually just check the child process right after its spawned, these shorthands also takes care of the spawning. This makes the tests more concise. --- test/common/README.md | 25 ++++++++++-------- test/common/child_process.js | 19 +++++++++++--- test/parallel/test-snapshot-api.js | 13 +++------- test/parallel/test-snapshot-basic.js | 32 ++++++++++------------- test/parallel/test-snapshot-warning.js | 35 +++++++++----------------- 5 files changed, 59 insertions(+), 65 deletions(-) diff --git a/test/common/README.md b/test/common/README.md index 3d35edf5510186..fa78b2792ef6ac 100644 --- a/test/common/README.md +++ b/test/common/README.md @@ -40,17 +40,16 @@ The `benchmark` module is used by tests to run benchmarks. The `child_process` module is used by tests that launch child processes. -### `expectSyncExit(child, options)` +### `spawnSyncAndExit(command[, args][, spawnOptions], expectations)` -Checks if a _synchronous_ child process runs in the way expected. If it does -not, print the stdout and stderr output from the child process and additional -information about it to the stderr of the current process before throwing -and error. This helps gathering more information about test failures -coming from child processes. +Spawns a child process synchronously using [`child_process.spawnSync()`][] and +check if it runs in the way expected. If it does not, print the stdout and +stderr output from the child process and additional information about it to +the stderr of the current process before throwing and error. This helps +gathering more information about test failures coming from child processes. -* `child` [\][]: a `ChildProcess` instance - returned by `child_process.spawnSync()`. -* `options` [\][] +* `command`, `args`, `spawnOptions` See [`child_process.spawnSync()`][] +* `expectations` [\][] * `status` [\][] Expected `child.status` * `signal` [\][] | `null` Expected `child.signal` * `stderr` [\][] | [\][] | @@ -65,8 +64,13 @@ coming from child processes. * `trim` [\][] Optional. Whether this method should trim out the whitespace characters when checking `stderr` and `stdout` outputs. Defaults to `false`. +* return [\][] + * `child` [\][] The child process returned by + [`child_process.spawnSync()`][]. + * `stderr` [\][] The output from the child process to stderr. + * `stdout` [\][] The output from the child process to stdout. -### `expectSyncExitWithoutError(child[, options])` +### `spawnSyncAndExitWithoutError(command[, args][, spawnOptions], expectations)` Similar to `expectSyncExit()` with the `status` expected to be 0 and `signal` expected to be `null`. Any other optional options are passed @@ -1160,6 +1164,7 @@ See [the WPT tests README][] for details. []: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Data_structures#Number_type []: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Data_structures#String_type [Web Platform Tests]: https://github.com/web-platform-tests/wpt +[`child_process.spawnSync()`]: ../../doc/api/child_process.md#child_processspawnsynccommand-args-options [`hijackstdio.hijackStdErr()`]: #hijackstderrlistener [`hijackstdio.hijackStdOut()`]: #hijackstdoutlistener [internationalization]: ../../doc/api/intl.md diff --git a/test/common/child_process.js b/test/common/child_process.js index a53dddc19f3216..10418a61b0b9d6 100644 --- a/test/common/child_process.js +++ b/test/common/child_process.js @@ -1,6 +1,7 @@ 'use strict'; const assert = require('assert'); +const { spawnSync } = require('child_process'); const common = require('./'); const util = require('util'); @@ -111,11 +112,21 @@ function expectSyncExit(child, { return { child, stderr: stderrStr, stdout: stdoutStr }; } -function expectSyncExitWithoutError(child, options) { +function spawnSyncAndExit(...args) { + const spawnArgs = args.slice(0, args.length - 1); + const expectations = args[args.length - 1]; + const child = spawnSync(...spawnArgs); + return expectSyncExit(child, expectations); +} + +function spawnSyncAndExitWithoutError(...args) { + const spawnArgs = args.slice(0, args.length); + const expectations = args[args.length - 1]; + const child = spawnSync(...spawnArgs); return expectSyncExit(child, { status: 0, signal: null, - ...options, + ...expectations, }); } @@ -124,6 +135,6 @@ module.exports = { logAfterTime, kExpiringChildRunTime, kExpiringParentTimer, - expectSyncExit, - expectSyncExitWithoutError, + spawnSyncAndExit, + spawnSyncAndExitWithoutError, }; diff --git a/test/parallel/test-snapshot-api.js b/test/parallel/test-snapshot-api.js index 1068ae3b4c7b46..2396dd32c345c7 100644 --- a/test/parallel/test-snapshot-api.js +++ b/test/parallel/test-snapshot-api.js @@ -4,10 +4,9 @@ require('../common'); const assert = require('assert'); -const { spawnSync } = require('child_process'); const tmpdir = require('../common/tmpdir'); const fixtures = require('../common/fixtures'); -const { expectSyncExitWithoutError } = require('../common/child_process'); +const { spawnSyncAndExitWithoutError } = require('../common/child_process'); const fs = require('fs'); const v8 = require('v8'); @@ -29,7 +28,7 @@ const entry = fixtures.path('snapshot', 'v8-startup-snapshot-api.js'); fs.writeFileSync(tmpdir.resolve(book), content, 'utf8'); } fs.copyFileSync(entry, tmpdir.resolve('entry.js')); - const child = spawnSync(process.execPath, [ + spawnSyncAndExitWithoutError(process.execPath, [ '--snapshot-blob', blobPath, '--build-snapshot', @@ -37,14 +36,12 @@ const entry = fixtures.path('snapshot', 'v8-startup-snapshot-api.js'); ], { cwd: tmpdir.path }); - - expectSyncExitWithoutError(child); const stats = fs.statSync(tmpdir.resolve('snapshot.blob')); assert(stats.isFile()); } { - const child = spawnSync(process.execPath, [ + spawnSyncAndExitWithoutError(process.execPath, [ '--snapshot-blob', blobPath, 'book1', @@ -54,9 +51,7 @@ const entry = fixtures.path('snapshot', 'v8-startup-snapshot-api.js'); ...process.env, BOOK_LANG: 'en_US', } - }); - - expectSyncExitWithoutError(child, { + }, { stderr: 'Reading book1.en_US.txt', stdout: 'This is book1.en_US.txt', trim: true diff --git a/test/parallel/test-snapshot-basic.js b/test/parallel/test-snapshot-basic.js index cd87caa3fcbce3..760469ed5dc896 100644 --- a/test/parallel/test-snapshot-basic.js +++ b/test/parallel/test-snapshot-basic.js @@ -5,10 +5,12 @@ require('../common'); const assert = require('assert'); -const { spawnSync } = require('child_process'); const tmpdir = require('../common/tmpdir'); const fixtures = require('../common/fixtures'); -const { expectSyncExitWithoutError, expectSyncExit } = require('../common/child_process'); +const { + spawnSyncAndExitWithoutError, + spawnSyncAndExit, +} = require('../common/child_process'); const fs = require('fs'); tmpdir.refresh(); @@ -18,14 +20,12 @@ if (!process.config.variables.node_use_node_snapshot) { // Check that Node.js built without an embedded snapshot // exits with 9 when node:embedded_snapshot_main is specified // as snapshot entry point. - const child = spawnSync(process.execPath, [ + spawnSyncAndExit(process.execPath, [ '--build-snapshot', snapshotScript, ], { cwd: tmpdir.path - }); - - expectSyncExit(child, { + }, { status: 9, signal: null, stderr: /Node\.js was built without embedded snapshot/ @@ -37,13 +37,12 @@ if (!process.config.variables.node_use_node_snapshot) { // By default, the snapshot blob path is cwd/snapshot.blob. { // Create the snapshot. - const child = spawnSync(process.execPath, [ + spawnSyncAndExitWithoutError(process.execPath, [ '--build-snapshot', snapshotScript, ], { cwd: tmpdir.path }); - expectSyncExitWithoutError(child); const stats = fs.statSync(tmpdir.resolve('snapshot.blob')); assert(stats.isFile()); } @@ -52,7 +51,7 @@ tmpdir.refresh(); const blobPath = tmpdir.resolve('my-snapshot.blob'); { // Create the snapshot. - const child = spawnSync(process.execPath, [ + spawnSyncAndExitWithoutError(process.execPath, [ '--snapshot-blob', blobPath, '--build-snapshot', @@ -60,38 +59,33 @@ const blobPath = tmpdir.resolve('my-snapshot.blob'); ], { cwd: tmpdir.path }); - expectSyncExitWithoutError(child); const stats = fs.statSync(blobPath); assert(stats.isFile()); } { // Check --help. - const child = spawnSync(process.execPath, [ + spawnSyncAndExitWithoutError(process.execPath, [ '--snapshot-blob', blobPath, '--help', ], { cwd: tmpdir.path + }, { + stdout: /--help/ }); - expectSyncExitWithoutError(child); - - assert(child.stdout.toString().includes('--help')); } { // Check -c. - const child = spawnSync(process.execPath, [ + spawnSyncAndExitWithoutError(process.execPath, [ '--snapshot-blob', blobPath, '-c', fixtures.path('snapshot', 'marked.js'), ], { cwd: tmpdir.path - }); - - // Check that it is a noop. - expectSyncExitWithoutError(child, { + }, { stderr: '', stdout: '', trim: true diff --git a/test/parallel/test-snapshot-warning.js b/test/parallel/test-snapshot-warning.js index 444f65af0b8b35..889fed59db54a9 100644 --- a/test/parallel/test-snapshot-warning.js +++ b/test/parallel/test-snapshot-warning.js @@ -7,10 +7,9 @@ require('../common'); const assert = require('assert'); -const { spawnSync } = require('child_process'); const tmpdir = require('../common/tmpdir'); const fixtures = require('../common/fixtures'); -const { expectSyncExitWithoutError } = require('../common/child_process'); +const { spawnSyncAndExitWithoutError } = require('../common/child_process'); const fs = require('fs'); const warningScript = fixtures.path('snapshot', 'warning.js'); @@ -20,7 +19,7 @@ const empty = fixtures.path('empty.js'); tmpdir.refresh(); { console.log('\n# Check snapshot scripts that do not emit warnings.'); - let child = spawnSync(process.execPath, [ + spawnSyncAndExitWithoutError(process.execPath, [ '--snapshot-blob', blobPath, '--build-snapshot', @@ -28,40 +27,36 @@ tmpdir.refresh(); ], { cwd: tmpdir.path }); - expectSyncExitWithoutError(child); const stats = fs.statSync(blobPath); assert(stats.isFile()); - child = spawnSync(process.execPath, [ + spawnSyncAndExitWithoutError(process.execPath, [ '--snapshot-blob', blobPath, warningScript, ], { cwd: tmpdir.path - }); - expectSyncExitWithoutError(child, { + }, { stderr(output) { const match = output.match(/Warning: test warning/g); assert.strictEqual(match.length, 1); return true; } }); - } tmpdir.refresh(); { console.log('\n# Check snapshot scripts that emit ' + 'warnings and --trace-warnings hint.'); - let child = spawnSync(process.execPath, [ + spawnSyncAndExitWithoutError(process.execPath, [ '--snapshot-blob', blobPath, '--build-snapshot', warningScript, ], { cwd: tmpdir.path - }); - expectSyncExitWithoutError(child, { + }, { stderr(output) { let match = output.match(/Warning: test warning/g); assert.strictEqual(match.length, 1); @@ -73,15 +68,13 @@ tmpdir.refresh(); const stats = fs.statSync(blobPath); assert(stats.isFile()); - child = spawnSync(process.execPath, [ + spawnSyncAndExitWithoutError(process.execPath, [ '--snapshot-blob', blobPath, warningScript, ], { cwd: tmpdir.path - }); - - expectSyncExitWithoutError(child, { + }, { stderr(output) { // Warnings should not be handled more than once. let match = output.match(/Warning: test warning/g); @@ -99,7 +92,7 @@ tmpdir.refresh(); const warningFile1 = tmpdir.resolve('warnings.txt'); const warningFile2 = tmpdir.resolve('warnings2.txt'); - let child = spawnSync(process.execPath, [ + spawnSyncAndExitWithoutError(process.execPath, [ '--snapshot-blob', blobPath, '--redirect-warnings', @@ -108,9 +101,7 @@ tmpdir.refresh(); warningScript, ], { cwd: tmpdir.path - }); - - expectSyncExitWithoutError(child, { + }, { stderr(output) { assert.doesNotMatch(output, /Warning: test warning/); } @@ -129,7 +120,7 @@ tmpdir.refresh(); maxRetries: 3, recursive: false, force: true }); - child = spawnSync(process.execPath, [ + spawnSyncAndExitWithoutError(process.execPath, [ '--snapshot-blob', blobPath, '--redirect-warnings', @@ -137,9 +128,7 @@ tmpdir.refresh(); warningScript, ], { cwd: tmpdir.path - }); - - expectSyncExitWithoutError(child, { + }, { stderr(output) { assert.doesNotMatch(output, /Warning: test warning/); return true;