From a4e2ced73bb6ebec95a82f48c9f0229a2d21c667 Mon Sep 17 00:00:00 2001 From: Evan Lucas Date: Wed, 12 Jul 2017 06:45:29 -0500 Subject: [PATCH] test: decrease duration of test-cli-syntax Previously, test/parallel/test-cli-syntax.js was spawning a lot of child processes, but using spawnSync, which made the test run each child process serially. This switches most of the test cases to use exec so that they are asynchronous. Locally, the test went from > 5 seconds to under 2 seconds. PR-URL: https://github.com/nodejs/node/pull/14187 Reviewed-By: Colin Ihrig Reviewed-By: Michael Dawson Reviewed-By: Rich Trott Reviewed-By: Refael Ackermann --- test/parallel/test-cli-syntax.js | 47 ++++++++++++++++++-------------- 1 file changed, 27 insertions(+), 20 deletions(-) diff --git a/test/parallel/test-cli-syntax.js b/test/parallel/test-cli-syntax.js index c27090db62bf95..6c04c0a2301757 100644 --- a/test/parallel/test-cli-syntax.js +++ b/test/parallel/test-cli-syntax.js @@ -1,9 +1,9 @@ 'use strict'; -require('../common'); +const common = require('../common'); const assert = require('assert'); -const spawnSync = require('child_process').spawnSync; const fixtures = require('../common/fixtures'); +const exec = require('child_process').exec; const node = process.execPath; @@ -29,12 +29,13 @@ const notFoundRE = /^Error: Cannot find module/m; // loop each possible option, `-c` or `--check` syntaxArgs.forEach(function(args) { const _args = args.concat(file); - const c = spawnSync(node, _args, {encoding: 'utf8'}); - // no output should be produced - assert.strictEqual(c.stdout, '', 'stdout produced'); - assert.strictEqual(c.stderr, '', 'stderr produced'); - assert.strictEqual(c.status, 0, `code == ${c.status}`); + const cmd = [node, ..._args].join(' '); + exec(cmd, common.mustCall((err, stdout, stderr) => { + assert.ifError(err); + assert.strictEqual(stdout, '', 'stdout produced'); + assert.strictEqual(stderr, '', 'stderr produced'); + })); }); }); @@ -50,15 +51,20 @@ const notFoundRE = /^Error: Cannot find module/m; // loop each possible option, `-c` or `--check` syntaxArgs.forEach(function(args) { const _args = args.concat(file); - const c = spawnSync(node, _args, {encoding: 'utf8'}); + const cmd = [node, ..._args].join(' '); + exec(cmd, common.mustCall((err, stdout, stderr) => { + assert.strictEqual(err instanceof Error, true); + assert.strictEqual(err.code, 1, `code === ${err.code}`); - // no stdout should be produced - assert.strictEqual(c.stdout, '', 'stdout produced'); + // no stdout should be produced + assert.strictEqual(stdout, '', 'stdout produced'); - // stderr should have a syntax error message - assert(syntaxErrorRE.test(c.stderr), 'stderr incorrect'); + // stderr should have a syntax error message + assert(syntaxErrorRE.test(stderr), 'stderr incorrect'); - assert.strictEqual(c.status, 1, `code == ${c.status}`); + // stderr should include the filename + assert(stderr.startsWith(file), "stderr doesn't start with the filename"); + })); }); }); @@ -72,14 +78,15 @@ const notFoundRE = /^Error: Cannot find module/m; // loop each possible option, `-c` or `--check` syntaxArgs.forEach(function(args) { const _args = args.concat(file); - const c = spawnSync(node, _args, {encoding: 'utf8'}); + const cmd = [node, ..._args].join(' '); + exec(cmd, common.mustCall((err, stdout, stderr) => { + // no stdout should be produced + assert.strictEqual(stdout, '', 'stdout produced'); - // no stdout should be produced - assert.strictEqual(c.stdout, '', 'stdout produced'); + // stderr should have a module not found error message + assert(notFoundRE.test(stderr), 'stderr incorrect'); - // stderr should have a module not found error message - assert(notFoundRE.test(c.stderr), 'stderr incorrect'); - - assert.strictEqual(c.status, 1, `code == ${c.status}`); + assert.strictEqual(err.code, 1, `code === ${err.code}`); + })); }); });