Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add summaryThreshold option to summary reporter #13895

Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
### Features

- `[jest-message-util]` Add support for [error causes](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Error/cause)
- `[jest-reporters]` Add `summaryThreshold` option to summary reporter to allow overriding the internal threshold that is used to print the summary of all failed tests when the number of test suites surpasses it ([#13895](https://github.com/facebook/jest/pull/13895))

### Fixes

Expand Down
25 changes: 25 additions & 0 deletions docs/Configuration.md
Original file line number Diff line number Diff line change
Expand Up @@ -1322,6 +1322,31 @@ const config: Config = {
export default config;
```

The `summary` reporter accepts options. Since it is included in the `default` reporter you may also pass the options there.

```js tab
/** @type {import('jest').Config} */
const config = {
reporters: [['default', {summaryThreshold: 10}]],
};

module.exports = config;
```

```ts tab
import type {Config} from 'jest';

const config: Config = {
reporters: [['default', {summaryThreshold: 10}]],
};

export default config;
```

The `summaryThreshold` option behaves in the following way, if the total number of test suites surpasses this threshold, a detailed summary of all failed tests will be printed after executing all the tests. If not set it defaults to an internal value.
stefanosgk marked this conversation as resolved.
Show resolved Hide resolved

For more information about the available options refer to `SummaryReporterOptions` type in the [reporter type definitions](https://github.com/facebook/jest/blob/main/packages/jest-reporters/src/types.ts).

stefanosgk marked this conversation as resolved.
Show resolved Hide resolved
#### Custom Reporters

:::tip
Expand Down
29 changes: 29 additions & 0 deletions e2e/__tests__/summaryThreshold.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
/**
* Copyright (c) Meta Platforms, Inc. and affiliates.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*/

import runJest from '../runJest';

['default', 'summary'].forEach(reporter => {
describe(`${reporter} reporter`, () => {
test('prints failure messages when total number of test suites is over summaryThreshold', () => {
const {exitCode, stderr} = runJest('summary-threshold', [
'--config',
JSON.stringify({
reporters: [[reporter, {summaryThreshold: 2}]],
}),
]);

expect(exitCode).toBe(1);
expect(stderr).toMatch(
/Summary of all failing tests(\n|.)*expect\(1\)\.toBe\(0\)/,
);
expect(stderr).toMatch(
/Summary of all failing tests(\n|.)*expect\(2\)\.toBe\(0\)/,
);
});
});
});
11 changes: 11 additions & 0 deletions e2e/summary-threshold/__tests__/summarySuite1.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
/**
* Copyright (c) Meta Platforms, Inc. and affiliates.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*/
'use strict';

test('fails', () => {
expect(1).toBe(0);
});
11 changes: 11 additions & 0 deletions e2e/summary-threshold/__tests__/summarySuite2.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
/**
* Copyright (c) Meta Platforms, Inc. and affiliates.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*/
'use strict';

test('fails', () => {
expect(2).toBe(0);
});
11 changes: 11 additions & 0 deletions e2e/summary-threshold/__tests__/summarySuite3.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
/**
* Copyright (c) Meta Platforms, Inc. and affiliates.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*/
'use strict';

test('passes', () => {
expect(1).toBe(1);
});
5 changes: 5 additions & 0 deletions e2e/summary-threshold/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
{
"jest": {
"testEnvironment": "node"
}
}
10 changes: 5 additions & 5 deletions packages/jest-core/src/TestScheduler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -336,12 +336,12 @@ class TestScheduler {
async _setupReporters() {
const {collectCoverage: coverage, notify, verbose} = this._globalConfig;
const reporters = this._globalConfig.reporters || [['default', {}]];
let summary = false;
let summaryOptions;
SimenB marked this conversation as resolved.
Show resolved Hide resolved

for (const [reporter, options] of reporters) {
switch (reporter) {
case 'default':
summary = true;
summaryOptions = options;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I went checking if options is always defined. Apparently normalization always add an object. That’s smart idea!

verbose
? this.addReporter(new VerboseReporter(this._globalConfig))
: this.addReporter(new DefaultReporter(this._globalConfig));
Expand All @@ -353,7 +353,7 @@ class TestScheduler {
);
break;
case 'summary':
summary = true;
summaryOptions = options;
break;
default:
await this._addCustomReporter(reporter, options);
Expand All @@ -368,8 +368,8 @@ class TestScheduler {
this.addReporter(new CoverageReporter(this._globalConfig, this._context));
}

if (summary) {
this.addReporter(new SummaryReporter(this._globalConfig));
if (summaryOptions) {
stefanosgk marked this conversation as resolved.
Show resolved Hide resolved
this.addReporter(new SummaryReporter(this._globalConfig, summaryOptions));
}
}

Expand Down
23 changes: 18 additions & 5 deletions packages/jest-reporters/src/SummaryReporter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,7 @@ import BaseReporter from './BaseReporter';
import getResultHeader from './getResultHeader';
import getSnapshotSummary from './getSnapshotSummary';
import getSummary from './getSummary';
import type {ReporterOnStartOptions} from './types';

const TEST_SUMMARY_THRESHOLD = 20;
import type {ReporterOnStartOptions, SummaryReporterOptions} from './types';

const NPM_EVENTS = new Set([
'prepublish',
Expand Down Expand Up @@ -54,13 +52,28 @@ const {npm_config_user_agent, npm_lifecycle_event, npm_lifecycle_script} =
export default class SummaryReporter extends BaseReporter {
private _estimatedTime: number;
private readonly _globalConfig: Config.GlobalConfig;
private readonly _summaryThreshold: number;

static readonly filename = __filename;

constructor(globalConfig: Config.GlobalConfig) {
constructor(
globalConfig: Config.GlobalConfig,
options: SummaryReporterOptions = {},
) {
super();
this._globalConfig = globalConfig;
this._estimatedTime = 0;
this.validateOptions(options);
this._summaryThreshold = options.summaryThreshold || 20;
}

private validateOptions(options: SummaryReporterOptions) {
if (
options.summaryThreshold &&
stefanosgk marked this conversation as resolved.
Show resolved Hide resolved
typeof options.summaryThreshold !== 'number'
) {
throw new TypeError('The option summaryThreshold should be a number');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it make sense to check if summaryThreshold is positive integer? Or am I overthinking?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since it wouldn't do any harm to pass a negative value, I think it's fine not to validate it.

}
}

// If we write more than one character at a time it is possible that
Expand Down Expand Up @@ -176,7 +189,7 @@ export default class SummaryReporter extends BaseReporter {
const runtimeErrors = aggregatedResults.numRuntimeErrorTestSuites;
if (
failedTests + runtimeErrors > 0 &&
aggregatedResults.numTotalTestSuites > TEST_SUMMARY_THRESHOLD
aggregatedResults.numTotalTestSuites > this._summaryThreshold
) {
this.log(chalk.bold('Summary of all failing tests'));
aggregatedResults.testResults.forEach(testResult => {
Expand Down
75 changes: 75 additions & 0 deletions packages/jest-reporters/src/__tests__/SummaryReporter.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -160,3 +160,78 @@ test('snapshots all have results (after update)', () => {
testReporter.onRunComplete(new Set(), aggregatedResults);
expect(results.join('').replace(/\\/g, '/')).toMatchSnapshot();
});

describe('summaryThreshold option', () => {
const aggregatedResults = {
numFailedTestSuites: 1,
numFailedTests: 1,
numPassedTestSuites: 2,
numRuntimeErrorTestSuites: 0,
numTotalTestSuites: 3,
numTotalTests: 3,
snapshot: {
filesRemovedList: [],
filesUnmatched: 0,
total: 0,
uncheckedKeysByFile: [],
unmatched: 0,
},
startTime: 0,
testResults: [
{
failureMessage: 'FailureMessage1',
numFailingTests: 1,
testFilePath: 'path1',
},
{
failureMessage: 'FailureMessage2',
numFailingTests: 1,
testFilePath: 'path2',
},
],
};

it('Should print failure messages when number of test suites is over the threshold', () => {
const options = {
summaryThreshold: aggregatedResults.numTotalTestSuites - 1,
};

requireReporter();
const testReporter = new SummaryReporter(globalConfig, options);
testReporter.onRunComplete(new Set(), aggregatedResults);
expect(results.join('').replace(/\\/g, '/')).toMatchSnapshot();
});

it('Should not print failure messages when number of test suites is under the threshold', () => {
const options = {
summaryThreshold: aggregatedResults.numTotalTestSuites + 1,
};

requireReporter();
const testReporter = new SummaryReporter(globalConfig, options);
testReporter.onRunComplete(new Set(), aggregatedResults);
expect(results.join('').replace(/\\/g, '/')).toMatchSnapshot();
});

it('Should not print failure messages when number of test suites is equal to the threshold', () => {
const options = {
summaryThreshold: aggregatedResults.numTotalTestSuites,
};

requireReporter();
const testReporter = new SummaryReporter(globalConfig, options);
testReporter.onRunComplete(new Set(), aggregatedResults);
expect(results.join('').replace(/\\/g, '/')).toMatchSnapshot();
});

it('Should throw error if threshold is not a number', () => {
const options = {
summaryThreshold: 'not a number',
};

requireReporter();
expect(() => new SummaryReporter(globalConfig, options)).toThrow(
'The option summaryThreshold should be a number',
);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -59,3 +59,36 @@ exports[`snapshots needs update with yarn test 1`] = `
<dim>Ran all test suites</intensity><dim>.</intensity>
"
`;

exports[`summaryThreshold option Should not print failure messages when number of test suites is equal to the threshold 1`] = `
"<bold>Test Suites: </intensity><bold><red>1 failed</color></intensity>, <bold><green>2 passed</color></intensity>, 3 total
<bold>Tests: </intensity><bold><red>1 failed</color></intensity>, 3 total
<bold>Snapshots: </intensity>0 total
<bold>Time:</intensity> 0.01 s
<dim>Ran all test suites</intensity><dim>.</intensity>
"
`;

exports[`summaryThreshold option Should not print failure messages when number of test suites is under the threshold 1`] = `
"<bold>Test Suites: </intensity><bold><red>1 failed</color></intensity>, <bold><green>2 passed</color></intensity>, 3 total
<bold>Tests: </intensity><bold><red>1 failed</color></intensity>, 3 total
<bold>Snapshots: </intensity>0 total
<bold>Time:</intensity> 0.01 s
<dim>Ran all test suites</intensity><dim>.</intensity>
"
`;

exports[`summaryThreshold option Should print failure messages when number of test suites is over the threshold 1`] = `
"<bold>Summary of all failing tests</intensity>
</><inverse><bold><red> FAIL </color></intensity></inverse></> <dim>../</intensity><bold>path1</intensity>
FailureMessage1
</><inverse><bold><red> FAIL </color></intensity></inverse></> <dim>../</intensity><bold>path2</intensity>
FailureMessage2

<bold>Test Suites: </intensity><bold><red>1 failed</color></intensity>, <bold><green>2 passed</color></intensity>, 3 total
<bold>Tests: </intensity><bold><red>1 failed</color></intensity>, 3 total
<bold>Snapshots: </intensity>0 total
<bold>Time:</intensity> 0.01 s
<dim>Ran all test suites</intensity><dim>.</intensity>
"
`;
4 changes: 4 additions & 0 deletions packages/jest-reporters/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -63,3 +63,7 @@ export type SummaryOptions = {
showSeed?: boolean;
seed?: number;
};

export type SummaryReporterOptions = {
stefanosgk marked this conversation as resolved.
Show resolved Hide resolved
summaryThreshold?: number;
};