From 8b2a60733c4c0c1d0356a4cfa27b0f784555e959 Mon Sep 17 00:00:00 2001 From: Caleb Everett Date: Wed, 11 Sep 2019 22:09:14 -0700 Subject: [PATCH] feat: make node-notifier an optional dependency (#8918) * feat: make node-notifier an optional dependency * Update CHANGELOG.md --- CHANGELOG.md | 1 + packages/jest-reporters/package.json | 4 +- .../src/__tests__/notify_reporter.test.ts | 42 +++++++++++++++++++ .../jest-reporters/src/notify_reporter.ts | 22 ++++++++-- 4 files changed, 64 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 707d04f364d6..46792cc676dc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -29,6 +29,7 @@ - `[jest-fake-timers]` `getTimerCount` will not include cancelled immediates ([#8764](https://github.com/facebook/jest/pull/8764)) - `[jest-leak-detector]` [**BREAKING**] Use `weak-napi` instead of `weak` package ([#8686](https://github.com/facebook/jest/pull/8686)) - `[jest-mock]` Fix for mockReturnValue overriding mockImplementationOnce ([#8398](https://github.com/facebook/jest/pull/8398)) +- `[jest-reporters]` Make node-notifer an optional dependency ([#8918](https://github.com/facebook/jest/pull/8918)) - `[jest-snapshot]` Remove only the added newlines in multiline snapshots ([#8859](https://github.com/facebook/jest/pull/8859)) - `[jest-snapshot]` Distinguish empty string from external snapshot not written ([#8880](https://github.com/facebook/jest/pull/8880)) - `[jest-snapshot]` [**BREAKING**] Distinguish empty string from internal snapshot not written ([#8898](https://github.com/facebook/jest/pull/8898)) diff --git a/packages/jest-reporters/package.json b/packages/jest-reporters/package.json index d6f3cb2f3eb8..26e2478139ab 100644 --- a/packages/jest-reporters/package.json +++ b/packages/jest-reporters/package.json @@ -23,7 +23,6 @@ "jest-runtime": "^24.9.0", "jest-util": "^24.9.0", "jest-worker": "^24.6.0", - "node-notifier": "^5.4.3", "slash": "^3.0.0", "source-map": "^0.6.0", "string-length": "^3.1.0" @@ -39,6 +38,9 @@ "@types/node-notifier": "^5.4.0", "strip-ansi": "^5.0.0" }, + "optionalDependencies": { + "node-notifier": "^5.4.3" + }, "engines": { "node": ">= 8" }, diff --git a/packages/jest-reporters/src/__tests__/notify_reporter.test.ts b/packages/jest-reporters/src/__tests__/notify_reporter.test.ts index 3c04db68af5c..e1a86899adc9 100644 --- a/packages/jest-reporters/src/__tests__/notify_reporter.test.ts +++ b/packages/jest-reporters/src/__tests__/notify_reporter.test.ts @@ -207,6 +207,48 @@ test('test failure-change with moduleName', () => { }); }); +describe('node-notifier is an optional dependency', () => { + beforeEach(() => { + jest.resetModules(); + }); + + const ctor = () => { + const config = makeGlobalConfig({ + notify: true, + notifyMode: 'success', + rootDir: 'some-test', + }); + return new NotifyReporter(config, () => {}, initialContext); + }; + + test('without node-notifier uses mock function that throws an error', () => { + jest.doMock('node-notifier', () => { + const error: any = new Error("Cannot find module 'node-notifier'"); + error.code = 'MODULE_NOT_FOUND'; + throw error; + }); + + expect(ctor).toThrow( + 'notify reporter requires optional dependeny node-notifier but it was not found', + ); + }); + + test('throws the error when require throws an unexpected error', () => { + const error = new Error('unexpected require error'); + jest.doMock('node-notifier', () => { + throw error; + }); + expect(ctor).toThrow(error); + }); + + test('uses node-notifier when it is available', () => { + const mockNodeNotifier = {notify: jest.fn()}; + jest.doMock('node-notifier', () => mockNodeNotifier); + const result = ctor(); + expect(result['_notifier']).toBe(mockNodeNotifier); + }); +}); + afterEach(() => { jest.clearAllMocks(); }); diff --git a/packages/jest-reporters/src/notify_reporter.ts b/packages/jest-reporters/src/notify_reporter.ts index 7735baaf2d81..70d22e1c1d8f 100644 --- a/packages/jest-reporters/src/notify_reporter.ts +++ b/packages/jest-reporters/src/notify_reporter.ts @@ -10,7 +10,6 @@ import * as util from 'util'; import exit = require('exit'); import {Config} from '@jest/types'; import {AggregatedResult} from '@jest/test-result'; -import {notify} from 'node-notifier'; import {Context, TestSchedulerContext} from './types'; import BaseReporter from './base_reporter'; @@ -19,6 +18,7 @@ const isDarwin = process.platform === 'darwin'; const icon = path.resolve(__dirname, '../assets/jest_logo.png'); export default class NotifyReporter extends BaseReporter { + private _notifier = loadNotifier(); private _startRun: (globalConfig: Config.GlobalConfig) => any; private _globalConfig: Config.GlobalConfig; private _context: TestSchedulerContext; @@ -78,7 +78,7 @@ export default class NotifyReporter extends BaseReporter { result.numPassedTests, ); - notify({icon, message, title}); + this._notifier.notify({icon, message, title}); } else if ( testsHaveRun && !success && @@ -106,9 +106,9 @@ export default class NotifyReporter extends BaseReporter { const quitAnswer = 'Exit tests'; if (!watchMode) { - notify({icon, message, title}); + this._notifier.notify({icon, message, title}); } else { - notify( + this._notifier.notify( { actions: [restartAnswer, quitAnswer], closeLabel: 'Close', @@ -137,3 +137,17 @@ export default class NotifyReporter extends BaseReporter { this._context.firstRun = false; } } + +function loadNotifier(): typeof import('node-notifier') { + try { + return require('node-notifier'); + } catch (err) { + if (err.code !== 'MODULE_NOT_FOUND') { + throw err; + } else { + throw Error( + 'notify reporter requires optional dependeny node-notifier but it was not found', + ); + } + } +}