Skip to content

Commit

Permalink
Add check to make sure one or more tests have run before notifying (#…
Browse files Browse the repository at this point in the history
…6495)

* add check to make sure tests have run before notifying

* update changelog

* add condition for aggregatedResults with no tests to not call notifier
  • Loading branch information
taylorwinfield authored and cpojer committed Jun 20, 2018
1 parent 87bf0dd commit dd9ec17
Show file tree
Hide file tree
Showing 4 changed files with 22 additions and 8 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

### Fixes

- `[jest-cli]` Add check to make sure one or more tests have run before notifying when using `--notify` ([#6495](https://github.com/facebook/jest/pull/6495))
- `[jest-cli]` Pass `globalConfig` as a parameter to `globalSetup` and `globalTeardown` functions ([#6486](https://github.com/facebook/jest/pull/6486))
- `[jest-config]` Add missing options to the `defaults` object ([#6428](https://github.com/facebook/jest/pull/6428))
- `[expect]` Using symbolic property names in arrays no longer causes the `toEqual` matcher to fail ([#6391](https://github.com/facebook/jest/pull/6391))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,10 +31,6 @@ Array [

exports[`test change 1`] = `
Array [
Object {
"message": "3 tests passed",
"title": "100% Passed",
},
Object {
"message": "3 of 3 tests failed",
"title": "100% Failed",
Expand All @@ -52,10 +48,6 @@ Array [

exports[`test failure-change 1`] = `
Array [
Object {
"message": "3 tests passed",
"title": "100% Passed",
},
Object {
"message": "3 of 3 tests failed",
"title": "100% Failed",
Expand Down
17 changes: 17 additions & 0 deletions packages/jest-cli/src/__tests__/notify_reporter.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,21 @@ const aggregatedResultsFailure: AggregatedResult = {
success: false,
};

const aggregatedResultsNoTests: AggregatedResult = {
numFailedTestSuites: 0,
numFailedTests: 0,
numPassedTestSuites: 0,
numPassedTests: 0,
numPendingTestSuites: 0,
numPendingTests: 0,
numRuntimeErrorTestSuites: 0,
numTotalTestSuites: 0,
numTotalTests: 0,
};

// Simulated sequence of events for NotifyReporter
const notifyEvents = [
aggregatedResultsNoTests,
aggregatedResultsSuccess,
aggregatedResultsFailure,
aggregatedResultsSuccess,
Expand Down Expand Up @@ -84,6 +97,10 @@ const testModes = (notifyMode: string, arl: Array<AggregatedResult>) => {
);
previousContext = newContext;
reporter.onRunComplete(new Set(), ar);

if (ar.numTotalTests === 0) {
expect(notify.notify).not.toHaveBeenCalled();
}
});

expect(
Expand Down
4 changes: 4 additions & 0 deletions packages/jest-cli/src/reporters/notify_reporter.js
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,10 @@ export default class NotifyReporter extends BaseReporter {
const notifyMode = this._globalConfig.notifyMode;
const statusChanged =
this._context.previousSuccess !== success || this._context.firstRun;
const testsHaveRun = result.numTotalTests !== 0;

if (
testsHaveRun &&
success &&
(notifyMode === 'always' ||
notifyMode === 'success' ||
Expand All @@ -60,6 +63,7 @@ export default class NotifyReporter extends BaseReporter {

notifier.notify({icon, message, title});
} else if (
testsHaveRun &&
!success &&
(notifyMode === 'always' ||
notifyMode === 'failure' ||
Expand Down

0 comments on commit dd9ec17

Please sign in to comment.