feat(testrunner): migrate from events to a delegate (#1647)

This allows an async handler for each event that can be awaited.
Drive-by: merge TestPass into TestRunner.

utils/testrunner/Reporter.js

@@ -33,13 +33,10 @@ class Reporter {
     this._verbose = verbose;
     this._summary = summary;
     this._testCounter = 0;
-    runner.on('started', this._onStarted.bind(this));
+    runner.setDelegate(this);
-    runner.on('finished', this._onFinished.bind(this));
-    runner.on('teststarted', this._onTestStarted.bind(this));
-    runner.on('testfinished', this._onTestFinished.bind(this));
   }
 
-  _onStarted(testRuns) {
+  onStarted(testRuns) {
     this._testCounter = 0;
     this._timestamp = Date.now();
     const allTests = this._runner.tests();
@@ -83,7 +80,7 @@ class Reporter {
     console.log('');
   }
 
-  _onFinished(result) {
+  onFinished(result) {
     this._printTestResults(result);
     if (!result.ok())
       this._printFailedResult(result);
@@ -149,10 +146,10 @@ class Reporter {
     console.log(`Finished in ${colors.yellow(seconds)} seconds`);
   }
 
-  _onTestStarted(testRun) {
+  onTestRunStarted(testRun) {
   }
 
-  _onTestFinished(testRun) {
+  onTestRunFinished(testRun) {
     if (this._verbose) {
       ++this._testCounter;
       this._printVerboseTestRunResult(this._testCounter, testRun);

utils/testrunner/TestRunner.js

@@ -15,7 +15,6 @@
  * limitations under the License.
  */
 
-const EventEmitter = require('events');
 const {SourceMapSupport} = require('./SourceMapSupport');
 const debug = require('debug');
 const Location = require('./Location');
@@ -368,8 +367,8 @@ class Result {
 }
 
 class TestWorker {
-  constructor(testPass, workerId, parallelIndex) {
+  constructor(testRunner, workerId, parallelIndex) {
-    this._testPass = testPass;
+    this._testRunner = testRunner;
     this._state = { parallelIndex };
     this._environmentStack = [];
     this._terminating = false;
@@ -474,7 +473,7 @@ class TestWorker {
       testRun._error = await promise;
       this._runningTestTerminate = null;
       if (testRun._error && testRun._error.stack)
-        await this._testPass._runner._sourceMapSupport.rewriteStackTraceWithSourceMaps(testRun._error);
+        await this._testRunner._sourceMapSupport.rewriteStackTraceWithSourceMaps(testRun._error);
       if (!testRun._error)
         testRun._result = TestResult.Ok;
       else if (testRun._error === TimeoutError)
@@ -497,7 +496,7 @@ class TestWorker {
 
   async _runHook(testRun, hook, fullName, passTest = false) {
     await this._willStartHook(hook, fullName);
-    const timeout = this._testPass._runner._timeout;
+    const timeout = this._testRunner._timeout;
     const { promise, terminate } = runUserCallback(hook.body, timeout, passTest ? [this._state, testRun.test()] : [this._state]);
     this._runningHookTerminate = terminate;
     let error = await promise;
@@ -518,7 +517,7 @@ class TestWorker {
         error = null;
       } else {
         if (error.stack)
-          await this._testPass._runner._sourceMapSupport.rewriteStackTraceWithSourceMaps(error);
+          await this._testRunner._sourceMapSupport.rewriteStackTraceWithSourceMaps(error);
         message = `${hook.location.toDetailedString()} - FAILED while running "${hook.name}" in suite "${fullName}": `;
       }
       await this._didFailHook(hook, fullName, message, error);
@@ -534,13 +533,13 @@ class TestWorker {
   async _willStartTestRun(testRun) {
     testRun._startTimestamp = Date.now();
     testRun._workerId = this._workerId;
-    this._testPass._runner.emit(TestRunner.Events.TestStarted, testRun);
+    await this._testRunner._delegate.onTestRunStarted(testRun);
   }
 
   async _didFinishTestRun(testRun) {
     testRun._endTimestamp = Date.now();
     testRun._workerId = this._workerId;
-    this._testPass._runner.emit(TestRunner.Events.TestFinished, testRun);
+    await this._testRunner._delegate.onTestRunFinished(testRun);
   }
 
   async _willStartTestBody(testRun) {
@@ -558,8 +557,8 @@ class TestWorker {
   async _didFailHook(hook, fullName, message, error) {
     debug('testrunner:hook')(`[${this._workerId}] "${hook.name}" FAILED for "${fullName}" (${hook.location})`);
     if (message)
-      this._testPass._result.addError(message, error, this);
+      this._testRunner._result.addError(message, error, this);
-    this._testPass._result.setResult(TestResult.Crashed, message);
+    this._testRunner._result.setResult(TestResult.Crashed, message);
   }
 
   async _didCompleteHook(hook, fullName) {
@@ -575,108 +574,8 @@ class TestWorker {
   }
 }
 
-class TestPass {
+class TestRunner {
-  constructor(runner, parallel, breakOnFailure) {
-    this._runner = runner;
-    this._workers = [];
-    this._nextWorkerId = 1;
-    this._parallel = parallel;
-    this._breakOnFailure = breakOnFailure;
-    this._errors = [];
-    this._result = new Result();
-    this._terminating = false;
-  }
-
-  async run(testRuns) {
-    const terminations = [
-      createTermination.call(this, 'SIGINT', TestResult.Terminated, 'SIGINT received'),
-      createTermination.call(this, 'SIGHUP', TestResult.Terminated, 'SIGHUP received'),
-      createTermination.call(this, 'SIGTERM', TestResult.Terminated, 'SIGTERM received'),
-      createTermination.call(this, 'unhandledRejection', TestResult.Crashed, 'UNHANDLED PROMISE REJECTION'),
-      createTermination.call(this, 'uncaughtException', TestResult.Crashed, 'UNHANDLED ERROR'),
-    ];
-    for (const termination of terminations)
-      process.on(termination.event, termination.handler);
-
-    this._result = new Result();
-    this._result.runs = testRuns;
-
-    const parallel = Math.min(this._parallel, testRuns.length);
-    const workerPromises = [];
-    for (let i = 0; i < parallel; ++i) {
-      const initialTestRunIndex = i * Math.floor(testRuns.length / parallel);
-      workerPromises.push(this._runWorker(initialTestRunIndex, testRuns, i));
-    }
-    await Promise.all(workerPromises);
-
-    for (const termination of terminations)
-      process.removeListener(termination.event, termination.handler);
-
-    if (testRuns.some(run => run.isFailure()))
-      this._result.setResult(TestResult.Failed, '');
-    return this._result;
-
-    function createTermination(event, result, message) {
-      return {
-        event,
-        message,
-        handler: error => this._terminate(result, message, event === 'SIGTERM', event.startsWith('SIG') ? null : error)
-      };
-    }
-  }
-
-  async _runWorker(testRunIndex, testRuns, parallelIndex) {
-    let worker = new TestWorker(this, this._nextWorkerId++, parallelIndex);
-    this._workers[parallelIndex] = worker;
-    while (!this._terminating) {
-      let skipped = 0;
-      while (skipped < testRuns.length && testRuns[testRunIndex]._result !== null) {
-        testRunIndex = (testRunIndex + 1) % testRuns.length;
-        skipped++;
-      }
-      const testRun = testRuns[testRunIndex];
-      if (testRun._result !== null) {
-        // All tests have been run.
-        break;
-      }
-
-      // Mark as running so that other workers do not run it again.
-      testRun._result = 'running';
-      await worker.run(testRun);
-      if (testRun.isFailure()) {
-        // Something went wrong during test run, let's use a fresh worker.
-        await worker.shutdown();
-        if (this._breakOnFailure) {
-          const message = `Terminating because a test has failed and |testRunner.breakOnFailure| is enabled`;
-          await this._terminate(TestResult.Terminated, message, false /* force */, null /* error */);
-          return;
-        }
-        worker = new TestWorker(this, this._nextWorkerId++, parallelIndex);
-        this._workers[parallelIndex] = worker;
-      }
-    }
-    await worker.shutdown();
-  }
-
-  async _terminate(result, message, force, error) {
-    debug('testrunner')(`TERMINATED result = ${result}, message = ${message}`);
-    this._terminating = true;
-    for (const worker of this._workers)
-      worker.terminate(force /* terminateHooks */);
-    this._result.setResult(result, message);
-    if (this._result.message === 'SIGINT received' && message === 'SIGTERM received')
-      this._result.message = message;
-    if (error) {
-      if (error.stack)
-        await this._runner._sourceMapSupport.rewriteStackTraceWithSourceMaps(error);
-      this._result.addError(message, error, this._workers.length === 1 ? this._workers[0] : null);
-    }
-  }
-}
-
-class TestRunner extends EventEmitter {
   constructor(options = {}) {
-    super();
     const {
       timeout = 10 * 1000, // Default timeout is 10 seconds.
       parallel = 1,
@@ -697,6 +596,17 @@ class TestRunner extends EventEmitter {
     this._suiteAttributes = new Map();
     this._testModifiers = new Map();
     this._testAttributes = new Map();
+    this._nextWorkerId = 1;
+    this._workers = [];
+    this._terminating = false;
+    this._result = null;
+
+    this._delegate = {
+      async onStarted(testRuns) {},
+      async onFinished(result) {},
+      async onTestRunStarted(testRun) {},
+      async onTestRunFinished(testRun) {},
+    };
 
     this.beforeAll = (callback) => this._currentEnvironment.beforeAll(callback);
     this.beforeEach = (callback) => this._currentEnvironment.beforeEach(callback);
@@ -785,6 +695,10 @@ class TestRunner extends EventEmitter {
     this._suiteAttributes.set(name, callback);
   }
 
+  setDelegate(delegate) {
+    this._delegate = delegate;
+  }
+
   async run(options = {}) {
     const { totalTimeout = 0 } = options;
     const testRuns = [];
@@ -795,31 +709,115 @@ class TestRunner extends EventEmitter {
       for (let i = 0; i < repeat; i++)
         testRuns.push(new TestRun(test));
     }
-    this.emit(TestRunner.Events.Started, testRuns);
 
-    let result;
+    this._result = new Result();
+
     if (this._crashIfTestsAreFocusedOnCI && process.env.CI && this.hasFocusedTestsOrSuites()) {
-      result = new Result();
+      await this._delegate.onStarted([]);
-      result.setResult(TestResult.Crashed, '"focused" tests or suites are probitted on CI');
+      this._result.setResult(TestResult.Crashed, '"focused" tests or suites are probitted on CI');
+      await this._delegate.onFinished(this._result);
     } else {
-      this._runningPass = new TestPass(this, this._parallel, this._breakOnFailure);
+      await this._delegate.onStarted(testRuns);
+      this._result.runs = testRuns;
+
       let timeoutId;
       if (totalTimeout) {
         timeoutId = setTimeout(() => {
-          this._runningPass._terminate(TestResult.Terminated, `Total timeout of ${totalTimeout}ms reached.`, true /* force */, null /* error */);
+          this._terminate(TestResult.Terminated, `Total timeout of ${totalTimeout}ms reached.`, true /* force */, null /* error */);
         }, totalTimeout);
       }
-      try {
+
-        result = await this._runningPass.run(testRuns).catch(e => { console.error(e); throw e; });
+      const terminations = [
-      } finally {
+        createTermination.call(this, 'SIGINT', TestResult.Terminated, 'SIGINT received'),
-        this._runningPass = null;
+        createTermination.call(this, 'SIGHUP', TestResult.Terminated, 'SIGHUP received'),
-        clearTimeout(timeoutId);
+        createTermination.call(this, 'SIGTERM', TestResult.Terminated, 'SIGTERM received'),
+        createTermination.call(this, 'unhandledRejection', TestResult.Crashed, 'UNHANDLED PROMISE REJECTION'),
+        createTermination.call(this, 'uncaughtException', TestResult.Crashed, 'UNHANDLED ERROR'),
+      ];
+      for (const termination of terminations)
+        process.on(termination.event, termination.handler);
+
+      const parallel = Math.min(this._parallel, testRuns.length);
+      const workerPromises = [];
+      for (let i = 0; i < parallel; ++i) {
+        const initialTestRunIndex = i * Math.floor(testRuns.length / parallel);
+        workerPromises.push(this._runWorker(initialTestRunIndex, testRuns, i));
+      }
+      await Promise.all(workerPromises);
+
+      for (const termination of terminations)
+        process.removeListener(termination.event, termination.handler);
+
+      if (testRuns.some(run => run.isFailure()))
+        this._result.setResult(TestResult.Failed, '');
+
+      clearTimeout(timeoutId);
+      await this._delegate.onFinished(this._result);
+
+      function createTermination(event, result, message) {
+        return {
+          event,
+          message,
+          handler: error => this._terminate(result, message, event === 'SIGTERM', event.startsWith('SIG') ? null : error),
+        };
       }
     }
-    this.emit(TestRunner.Events.Finished, result);
+
+    const result = this._result;
+    this._result = null;
+    this._workers = [];
+    this._terminating = false;
     return result;
   }
 
+  async _runWorker(testRunIndex, testRuns, parallelIndex) {
+    let worker = new TestWorker(this, this._nextWorkerId++, parallelIndex);
+    this._workers[parallelIndex] = worker;
+    while (!this._terminating) {
+      let skipped = 0;
+      while (skipped < testRuns.length && testRuns[testRunIndex]._result !== null) {
+        testRunIndex = (testRunIndex + 1) % testRuns.length;
+        skipped++;
+      }
+      const testRun = testRuns[testRunIndex];
+      if (testRun._result !== null) {
+        // All tests have been run.
+        break;
+      }
+
+      // Mark as running so that other workers do not run it again.
+      testRun._result = 'running';
+      await worker.run(testRun);
+      if (testRun.isFailure()) {
+        // Something went wrong during test run, let's use a fresh worker.
+        await worker.shutdown();
+        if (this._breakOnFailure) {
+          const message = `Terminating because a test has failed and |testRunner.breakOnFailure| is enabled`;
+          await this._terminate(TestResult.Terminated, message, false /* force */, null /* error */);
+          return;
+        }
+        worker = new TestWorker(this, this._nextWorkerId++, parallelIndex);
+        this._workers[parallelIndex] = worker;
+      }
+    }
+    await worker.shutdown();
+  }
+
+  async _terminate(result, message, force, error) {
+    debug('testrunner')(`TERMINATED result = ${result}, message = ${message}`);
+    this._terminating = true;
+    for (const worker of this._workers)
+      worker.terminate(force /* terminateHooks */);
+    this._result.setResult(result, message);
+    if (this._result.message === 'SIGINT received' && message === 'SIGTERM received')
+      this._result.message = message;
+    if (error) {
+      if (error.stack)
+        await this._sourceMapSupport.rewriteStackTraceWithSourceMaps(error);
+      this._result.addError(message, error, this._workers.length === 1 ? this._workers[0] : null);
+    }
+  }
+
   _testsToRun() {
     if (!this.hasFocusedTestsOrSuites())
       return this._tests;
@@ -844,9 +842,9 @@ class TestRunner extends EventEmitter {
   }
 
   async terminate() {
-    if (!this._runningPass)
+    if (!this._result)
       return;
-    await this._runningPass._terminate(TestResult.Terminated, 'Terminated with |TestRunner.terminate()| call', true /* force */, null /* error */);
+    await this._terminate(TestResult.Terminated, 'Terminated with |TestRunner.terminate()| call', true /* force */, null /* error */);
   }
 
   timeout() {
@@ -877,11 +875,4 @@ class TestRunner extends EventEmitter {
   }
 }
 
-TestRunner.Events = {
-  Started: 'started',
-  Finished: 'finished',
-  TestStarted: 'teststarted',
-  TestFinished: 'testfinished',
-};
-
 module.exports = TestRunner;

utils/testrunner/test/testrunner.spec.js

@@ -795,8 +795,8 @@ module.exports.addTests = function({testRunner, expect}) {
     });
   });
 
-  describe('TestRunner Events', () => {
+  describe('TestRunner delegate', () => {
-    it('should emit events in proper order', async() => {
+    it('should call delegate methpds in proper order', async() => {
       const log = [];
       const t = newTestRunner();
       t.beforeAll(() => log.push('beforeAll'));
@@ -804,10 +804,12 @@ module.exports.addTests = function({testRunner, expect}) {
       t.it('test#1', () => log.push('test#1'));
       t.afterEach(() => log.push('afterEach'));
       t.afterAll(() => log.push('afterAll'));
-      t.on('started', () => log.push('E:started'));
+      t.setDelegate({
-      t.on('teststarted', () => log.push('E:teststarted'));
+        onStarted: () => log.push('E:started'),
-      t.on('testfinished', () => log.push('E:testfinished'));
+        onTestRunStarted: () => log.push('E:teststarted'),
-      t.on('finished', () => log.push('E:finished'));
+        onTestRunFinished: () => log.push('E:testfinished'),
+        onFinished: () => log.push('E:finished'),
+      });
       await t.run();
       expect(log).toEqual([
         'E:started',
@@ -821,10 +823,15 @@ module.exports.addTests = function({testRunner, expect}) {
         'E:finished',
       ]);
     });
-    it('should emit finish event with result', async() => {
+    it('should call onFinished with result', async() => {
       const t = newTestRunner();
       const [result] = await Promise.all([
-        new Promise(x => t.once('finished', x)),
+        new Promise(x => t.setDelegate({
+          onStarted() {},
+          onFinished(result) { x(result); },
+          onTestRunStarted() {},
+          onTestRunFinished() {},
+        })),
         t.run(),
       ]);
       expect(result.result).toBe('ok');