fix(testrunner): allow worker fixture to throw/timeout (#3610)

test-runner/src/fixtures.ts

@@ -101,8 +101,8 @@ class Fixture<Config> {
     if (this._setup) {
       debug('pw:test:hook')(`teardown "${this.name}"`);
       this._teardownFenceCallback();
-    }
       await this._tearDownComplete;
+    }
     this.pool.instances.delete(this.name);
   }
 }
@@ -134,31 +134,27 @@ export class FixturePool<Config> {
     }
   }
 
-  async resolveParametersAndRun(fn: Function, timeout: number, config: Config, test?: Test) {
+  async resolveParametersAndRun(fn: Function, config: Config, test?: Test) {
     const names = fixtureParameterNames(fn);
     for (const name of names)
       await this.setupFixture(name, config, test);
     const params = {};
     for (const n of names)
       params[n] = this.instances.get(n).value;
-
-    if (!timeout)
     return fn(params);
-
-    let timer: NodeJS.Timer;
-    let timerPromise = new Promise(f => timer = setTimeout(f, timeout));
-    return Promise.race([
-      Promise.resolve(fn(params)).then(() => clearTimeout(timer)),
-      timerPromise.then(() => Promise.reject(new Error(`Timeout of ${timeout}ms exceeded`)))
-    ]);
   }
 
   wrapTestCallback(callback: any, timeout: number, config: Config, test: Test) {
     if (!callback)
       return callback;
     return async() => {
+      let timer: NodeJS.Timer;
+      let timerPromise = new Promise(f => timer = setTimeout(f, timeout));
       try {
-        await this.resolveParametersAndRun(callback, timeout, config, test);
+        await Promise.race([
+          this.resolveParametersAndRun(callback, config, test).then(() => clearTimeout(timer)),
+          timerPromise.then(() => Promise.reject(new Error(`Timeout of ${timeout}ms exceeded`)))
+        ]);
       } catch (e) {
         test.error = serializeError(e);
         throw e;

test-runner/src/runner.ts

@@ -233,6 +233,7 @@ class OopWorker extends Worker {
       stdio: ['ignore', 'ignore', 'ignore', 'ipc']
     });
     this.process.on('exit', () => this.emit('exit'));
+    this.process.on('error', (e) => {});  // do not yell at a send to dead process.
     this.process.on('message', message => {
       const { method, params } = message;
       this.emit(method, params);

test-runner/src/test.ts

@@ -165,7 +165,7 @@ export function serializeConfiguration(configuration: Configuration): string {
   return tokens.join(', ');
 }
 
-export function serializeError(error: Error): any {
+export function serializeError(error: Error | any): any {
   if (error instanceof Error) {
     return {
       message: error.message,

test-runner/src/testRunner.ts

@@ -43,6 +43,7 @@ export class TestRunner extends EventEmitter {
   private _parsedGeneratorConfiguration: any = {};
   private _config: RunnerConfig;
   private _timeout: number;
+  private _test: Test | null = null;
 
   constructor(entry: TestRunnerEntry, config: RunnerConfig, workerId: number) {
     super();
@@ -63,6 +64,17 @@ export class TestRunner extends EventEmitter {
     this._trialRun = true;
   }
 
+  fatalError(error: Error | any) {
+    this._fatalError = serializeError(error);
+    if (this._test) {
+      this._test.error = this._fatalError;
+      this.emit('fail', {
+        test: this._serializeTest(),
+      });
+    }
+    this._reportDone();
+  }
+
   async run() {
     setParameters(this._parsedGeneratorConfiguration);
 
@@ -102,30 +114,32 @@ export class TestRunner extends EventEmitter {
   private async _runTest(test: Test) {
     if (this._failedWithError)
       return false;
+    this._test = test;
     const ordinal = ++this._currentOrdinal;
     if (this._ordinals.size && !this._ordinals.has(ordinal))
       return;
     this._remaining.delete(ordinal);
     if (test.pending || test.suite._isPending()) {
-      this.emit('pending', { test: this._serializeTest(test) });
+      this.emit('pending', { test: this._serializeTest() });
       return;
     }
 
-    this.emit('test', { test: this._serializeTest(test) });
+    this.emit('test', { test: this._serializeTest() });
     try {
       await this._runHooks(test.suite, 'beforeEach', 'before');
       test._startTime = Date.now();
       if (!this._trialRun)
         await this._testWrapper(test)();
-      this.emit('pass', { test: this._serializeTest(test) });
+      this.emit('pass', { test: this._serializeTest() });
       await this._runHooks(test.suite, 'afterEach', 'after');
     } catch (error) {
       test.error = serializeError(error);
       this._failedWithError = test.error;
       this.emit('fail', {
-        test: this._serializeTest(test),
+        test: this._serializeTest(),
       });
     }
+    this._test = null;
   }
 
   private async _runHooks(suite: Suite, type: string, dir: 'before' | 'after') {
@@ -139,7 +153,7 @@ export class TestRunner extends EventEmitter {
     if (dir === 'before')
       all.reverse();
     for (const hook of all)
-      await fixturePool.resolveParametersAndRun(hook, 0, this._config);
+      await fixturePool.resolveParametersAndRun(hook, this._config);
   }
 
   private _reportDone() {
@@ -155,11 +169,11 @@ export class TestRunner extends EventEmitter {
     return fixturePool.wrapTestCallback(test.fn, timeout, { ...this._config }, test);
   }
 
-  private _serializeTest(test) {
+  private _serializeTest() {
     return {
-      id: `${test._ordinal}@${this._configuredFile}`,
+      id: `${this._test._ordinal}@${this._configuredFile}`,
-      error: test.error,
+      error: this._test.error,
-      duration: Date.now() - test._startTime,
+      duration: Date.now() - this._test._startTime,
     };
   }
 }

test-runner/src/worker.ts

@@ -47,6 +47,16 @@ process.on('SIGTERM',() => {});
 let workerId: number;
 let testRunner: TestRunner;
 
+process.on('unhandledRejection', (reason, promise) => {
+  if (testRunner && !closed)
+    testRunner.fatalError(reason);
+});
+
+process.on('uncaughtException', error => {
+  if (testRunner && !closed)
+    testRunner.fatalError(error);
+});
+
 process.on('message', async message => {
   if (message.method === 'init') {
     workerId = message.params.workerId;
@@ -76,7 +86,7 @@ async function gracefullyCloseAndExit() {
   setTimeout(() => process.exit(0), 30000);
   // Meanwhile, try to gracefully close all browsers.
   if (testRunner)
-    await testRunner.stop();
+    testRunner.stop();
   await fixturePool.teardownScope('worker');
   process.exit(0);
 }

test-runner/test/assets/worker-fixture-error.js

@@ -0,0 +1,24 @@
+/**
+ * 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 { registerWorkerFixture } = require('../../');
+
+registerWorkerFixture('failure', async ({}, runTest) => {
+	throw new Error('Worker failed');
+});
+
+it('fails', async({failure}) => {	
+});

test-runner/test/assets/worker-fixture-timeout.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 { registerWorkerFixture } = require('../../');
+
+registerWorkerFixture('timeout', async ({}, runTest) => {
+});
+
+it('fails', async({timeout}) => {	
+});

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

@@ -44,20 +44,33 @@ it('should access error in fixture', async() => {
   expect(data.message).toContain('Object.is equality');
 });
 
-async function runTest(filePath: string) {
+it('should handle worker fixture timeout', async() => {
+  const result = await runTest('worker-fixture-timeout.js', 1000);
+  expect(result.exitCode).toBe(1);
+  expect(result.output).toContain('Timeout of 1000ms');
+});
+
+it('should handle worker fixture error', async() => {
+  const result = await runTest('worker-fixture-error.js');
+  expect(result.exitCode).toBe(1);
+  expect(result.output).toContain('Worker failed');
+});
+
+async function runTest(filePath: string, timeout = 10000) {
   const outputDir = path.join(__dirname, 'test-results')
   await removeFolderAsync(outputDir).catch(e => {});
 
   const { output, status } = spawnSync('node', [
     path.join(__dirname, '..', 'cli.js'),
     path.join(__dirname, 'assets', filePath),
-    '--output=' + outputDir
+    '--output=' + outputDir,
+    '--timeout=' + timeout
   ]);
   const passed = (/(\d+) passed/.exec(output.toString()) || [])[1];
   const failed = (/(\d+) failed/.exec(output.toString()) || [])[1];
   return {
     exitCode: status,
-    output,
+    output: output.toString(),
     passed: parseInt(passed),
     failed: parseInt(failed || '0')
   }