Fix: missing --aggregate-output in npm-run-all

mysticatea · mysticatea · commit 693261b61690 · 2017-11-03T18:20:25.000+09:00
diff --git a/bin/common/parse-cli-args.js b/bin/common/parse-cli-args.js
@@ -221,8 +221,11 @@ function parseCLIArgsCore(set, args) {    // eslint-disable-line complexity
         }
     }
 
+    if (!set.parallel && set.aggregateOutput) {
+        throw new Error("Invalid Option: --aggregate-output (without parallel)")
+    }
     if (!set.parallel && set.race) {
-        const race = args.indexOf("--race") !== -1 ? "race" : "r"
+        const race = args.indexOf("--race") !== -1 ? "--race" : "-r"
         throw new Error(`Invalid Option: ${race} (without parallel)`)
     }
     if (!set.parallel && set.maxParallel !== 0) {
diff --git a/bin/npm-run-all/help.js b/bin/npm-run-all/help.js
@@ -27,6 +27,8 @@ Usage:
     <tasks> : A list of npm-scripts' names and Glob-like patterns.
 
 Options:
+    --aggregate-output   - - - Avoid interleaving output by delaying printing of
+                               each command's output until it has finished.
     -c, --continue-on-error  - Set the flag to continue executing
                                other/subsequent tasks even if a task threw an
                                error. 'npm-run-all' itself will exit with
diff --git a/bin/npm-run-all/main.js b/bin/npm-run-all/main.js
@@ -52,6 +52,7 @@ module.exports = function npmRunAll(args, stdout, stderr) {
                         arguments: argv.rest,
                         race: group.parallel && argv.race,
                         npmPath: argv.npmPath,
+                        aggregateOutput: group.parallel && argv.aggregateOutput,
                     }
                 ))
             },
diff --git a/bin/run-p/help.js b/bin/run-p/help.js
@@ -27,14 +27,14 @@ Usage:
     <tasks> : A list of npm-scripts' names and Glob-like patterns.
 
 Options:
+    --aggregate-output   - - - Avoid interleaving output by delaying printing of
+                               each command's output until it has finished.
     -c, --continue-on-error  - Set the flag to continue executing other tasks
                                even if a task threw an error. 'run-p' itself
                                will exit with non-zero code if one or more tasks
                                threw error(s).
     --max-parallel <number>  - Set the maximum number of parallelism. Default is
                                unlimited.
-    --aggregate-output       - Avoid interleaving output by delaying printing of
-                               each command's output until it has finished.
     --npm-path <string>  - - - Set the path to npm. Default is the value of
                                environment variable npm_execpath.
                                If the variable is not defined, then it's "npm."
diff --git a/docs/node-api.md b/docs/node-api.md
@@ -35,6 +35,10 @@ Run npm-scripts.
 
 - **patterns** `string|string[]` -- Glob-like patterns for script names.
 - **options** `object`
+  - **options.aggregateOutput** `boolean` --
+    The flag to avoid interleaving output by delaying printing of each command's output until it has finished.
+    This option is valid only with `options.parallel` option.
+    Default is `false`.
   - **options.arguments** `string[]` --
     An argument list to replace argument placeholders (such as `{1}`, `{2}`). If pattern text has `{1}`, it's replaced by `options.arguments[0]`.
     Default is an empty array.
@@ -47,6 +51,7 @@ Run npm-scripts.
     Default is `false`.
   - **options.maxParallel** `number` --
     The maximum number of parallelism.
+    This option is valid only with `options.parallel` option.
     Default is `Number.POSITIVE_INFINITY`.
   - **options.npmPath** `string` --
     The path to npm.
diff --git a/docs/npm-run-all.md b/docs/npm-run-all.md
@@ -13,6 +13,8 @@ Usage:
     <tasks> : A list of npm-scripts' names and Glob-like patterns.
 
 Options:
+    --aggregate-output   - - - Avoid interleaving output by delaying printing of
+                               each command's output until it has finished.
     -c, --continue-on-error  - Set the flag to continue executing
                                other/subsequent tasks even if a task threw an
                                error. 'npm-run-all' itself will exit with
diff --git a/docs/run-p.md b/docs/run-p.md
@@ -16,14 +16,14 @@ Usage:
     <tasks> : A list of npm-scripts' names and Glob-like patterns.
 
 Options:
+    --aggregate-output   - - - Avoid interleaving output by delaying printing of
+                               each command's output until it has finished.
     -c, --continue-on-error  - Set the flag to continue executing other tasks
                                even if a task threw an error. 'run-p' itself
                                will exit with non-zero code if one or more tasks
                                threw error(s).
     --max-parallel <number>  - Set the maximum number of parallelism. Default is
                                unlimited.
-    --aggregate-output       - Avoid interleaving output by delaying printing of
-                               each command's output until it has finished.
     --npm-path <string>  - - - Set the path to npm. Default is the value of
                                environment variable npm_execpath.
                                If the variable is not defined, then it's "npm."
