Repository: nodejs/node
Pull request: test: use more informative errors in `test-runner-cli`

The `1 is not equal to 0` errors give very little info on what's actually going wrong when the test fails. Checking stdout and stderr first almost always gives more informative errors.
<!--
Before submitting a pull request, please read:

- the CONTRIBUTING guide at https://github.com/nodejs/node/blob/HEAD/CONTRIBUTING.md
- the commit message formatting guidelines at
  https://github.com/nodejs/node/blob/HEAD/doc/contributing/pull-requests.md#commit-message-guidelines

For code changes:
1. Include tests for any bug fixes or new features.
2. Update documentation if relevant.
3. Ensure that `make -j4 test` (UNIX), or `vcbuild test` (Windows) passes.

If you believe this PR should be highlighted in the Node.js CHANGELOG
please add the `notable-change` label.

Developer's Certificate of Origin 1.1

By making a contribution to this project, I certify that:

(a) The contribution was created in whole or in part by me and I
    have the right to submit it under the open source license
    indicated in the file; or

(b) The contribution is based upon previous work that, to the best
    of my knowledge, is covered under an appropriate open source
    license and I have the right under that license to submit that
    work with modifications, whether created in whole or in part
    by me, under the same open source license (unless I am
    permitted to submit under a different license), as indicated
    in the file; or

(c) The contribution was provided directly to me by some other
    person who certified (a), (b) or (c) and I have not modified
    it.

(d) I understand and agree that this project and the contribution
    are public and that a record of the contribution (including all
    personal information I submit with it, including my sign-off) is
    maintained indefinitely and may be redistributed consistent with
    this project or the open source license(s) involved.
-->


Changed files:
filename: test/parallel/test-runner-cli.js
patch: @@ -67,8 +67,6 @@ for (const isolation of ['none', 'process']) {
                   `--experimental-${type}-types`, `--experimental-test-isolation=${isolation}`];
     const child = spawnSync(process.execPath, args, { cwd: join(testFixtures, 'matching-patterns') });
 
-    assert.strictEqual(child.status, 0);
-    assert.strictEqual(child.signal, null);
     assert.strictEqual(child.stderr.toString(), '');
     const stdout = child.stdout.toString();
 
