From 0dd0c855df787065cb4adcafbc3d4ed227482e60 Mon Sep 17 00:00:00 2001 From: Dmitrii Abramov Date: Mon, 17 Jul 2017 14:38:31 -0700 Subject: [PATCH] custom reporter error handling --- .../__tests__/custom_reporters.test.js | 32 ++++++++++++++++++- packages/jest-cli/src/cli/index.js | 10 +++--- packages/jest-cli/src/reporter_dispatcher.js | 31 ++++++++++-------- packages/jest-cli/src/test_runner.js | 26 +++++++-------- types/TestRunner.js | 6 ++-- 5 files changed, 69 insertions(+), 36 deletions(-) diff --git a/integration_tests/__tests__/custom_reporters.test.js b/integration_tests/__tests__/custom_reporters.test.js index 7055767038ea..a3b08bee4a6b 100644 --- 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', '/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 index 61f3ef096446..12400fa058b6 100644 --- 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 index 262b2f605669..ec72faf40660 100644 --- 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, results: AggregatedResult) { diff --git a/packages/jest-cli/src/test_runner.js b/packages/jest-cli/src/test_runner.js index efaf6ad5c5be..1c1a117eabad 100644 --- 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 index f00f2d09cfe4..55615d79a661 100644 --- a/types/TestRunner.js +++ b/types/TestRunner.js @@ -26,12 +26,12 @@ export type Reporter = { test: Test, testResult: TestResult, aggregatedResult: AggregatedResult, - ) => void, + ) => ?Promise, +onRunStart: ( results: AggregatedResult, options: ReporterOnStartOptions, - ) => void, - +onTestStart: (test: Test) => void, + ) => ?Promise, + +onTestStart: (test: Test) => ?Promise, +onRunComplete: ( contexts: Set, results: AggregatedResult,