fix(testrunner): report suite-level errors (#3661)

test-runner/src/runner.ts

@@ -43,7 +43,7 @@ export class Runner {
     this._suite = suite;
     for (const suite of this._suite.suites) {
       suite.findTest(test => {
-        this._testById.set(`${test._ordinal}@${suite.file}::[${suite._configurationString}]`, { test, result: test._appendResult() });
+        this._testById.set(test._id, { test, result: test._appendResult() });
       });
     }
 
@@ -58,12 +58,12 @@ export class Runner {
   _filesSortedByWorkerHash(): TestRunnerEntry[] {
     const result: TestRunnerEntry[] = [];
     for (const suite of this._suite.suites) {
-      const ordinals: number[] = [];
+      const ids: string[] = [];
-      suite.findTest(test => ordinals.push(test._ordinal) && false);
+      suite.findTest(test => ids.push(test._id) && false);
-      if (!ordinals.length)
+      if (!ids.length)
         continue;
       result.push({
-        ordinals,
+        ids,
         file: suite.file,
         configuration: suite.configuration,
         configurationString: suite._configurationString,
@@ -98,7 +98,7 @@ export class Runner {
     await Promise.all(jobs);
   }
 
-  async _runJob(worker, entry) {
+  async _runJob(worker: Worker, entry: TestRunnerEntry) {
     worker.run(entry);
     let doneCallback;
     const result = new Promise(f => doneCallback = f);
@@ -112,8 +112,16 @@ export class Runner {
       // When worker encounters error, we will restart it.
       this._restartWorker(worker);
 
-      // In case of fatal error, we are done with the entry.
+      // In case of fatal error without test id, we are done with the entry.
-      if (params.fatalError) {
+      if (params.fatalError && !params.failedTestId) {
+        // Report all the tests are failing with this error.
+        for (const id of entry.ids) {
+          const { test, result } = this._testById.get(id);
+          this._reporter.onTestBegin(test);
+          result.status = 'failed';
+          result.error = params.fatalError;
+          this._reporter.onTestEnd(test, result);
+        }
         doneCallback();
         return;
       }
@@ -123,11 +131,11 @@ export class Runner {
         const pair = this._testById.get(params.failedTestId);
         if (pair.test.results.length < this._config.retries + 1) {
           pair.result = pair.test._appendResult();
-          remaining.unshift(pair.test._ordinal);
+          remaining.unshift(pair.test._id);
         }
       }
       if (remaining.length)
-        this._queue.unshift({ ...entry, ordinals: remaining });
+        this._queue.unshift({ ...entry, ids: remaining });
 
       // This job is over, we just scheduled another one.
       doneCallback();

test-runner/src/test.ts

@@ -27,7 +27,7 @@ export class Test {
   fn: Function;
   results: TestResult[] = [];
 
-  _ordinal: number;
+  _id: string;
   _overriddenFn: Function;
   _startTime: number;
 
@@ -157,7 +157,7 @@ export class Suite {
     let ordinal = 0;
     this.findTest((test: Test) => {
       // All tests are identified with their ordinals.
-      test._ordinal = ordinal++;
+      test._id = `${ordinal++}@${this.file}::[${this._configurationString}]`;
     });
   }
 

test-runner/src/testCollector.ts

@@ -57,7 +57,6 @@ export class TestCollector {
     const revertBabelRequire = spec(suite, file, this._config.timeout);
     require(file);
     revertBabelRequire();
-    suite._renumber();
 
     const workerGeneratorConfigurations = new Map();
 
@@ -104,6 +103,7 @@ export class TestCollector {
         clone.title = path.basename(file) + (hash.length ? `::[${hash}]` : '') + ' ' + (i ? ` #repeat-${i}#` : '');
         clone.configuration = configuration;
         clone._configurationString = configurationString + `#repeat-${i}#`;
+        clone._renumber();
       }
     }
   }
@@ -122,7 +122,6 @@ export class TestCollector {
           continue;
         const testCopy = test._clone();
         testCopy.only = test.only;
-        testCopy._ordinal = test._ordinal;
         copy._addTest(testCopy);
       }
     }

test-runner/src/testRunner.ts

@@ -26,7 +26,7 @@ export const fixturePool = new FixturePool();
 
 export type TestRunnerEntry = {
   file: string;
-  ordinals: number[];
+  ids: string[];
   configurationString: string;
   configuration: Configuration;
   hash: string;
@@ -43,9 +43,8 @@ function chunkToParams(chunk: Buffer | string):  { text?: string, buffer?: strin
 export class TestRunner extends EventEmitter {
   private _failedTestId: string | undefined;
   private _fatalError: any | undefined;
-  private _file: any;
+  private _ids: Set<string>;
-  private _ordinals: Set<number>;
+  private _remaining: Set<string>;
-  private _remaining: Set<number>;
   private _trialRun: any;
   private _configuredFile: any;
   private _parsedGeneratorConfiguration: any = {};
@@ -55,12 +54,15 @@ export class TestRunner extends EventEmitter {
   private _stdOutBuffer: (string | Buffer)[] = [];
   private _stdErrBuffer: (string | Buffer)[] = [];
   private _testResult: TestResult | null = null;
+  private _suite: Suite;
 
   constructor(entry: TestRunnerEntry, config: RunnerConfig, workerId: number) {
     super();
-    this._file = entry.file;
+    this._suite = new Suite('');
-    this._ordinals = new Set(entry.ordinals);
+    this._suite.file = entry.file;
-    this._remaining = new Set(entry.ordinals);
+    this._suite._configurationString = entry.configurationString;
+    this._ids = new Set(entry.ids);
+    this._remaining = new Set(entry.ids);
     this._trialRun = config.trialRun;
     this._timeout = config.timeout;
     this._config = config;
@@ -68,7 +70,7 @@ export class TestRunner extends EventEmitter {
     for (const {name, value} of entry.configuration)
       this._parsedGeneratorConfiguration[name] = value;
     this._parsedGeneratorConfiguration['parallelIndex'] = workerId;
-    setCurrentTestFile(this._file);
+    setCurrentTestFile(this._suite.file);
   }
 
   stop() {
@@ -108,14 +110,13 @@ export class TestRunner extends EventEmitter {
   async run() {
     setParameters(this._parsedGeneratorConfiguration);
 
-    const suite = new Suite('');
+    const revertBabelRequire = spec(this._suite, this._suite.file, this._timeout);
-    const revertBabelRequire = spec(suite, this._file, this._timeout);
+    require(this._suite.file);
-    require(this._file);
     revertBabelRequire();
-    suite._renumber();
+    this._suite._renumber();
 
-    rerunRegistrations(this._file, 'test');
+    rerunRegistrations(this._suite.file, 'test');
-    await this._runSuite(suite);
+    await this._runSuite(this._suite);
     this._reportDone();
   }
 
@@ -144,11 +145,11 @@ export class TestRunner extends EventEmitter {
   private async _runTest(test: Test) {
     if (this._failedTestId)
       return false;
-    if (this._ordinals.size && !this._ordinals.has(test._ordinal))
+    if (this._ids.size && !this._ids.has(test._id))
       return;
-    this._remaining.delete(test._ordinal);
+    this._remaining.delete(test._id);
 
-    const id = `${test._ordinal}@${this._configuredFile}`;
+    const id = test._id;
     this._testId = id;
     this.emit('testBegin', { id });
 

test-runner/src/worker.ts

@@ -29,13 +29,13 @@ global.console = new Console({
 });
 
 process.stdout.write = chunk => {
-  if (testRunner && !closed)
+  if (testRunner)
     testRunner.stdout(chunk);
   return true;
 };
 
 process.stderr.write = chunk => {
-  if (testRunner && !closed)
+  if (testRunner)
     testRunner.stderr(chunk);
   return true;
 };
@@ -48,12 +48,12 @@ let workerId: number;
 let testRunner: TestRunner;
 
 process.on('unhandledRejection', (reason, promise) => {
-  if (testRunner && !closed)
+  if (testRunner)
     testRunner.fatalError(reason);
 });
 
 process.on('uncaughtException', error => {
-  if (testRunner && !closed)
+  if (testRunner)
     testRunner.fatalError(error);
 });
 
@@ -73,8 +73,6 @@ process.on('message', async message => {
       testRunner.on(event, sendMessageToParent.bind(null, event));
     await testRunner.run();
     testRunner = null;
-    // Mocha runner adds these; if we don't remove them, we'll get a leak.
-    process.removeAllListeners('uncaughtException');
   }
 });
 

test-runner/test/assets/suite-error.js

@@ -0,0 +1,23 @@
+/**
+ * Copyright (c) Microsoft Corporation.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+const { parameters } = require('../../');
+
+if (typeof parameters.parallelIndex === 'number')
+  throw new Error('Suite error');
+
+it('passes',() => {
+  expect(1 + 1).toBe(2);
+});

test-runner/test/exit-code.spec.ts

@@ -93,6 +93,13 @@ it('should repeat each', async () => {
     expect(suite.tests.length).toBe(1);
 });
 
+it('should report suite errors', async () => {
+  const { exitCode, failed, output } = await runTest('suite-error.js');
+  expect(exitCode).toBe(1);
+  expect(failed).toBe(1);
+  expect(output).toContain('Suite error');
+});
+
 async function runTest(filePath: string, params: any = {}) {
   const outputDir = path.join(__dirname, 'test-results');
   const reportFile = path.join(outputDir, 'results.json');