diff --git a/test-runner/src/runner.ts b/test-runner/src/runner.ts index dae49bfed7..f9dec0af08 100644 --- a/test-runner/src/runner.ts +++ b/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[] = []; - suite.findTest(test => ordinals.push(test._ordinal) && false); - if (!ordinals.length) + const ids: string[] = []; + suite.findTest(test => ids.push(test._id) && false); + 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. - if (params.fatalError) { + // In case of fatal error without test id, we are done with the entry. + 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(); diff --git a/test-runner/src/test.ts b/test-runner/src/test.ts index f9aee9ead2..ebfff51b45 100644 --- a/test-runner/src/test.ts +++ b/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}]`; }); } diff --git a/test-runner/src/testCollector.ts b/test-runner/src/testCollector.ts index be8d82eb82..7e13d5ee96 100644 --- a/test-runner/src/testCollector.ts +++ b/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); } } diff --git a/test-runner/src/testRunner.ts b/test-runner/src/testRunner.ts index d6691ab087..6c1e8a5c10 100644 --- a/test-runner/src/testRunner.ts +++ b/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 _ordinals: Set; - private _remaining: Set; + private _ids: Set; + private _remaining: Set; 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._ordinals = new Set(entry.ordinals); - this._remaining = new Set(entry.ordinals); + this._suite = new Suite(''); + this._suite.file = entry.file; + 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(suite, this._file, this._timeout); - require(this._file); + const revertBabelRequire = spec(this._suite, this._suite.file, this._timeout); + require(this._suite.file); revertBabelRequire(); - suite._renumber(); + this._suite._renumber(); - rerunRegistrations(this._file, 'test'); - await this._runSuite(suite); + rerunRegistrations(this._suite.file, 'test'); + 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 }); diff --git a/test-runner/src/worker.ts b/test-runner/src/worker.ts index 08ac48d798..fbe2d1479e 100644 --- a/test-runner/src/worker.ts +++ b/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'); } }); diff --git a/test-runner/test/assets/suite-error.js b/test-runner/test/assets/suite-error.js new file mode 100644 index 0000000000..cf244a37f0 --- /dev/null +++ b/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); +}); diff --git a/test-runner/test/exit-code.spec.ts b/test-runner/test/exit-code.spec.ts index ded73a2e3c..93835620ca 100644 --- a/test-runner/test/exit-code.spec.ts +++ b/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');