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 1 commit
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
2 changes: 1 addition & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +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 `forceFailSummary` option to summary reporter to always print failure messages
- `[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
10 changes: 3 additions & 7 deletions docs/Configuration.md
Original file line number Diff line number Diff line change
Expand Up @@ -1327,9 +1327,7 @@ The `summary` reporter accepts options. Since it is included in the `default` re
```js tab
/** @type {import('jest').Config} */
const config = {
reporters: [
['default', {forceFailSummary: true}],
],
reporters: [['default', {summaryThreshold: 10}]],
};

module.exports = config;
Expand All @@ -1339,15 +1337,13 @@ module.exports = config;
import type {Config} from 'jest';

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

export default config;
```

The `forceFailSummary` option will force to always print a summary of failed tests after executing all the tests. If not set the default behavior is to print this summary only if the number of test suites surpasses an internal threshold.
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).

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"
}
}
19 changes: 13 additions & 6 deletions packages/jest-reporters/src/SummaryReporter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,6 @@ import getSnapshotSummary from './getSnapshotSummary';
import getSummary from './getSummary';
import type {ReporterOnStartOptions, SummaryReporterOptions} from './types';

const TEST_SUMMARY_THRESHOLD = 20;

const NPM_EVENTS = new Set([
'prepublish',
'publish',
Expand Down Expand Up @@ -54,7 +52,7 @@ 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 _options: SummaryReporterOptions;
private readonly _summaryThreshold: number;

static readonly filename = __filename;

Expand All @@ -64,8 +62,18 @@ export default class SummaryReporter extends BaseReporter {
) {
super();
this._globalConfig = globalConfig;
this._options = options;
this._estimatedTime = 0;
this.validateOptions(options);
this._summaryThreshold = options.summaryThreshold || 20;
}

private validateOptions(options: SummaryReporterOptions) {
if (
options.summaryThreshold &&
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 @@ -181,8 +189,7 @@ export default class SummaryReporter extends BaseReporter {
const runtimeErrors = aggregatedResults.numRuntimeErrorTestSuites;
if (
failedTests + runtimeErrors > 0 &&
(this._options.forceFailSummary ||
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>
"
`;
2 changes: 1 addition & 1 deletion packages/jest-reporters/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -65,5 +65,5 @@ export type SummaryOptions = {
};

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