diff --git a/lib/index.js b/lib/index.js
@@ -236,8 +236,11 @@ module.exports = function npmRunAll(patternOrPatterns, options) { //eslint-disab
         if (typeof maxParallel !== "number" || !(maxParallel >= 0)) {
             throw new Error("Invalid options.maxParallel")
         }
+        if (!parallel && aggregateOutput) {
+            throw new Error("Invalid options.aggregateOutput; It requires options.parallel")
+        }
         if (!parallel && race) {
-            throw new Error("Invalid options.race")
+            throw new Error("Invalid options.race; It requires options.parallel")
         }
 
         const prefixOptions = [].concat(
diff --git a/test/aggregate-output.js b/test/aggregate-output.js
@@ -36,16 +36,11 @@ describe("[aggregated-output] npm-run-all", () => {
     }
 
     describe("should not intermingle output of various commands", () => {
-        const EXPECTED_SERIALIZED_TEXT = [
-            createExpectedOutput("first"),
-            createExpectedOutput("second"),
-            `${createExpectedOutput("third")}\n`,
-        ].join("\n")
-
         const EXPECTED_PARALLELIZED_TEXT = [
             createExpectedOutput("second"),
             createExpectedOutput("third"),
-            `${createExpectedOutput("first")}\n`,
+            createExpectedOutput("first"),
+            "",
         ].join("\n")
 
         let stdout = null
@@ -54,28 +49,59 @@ describe("[aggregated-output] npm-run-all", () => {
             stdout = new BufferStream()
         })
 
-        it("Node API", async () => {
+        it("Node API with parallel", async () => {
             await nodeApi(
                 ["test-task:delayed first 300", "test-task:delayed second 100", "test-task:delayed third 200"],
-                { stdout, silent: true, aggregateOutput: true }
+                { stdout, parallel: true, silent: true, aggregateOutput: true }
             )
-            assert.equal(stdout.value, EXPECTED_SERIALIZED_TEXT)
+            assert.equal(stdout.value, EXPECTED_PARALLELIZED_TEXT)
         })
 
-        it("npm-run-all command", async () => {
+        it("Node API without parallel should fail", async () => {
+            try {
+                await nodeApi(
+                    ["test-task:delayed first 300", "test-task:delayed second 100", "test-task:delayed third 200"],
+                    { stdout, silent: true, aggregateOutput: true }
+                )
+            }
+            catch (_err) {
+                return
+            }
+            assert(false, "should fail")
+        })
+
+        it("npm-run-all command with parallel", async () => {
             await runAll(
-                ["test-task:delayed first 300", "test-task:delayed second 100", "test-task:delayed third 200", "--silent", "--aggregate-output"],
+                ["--parallel", "test-task:delayed first 300", "test-task:delayed second 100", "test-task:delayed third 200", "--silent", "--aggregate-output"],
                 stdout
             )
-            assert.equal(stdout.value, EXPECTED_SERIALIZED_TEXT)
+            assert.equal(stdout.value, EXPECTED_PARALLELIZED_TEXT)
         })
 
-        it("run-s command", async () => {
-            await runSeq(
-                ["test-task:delayed first 300", "test-task:delayed second 100", "test-task:delayed third 200", "--silent", "--aggregate-output"],
-                stdout
-            )
-            assert.equal(stdout.value, EXPECTED_SERIALIZED_TEXT)
+        it("npm-run-all command without parallel should fail", async () => {
+            try {
+                await runAll(
+                    ["test-task:delayed first 300", "test-task:delayed second 100", "test-task:delayed third 200", "--silent", "--aggregate-output"],
+                    stdout
+                )
+            }
+            catch (_err) {
+                return
+            }
+            assert(false, "should fail")
+        })
+
+        it("run-s command should fail", async () => {
+            try {
+                await runSeq(
+                    ["test-task:delayed first 300", "test-task:delayed second 100", "test-task:delayed third 200", "--silent", "--aggregate-output"],
+                    stdout
+                )
+            }
+            catch (_err) {
+                return
+            }
+            assert(false, "should fail")
         })
 
         it("run-p command", async () => {

Original file line number	Diff line number	Diff line change
`@@ -221,8 +221,11 @@ function parseCLIArgsCore(set, args) { // eslint-disable-line complexity`
`221`	`221`	`}`
`222`	`222`	`}`
`223`	`223`
	`224`	`+ if (!set.parallel && set.aggregateOutput) {`
	`225`	`+ throw new Error("Invalid Option: --aggregate-output (without parallel)")`
	`226`	`+ }`
`224`	`227`	`if (!set.parallel && set.race) {`
`225`		`- const race = args.indexOf("--race") !== -1 ? "race" : "r"`
	`228`	`+ const race = args.indexOf("--race") !== -1 ? "--race" : "-r"`
`226`	`229`	throw new Error(`Invalid Option: ${race} (without parallel)`)
`227`	`230`	`}`
`228`	`231`	`if (!set.parallel && set.maxParallel !== 0) {`
Original file line number	Diff line number	Diff line change
`@@ -52,6 +52,7 @@ module.exports = function npmRunAll(args, stdout, stderr) {`
`52`	`52`	`arguments: argv.rest,`
`53`	`53`	`race: group.parallel && argv.race,`
`54`	`54`	`npmPath: argv.npmPath,`
	`55`	`+ aggregateOutput: group.parallel && argv.aggregateOutput,`
`55`	`56`	`}`
`56`	`57`	`))`
`57`	`58`	`},`
Original file line number	Diff line number	Diff line change
`@@ -236,8 +236,11 @@ module.exports = function npmRunAll(patternOrPatterns, options) { //eslint-disab`
`236`	`236`	`if (typeof maxParallel !== "number" \|\| !(maxParallel >= 0)) {`
`237`	`237`	`throw new Error("Invalid options.maxParallel")`
`238`	`238`	`}`
	`239`	`+ if (!parallel && aggregateOutput) {`
	`240`	`+ throw new Error("Invalid options.aggregateOutput; It requires options.parallel")`
	`241`	`+ }`
`239`	`242`	`if (!parallel && race) {`
`240`		`- throw new Error("Invalid options.race")`
	`243`	`+ throw new Error("Invalid options.race; It requires options.parallel")`
`241`	`244`	`}`
`242`	`245`
`243`	`246`	`const prefixOptions = [].concat(`