Skip to content

Commit

Permalink
Fix reporters' config (#6542)
Browse files Browse the repository at this point in the history
* Fix `reporters` to handle string params

Documentation shows reporters as `reporters [array<moduleName | [moduleName, options]>]`, the code was incorrectly handling the following cases:

```
// no `string` moduleName
// _addCustomReporters(reporters: Array<ReporterConfig>)
reporters: ['default', 'someother']

// _shouldAddDefaultReporters(reporters?: Array<ReporterConfig>): boolean {
reporters: [['default', {}], 'someother']
```

This fix, use  the already existing `_getReporterProps` to compare `{path} === 'default'`.

* Update CHANGELOG.md

* Update Config.js

* prettier code

* prettier code

* regression test

- config for reporters supports `default`

* Update CHANGELOG.md
  • Loading branch information
artola authored and aaronabramov committed Aug 16, 2018
1 parent 2736b9b commit 9737621
Show file tree
Hide file tree
Showing 4 changed files with 53 additions and 11 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,7 @@
- `[expect]` `toEqual` no longer tries to compare non-enumerable symbolic properties, to be consistent with non-symbolic properties. ([#6398](https://github.com/facebook/jest/pull/6398))
- `[jest-util]` `console.timeEnd` now properly log elapsed time in milliseconds. ([#6456](https://github.com/facebook/jest/pull/6456))
- `[jest-mock]` Fix `MockNativeMethods` access in react-native `jest.mock()` ([#6505](https://github.com/facebook/jest/pull/6505))
- `[jest-cli]` Fix `reporters` for `moduleName` = `'default'` ([#6542](https://github.com/facebook/jest/pull/6542))

### Chore & Maintenance

Expand Down
20 changes: 11 additions & 9 deletions packages/jest-cli/src/TestScheduler.js
Original file line number Diff line number Diff line change
Expand Up @@ -255,10 +255,14 @@ export default class TestScheduler {
}
}

_shouldAddDefaultReporters(reporters?: Array<ReporterConfig>): boolean {
_shouldAddDefaultReporters(
reporters?: Array<string | ReporterConfig>,
): boolean {
return (
!reporters ||
!!reporters.find(reporterConfig => reporterConfig[0] === 'default')
!!reporters.find(
reporter => this._getReporterProps(reporter).path === 'default',
)
);
}

Expand Down Expand Up @@ -303,14 +307,12 @@ export default class TestScheduler {
this.addReporter(new SummaryReporter(this._globalConfig));
}

_addCustomReporters(reporters: Array<ReporterConfig>) {
const customReporters = reporters.filter(
reporterConfig => reporterConfig[0] !== 'default',
);

customReporters.forEach((reporter, index) => {
_addCustomReporters(reporters: Array<string | ReporterConfig>) {
reporters.forEach((reporter, index) => {
const {options, path} = this._getReporterProps(reporter);

if (path === 'default') return;

try {
// $FlowFixMe
const Reporter = require(path);
Expand All @@ -331,7 +333,7 @@ export default class TestScheduler {
* to make dealing with them less painful.
*/
_getReporterProps(
reporter: ReporterConfig,
reporter: string | ReporterConfig,
): {path: string, options?: Object} {
if (typeof reporter === 'string') {
return {options: this._options, path: reporter};
Expand Down
39 changes: 39 additions & 0 deletions packages/jest-cli/src/__tests__/TestScheduler.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,45 @@ jest.mock('jest-runner-parallel', () => jest.fn(() => mockParallelRunner), {
virtual: true,
});

test('config for reporters supports `default`', () => {
const undefinedReportersScheduler = new TestScheduler(
{
reporters: undefined,
},
{},
);
const numberOfReporters =
undefinedReportersScheduler._dispatcher._reporters.length;

const stringDefaultReportersScheduler = new TestScheduler(
{
reporters: ['default'],
},
{},
);
expect(stringDefaultReportersScheduler._dispatcher._reporters.length).toBe(
numberOfReporters,
);

const defaultReportersScheduler = new TestScheduler(
{
reporters: [['default', {}]],
},
{},
);
expect(defaultReportersScheduler._dispatcher._reporters.length).toBe(
numberOfReporters,
);

const emptyReportersScheduler = new TestScheduler(
{
reporters: [],
},
{},
);
expect(emptyReportersScheduler._dispatcher._reporters.length).toBe(0);
});

test('.addReporter() .removeReporter()', () => {
const scheduler = new TestScheduler({}, {});
const reporter = new SummaryReporter();
Expand Down
4 changes: 2 additions & 2 deletions types/Config.js
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ export type InitialOptions = {
globalSetup?: ?string,
globalTeardown?: ?string,
haste?: HasteConfig,
reporters?: Array<ReporterConfig | string>,
reporters?: Array<string | ReporterConfig>,
logHeapUsage?: boolean,
lastCommit?: boolean,
listTests?: boolean,
Expand Down Expand Up @@ -219,7 +219,7 @@ export type GlobalConfig = {|
passWithNoTests: boolean,
projects: Array<Glob>,
replname: ?string,
reporters: Array<ReporterConfig>,
reporters: Array<string | ReporterConfig>,
runTestsByPath: boolean,
rootDir: Path,
silent: boolean,
Expand Down

0 comments on commit 9737621

Please sign in to comment.