@@ -78,6 +76,8 @@ for (const isolation of ['none', 'process']) {
     assert.match(stdout, /ok 4 - this should pass/);
     assert.match(stdout, /ok 5 - this should pass/);
     assert.match(stdout, /ok 6 - this should pass/);
+    assert.strictEqual(child.status, 0);
+    assert.strictEqual(child.signal, null);
   }
 
   {
@@ -97,9 +97,9 @@ for (const isolation of ['none', 'process']) {
     assert.match(stdout, /ok 4 - this should pass/);
     assert.match(stdout, /ok 5 - this should be skipped/);
     assert.match(stdout, /ok 6 - this should be executed/);
+    assert.strictEqual(child.stderr.toString(), '');
     assert.strictEqual(child.status, 1);
     assert.strictEqual(child.signal, null);
-    assert.strictEqual(child.stderr.toString(), '');
   }
 
   {
@@ -112,11 +112,11 @@ for (const isolation of ['none', 'process']) {
     ];
     const child = spawnSync(process.execPath, args, { cwd: testFixtures });
 
-    assert.strictEqual(child.status, 1);
-    assert.strictEqual(child.signal, null);
     assert.strictEqual(child.stderr.toString(), '');
     const stdout = child.stdout.toString();
     assert.match(stdout, /not ok 1 - .+index\.js/);
+    assert.strictEqual(child.status, 1);
+    assert.strictEqual(child.signal, null);
   }
 
   {
@@ -129,11 +129,11 @@ for (const isolation of ['none', 'process']) {
     ];
     const child = spawnSync(process.execPath, args);
 
-    assert.strictEqual(child.status, 1);
-    assert.strictEqual(child.signal, null);
     assert.strictEqual(child.stderr.toString(), '');
     const stdout = child.stdout.toString();
     assert.match(stdout, /not ok 1 - .+test-nm\.js/);
+    assert.strictEqual(child.status, 1);
+    assert.strictEqual(child.signal, null);
   }
 
   {
@@ -142,8 +142,6 @@ for (const isolation of ['none', 'process']) {
     const options = { cwd: join(testFixtures, 'default-behavior') };
     const child = spawnSync(process.execPath, args, options);
 
-    assert.strictEqual(child.status, 1);
-    assert.strictEqual(child.signal, null);
     assert.strictEqual(child.stderr.toString(), '');
     const stdout = child.stdout.toString();
     assert.match(stdout, /this should pass/);
@@ -152,6 +150,9 @@ for (const isolation of ['none', 'process']) {
     assert.match(stdout, /this should pass/);
     assert.match(stdout, /this should be skipped/);
     assert.match(stdout, /this should be executed/);
+
+    assert.strictEqual(child.status, 1);
+    assert.strictEqual(child.signal, null);
   }
 
   {
@@ -166,8 +167,6 @@ for (const isolation of ['none', 'process']) {
     ];
     const child = spawnSync(process.execPath, args);
 
-    assert.strictEqual(child.status, 1);
-    assert.strictEqual(child.signal, null);
     assert.strictEqual(child.stderr.toString(), '');
     const stdout = child.stdout.toString();
     assert.match(stdout, /# Subtest: this should pass/);
@@ -204,6 +203,9 @@ for (const isolation of ['none', 'process']) {
     assert.match(stdout, /# cancelled 0/);
     assert.match(stdout, /# skipped 1/);
     assert.match(stdout, /# todo 0/);
+
+    assert.strictEqual(child.status, 1);
+    assert.strictEqual(child.signal, null);
   }
 }
 
@@ -219,11 +221,11 @@ for (const isolation of ['none', 'process']) {
   for (const args of flags) {
     const child = spawnSync(process.execPath, args);
 
-    assert.notStrictEqual(child.status, 0);
-    assert.strictEqual(child.signal, null);
     assert.strictEqual(child.stdout.toString(), '');
     const stderr = child.stderr.toString();
     assert.match(stderr, /--test/);
+    assert.notStrictEqual(child.status, 0);
+    assert.strictEqual(child.signal, null);
   }
 }
 
@@ -236,8 +238,6 @@ for (const isolation of ['none', 'process']) {
   ];
   const child = spawnSync(process.execPath, args);
 
-  assert.strictEqual(child.status, 0);
-  assert.strictEqual(child.signal, null);
   assert.strictEqual(child.stderr.toString(), '');
   const stdout = child.stdout.toString();
   assert.match(stdout, /# stderr 1/);
@@ -251,6 +251,9 @@ for (const isolation of ['none', 'process']) {
   assert.match(stdout, /ok 1 - a test/);
   assert.match(stdout, /# tests 1/);
   assert.match(stdout, /# pass 1/);
+
+  assert.strictEqual(child.status, 0);
+  assert.strictEqual(child.signal, null);
 }
 
 {
@@ -263,70 +266,70 @@ for (const isolation of ['none', 'process']) {
   const child = spawnSync(process.execPath, args);
 
   assert.strictEqual(child.stderr.toString(), '');
-  assert.strictEqual(child.status, 0);
-  assert.strictEqual(child.signal, null);
   const stdout = child.stdout.toString();
   assert.match(stdout, /this should pass/);
+  assert.strictEqual(child.status, 0);
+  assert.strictEqual(child.signal, null);
 }
 
 {
   // --test-shard option validation
   const args = ['--test', '--test-shard=1', join(testFixtures, 'index.js')];
   const child = spawnSync(process.execPath, args, { cwd: testFixtures });
 
-  assert.strictEqual(child.status, 1);
-  assert.strictEqual(child.signal, null);
   assert.match(child.stderr.toString(), /The argument '--test-shard' must be in the form of <index>\/<total>\. Received '1'/);
   const stdout = child.stdout.toString();
   assert.strictEqual(stdout, '');
+  assert.strictEqual(child.status, 1);
+  assert.strictEqual(child.signal, null);
 }
 
 {
   // --test-shard option validation
   const args = ['--test', '--test-shard=1/2/3', join(testFixtures, 'index.js')];
   const child = spawnSync(process.execPath, args, { cwd: testFixtures });
 
-  assert.strictEqual(child.status, 1);
-  assert.strictEqual(child.signal, null);
   assert.match(child.stderr.toString(), /The argument '--test-shard' must be in the form of <index>\/<total>\. Received '1\/2\/3'/);
   const stdout = child.stdout.toString();
   assert.strictEqual(stdout, '');
+  assert.strictEqual(child.status, 1);
+  assert.strictEqual(child.signal, null);
 }
 
 {
   // --test-shard option validation
   const args = ['--test', '--test-shard=0/3', join(testFixtures, 'index.js')];
   const child = spawnSync(process.execPath, args, { cwd: testFixtures });
 
-  assert.strictEqual(child.status, 1);
-  assert.strictEqual(child.signal, null);
   assert.match(child.stderr.toString(), /The value of "options\.shard\.index" is out of range\. It must be >= 1 && <= 3\. Received 0/);
   const stdout = child.stdout.toString();
   assert.strictEqual(stdout, '');
+  assert.strictEqual(child.status, 1);
+  assert.strictEqual(child.signal, null);
 }
 
 {
   // --test-shard option validation
   const args = ['--test', '--test-shard=0xf/20abcd', join(testFixtures, 'index.js')];
   const child = spawnSync(process.execPath, args, { cwd: testFixtures });
 
-  assert.strictEqual(child.status, 1);
-  assert.strictEqual(child.signal, null);
   assert.match(child.stderr.toString(), /The argument '--test-shard' must be in the form of <index>\/<total>\. Received '0xf\/20abcd'/);
   const stdout = child.stdout.toString();
   assert.strictEqual(stdout, '');
+  assert.strictEqual(child.status, 1);
+  assert.strictEqual(child.signal, null);
 }
 
 {
   // --test-shard option validation
   const args = ['--test', '--test-shard=hello', join(testFixtures, 'index.js')];
   const child = spawnSync(process.execPath, args, { cwd: testFixtures });
 
-  assert.strictEqual(child.status, 1);
-  assert.strictEqual(child.signal, null);
   assert.match(child.stderr.toString(), /The argument '--test-shard' must be in the form of <index>\/<total>\. Received 'hello'/);
   const stdout = child.stdout.toString();
   assert.strictEqual(stdout, '');
+  assert.strictEqual(child.status, 1);
+  assert.strictEqual(child.signal, null);
 }
 
 {
@@ -339,8 +342,6 @@ for (const isolation of ['none', 'process']) {
   ];
   const child = spawnSync(process.execPath, args);
 
-  assert.strictEqual(child.status, 0);
-  assert.strictEqual(child.signal, null);
   assert.strictEqual(child.stderr.toString(), '');
   const stdout = child.stdout.toString();
   assert.match(stdout, /# Subtest: a\.cjs this should pass/);
@@ -362,6 +363,9 @@ for (const isolation of ['none', 'process']) {
   assert.match(stdout, /# pass 5/);
   assert.match(stdout, /# fail 0/);
   assert.match(stdout, /# skipped 0/);
+
+  assert.strictEqual(child.status, 0);
+  assert.strictEqual(child.signal, null);
 }
 
 {
@@ -374,8 +378,6 @@ for (const isolation of ['none', 'process']) {
   ];
   const child = spawnSync(process.execPath, args);
 
-  assert.strictEqual(child.status, 0);
-  assert.strictEqual(child.signal, null);
   assert.strictEqual(child.stderr.toString(), '');
   const stdout = child.stdout.toString();
   assert.match(stdout, /# Subtest: b\.cjs this should pass/);
@@ -397,15 +399,16 @@ for (const isolation of ['none', 'process']) {
   assert.match(stdout, /# pass 5/);
   assert.match(stdout, /# fail 0/);
   assert.match(stdout, /# skipped 0/);
+
+  assert.strictEqual(child.status, 0);
+  assert.strictEqual(child.signal, null);
 }
 
 {
   // Should not match files like latest.js
   const args = ['--test', '--test-reporter=tap'];
   const child = spawnSync(process.execPath, args, { cwd: join(testFixtures, 'issue-54726') });
 
-  assert.strictEqual(child.status, 0);
-  assert.strictEqual(child.signal, null);
   assert.strictEqual(child.stderr.toString(), '');
   const stdout = child.stdout.toString();
 
@@ -416,4 +419,7 @@ for (const isolation of ['none', 'process']) {
   assert.match(stdout, /cancelled 0/);
   assert.match(stdout, /skipped 0/);
   assert.match(stdout, /todo 0/);
+
+  assert.strictEqual(child.status, 0);
+  assert.strictEqual(child.signal, null);
 }