custom reporter error handling (jestjs#4051)

tushardhole · Jul 18, 2017 · 983246d · 983246d
1 parent 0180b81
commit 983246d
Show file tree

Hide file tree

Showing 5 changed files with 69 additions and 36 deletions.
diff --git a/integration_tests/__tests__/custom_reporters.test.js b/integration_tests/__tests__/custom_reporters.test.js
@@ -8,8 +8,15 @@
 'use strict';
 
 const skipOnWindows = require('skipOnWindows');
-const {extractSummary} = require('../utils');
+const {cleanup, extractSummary, writeFiles} = require('../utils');
 const runJest = require('../runJest');
+const os = require('os');
+const path = require('path');
+
+const DIR = path.resolve(os.tmpdir(), 'custom_reporters_test_dir');
+
+beforeEach(() => cleanup(DIR));
+afterEach(() => cleanup(DIR));
 
 describe('Custom Reporters Integration', () => {
   skipOnWindows.suite();
@@ -120,4 +127,27 @@ describe('Custom Reporters Integration', () => {
 
     expect(stdout).toMatchSnapshot();
   });
+
+  test('prints reporter errors', () => {
+    writeFiles(DIR, {
+      '__tests__/test.test.js': `test('test', () => {});`,
+      'package.json': JSON.stringify({
+        jest: {
+          testEnvironment: 'node',
+          reporters: ['default', '<rootDir>/reporter.js'],
+        },
+      }),
+      'reporter.js': `
+        module.exports = class Reporter {
+          onRunStart() {
+            throw new Error('ON_RUN_START_ERROR');
+          }
+        };
+      `,
+    });
+
+    const {stderr, status} = runJest(DIR);
+    expect(stderr).toMatch(/ON_RUN_START_ERROR/);
+    expect(status).toBe(1);
+  });
 });
diff --git a/packages/jest-cli/src/cli/index.js b/packages/jest-cli/src/cli/index.js
@@ -258,7 +258,7 @@ const _run = async (
   );
 
   globalConfig.watch || globalConfig.watchAll
-    ? _runWatch(
+    ? await _runWatch(
         contexts,
         configs,
         hasDeprecationWarnings,
@@ -267,7 +267,7 @@ const _run = async (
         hasteMapInstances,
         changedFilesPromise,
       )
-    : _runWithoutWatch(
+    : await _runWithoutWatch(
         globalConfig,
         contexts,
         outputStream,
@@ -304,11 +304,11 @@ const _runWithoutWatch = async (
   onComplete,
   changedFilesPromise,
 ) => {
-  const startRun = () => {
+  const startRun = async () => {
     if (!globalConfig.listTests) {
       preRunMessage.print(outputStream);
     }
-    runJest({
+    return await runJest({
       changedFilesPromise,
       contexts,
       globalConfig,
@@ -318,7 +318,7 @@ const _runWithoutWatch = async (
       testWatcher: new TestWatcher({isWatchMode: false}),
     });
   };
-  return startRun();
+  return await startRun();
 };
 
 module.exports = {

diff --git a/packages/jest-cli/src/reporter_dispatcher.js b/packages/jest-cli/src/reporter_dispatcher.js
@@ -36,24 +36,27 @@ class ReporterDispatcher {
     );
   }
 
-  onTestResult(test: Test, testResult: TestResult, results: AggregatedResult) {
-    this._reporters.forEach(
-      reporter =>
-        reporter.onTestResult &&
-        reporter.onTestResult(test, testResult, results),
-    );
+  async onTestResult(
+    test: Test,
+    testResult: TestResult,
+    results: AggregatedResult,
+  ) {
+    for (const reporter of this._reporters) {
+      reporter.onTestResult &&
+        (await reporter.onTestResult(test, testResult, results));
+    }
   }
 
-  onTestStart(test: Test) {
-    this._reporters.forEach(
-      reporter => reporter.onTestStart && reporter.onTestStart(test),
-    );
+  async onTestStart(test: Test) {
+    for (const reporter of this._reporters) {
+      reporter.onTestStart && (await reporter.onTestStart(test));
+    }
   }
 
-  onRunStart(results: AggregatedResult, options: ReporterOnStartOptions) {
-    this._reporters.forEach(
-      reporter => reporter.onRunStart && reporter.onRunStart(results, options),
-    );
+  async onRunStart(results: AggregatedResult, options: ReporterOnStartOptions) {
+    for (const reporter of this._reporters) {
+      reporter.onRunStart && (await reporter.onRunStart(results, options));
+    }
   }
 
   async onRunComplete(contexts: Set<Context>, results: AggregatedResult) {

diff --git a/packages/jest-cli/src/test_runner.js b/packages/jest-cli/src/test_runner.js
@@ -49,7 +49,7 @@ export type TestRunnerOptions = {|
   startRun: (globalConfig: GlobalConfig) => *,
 |};
 
-type OnTestFailure = (test: Test, err: TestError) => void;
+type OnTestFailure = (test: Test, err: TestError) => Promise<*>;
 type OnTestSuccess = (test: Test, result: TestResult) => Promise<*>;
 
 const TEST_WORKER_PATH = require.resolve('./test_worker');
@@ -99,24 +99,24 @@ class TestRunner {
         timings.length > 0 &&
         timings.every(timing => timing < SLOW_TEST_TIME));
 
-    const onResult = (test: Test, testResult: TestResult) => {
+    const onResult = async (test: Test, testResult: TestResult) => {
       if (watcher.isInterrupted()) {
         return Promise.resolve();
       }
       if (testResult.testResults.length === 0) {
         const message = 'Your test suite must contain at least one test.';
-        onFailure(test, {
+        await onFailure(test, {
           message,
           stack: new Error(message).stack,
         });
         return Promise.resolve();
       }
       addResult(aggregatedResults, testResult);
-      this._dispatcher.onTestResult(test, testResult, aggregatedResults);
+      await this._dispatcher.onTestResult(test, testResult, aggregatedResults);
       return this._bailIfNeeded(contexts, aggregatedResults, watcher);
     };
 
-    const onFailure = (test: Test, error: TestError) => {
+    const onFailure = async (test: Test, error: TestError) => {
       if (watcher.isInterrupted()) {
         return;
       }
@@ -128,7 +128,7 @@ class TestRunner {
         test.path,
       );
       addResult(aggregatedResults, testResult);
-      this._dispatcher.onTestResult(test, testResult, aggregatedResults);
+      await this._dispatcher.onTestResult(test, testResult, aggregatedResults);
     };
 
     const updateSnapshotState = () => {
@@ -149,7 +149,7 @@ class TestRunner {
       );
     };
 
-    this._dispatcher.onRunStart(aggregatedResults, {
+    await this._dispatcher.onRunStart(aggregatedResults, {
       estimatedTime,
       showStatus: !runInBand,
     });
@@ -194,12 +194,12 @@ class TestRunner {
       (promise, test) =>
         mutex(() =>
           promise
-            .then(() => {
+            .then(async () => {
               if (watcher.isInterrupted()) {
                 throw new CancelRun();
               }
 
-              this._dispatcher.onTestStart(test);
+              await this._dispatcher.onTestStart(test);
               return runTest(
                 test.path,
                 this._globalConfig,
@@ -235,11 +235,11 @@ class TestRunner {
     // Send test suites to workers continuously instead of all at once to track
     // the start time of individual tests.
     const runTestInWorker = test =>
-      mutex(() => {
+      mutex(async () => {
         if (watcher.isInterrupted()) {
           return Promise.reject();
         }
-        this._dispatcher.onTestStart(test);
+        await this._dispatcher.onTestStart(test);
         return worker({
           config: test.context.config,
           globalConfig: this._globalConfig,
@@ -250,8 +250,8 @@ class TestRunner {
         });
       });
 
-    const onError = (err, test) => {
-      onFailure(test, err);
+    const onError = async (err, test) => {
+      await onFailure(test, err);
       if (err.type === 'ProcessTerminatedError') {
         console.error(
           'A worker process has quit unexpectedly! ' +

diff --git a/types/TestRunner.js b/types/TestRunner.js
@@ -26,12 +26,12 @@ export type Reporter = {
     test: Test,
     testResult: TestResult,
     aggregatedResult: AggregatedResult,
-  ) => void,
+  ) => ?Promise<void>,
   +onRunStart: (
     results: AggregatedResult,
     options: ReporterOnStartOptions,
-  ) => void,
-  +onTestStart: (test: Test) => void,
+  ) => ?Promise<void>,
+  +onTestStart: (test: Test) => ?Promise<void>,
   +onRunComplete: (
     contexts: Set<Context>,
     results: AggregatedResult,