Fix reporters' config #6542
Fix reporters' config #6542
Conversation
|
Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please sign up at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need the corporate CLA signed. If you have received this in error or have any questions, please contact us at cla@fb.com. Thanks! |
Codecov Report
@@ Coverage Diff @@
## master #6542 +/- ##
==========================================
+ Coverage 63.43% 63.56% +0.12%
==========================================
Files 235 235
Lines 9115 9114 -1
Branches 3 3
==========================================
+ Hits 5782 5793 +11
+ Misses 3332 3320 -12
Partials 1 1
Continue to review full report at Codecov.
|
|
Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks! |
|
@artola sorry about the lack of feedback here. Could you rebase it to fix the conflicts? The code itself LGTM, but I haven't really been in this part of the code base before |
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'`.
- config for reporters supports `default`
|
i'm not sure why flow didn't catch that, but the code makes total sense! |
| @@ -123,6 +123,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)) | |||
There was a problem hiding this comment.
this should be higher up (under master)
|
This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
Summary
Fix
reportersto handle string paramsDocumentation shows reporters as
reporters [array<moduleName | [moduleName, options]>], the code was incorrectly handling the following cases:This fix, re-use the already existing
_getReporterPropsto compare{path} === 'default'.Test plan
Regression test. See: https://github.com/artola/jest/blob/8f446d46f527e39cc3ac9adbd41a7403d3bc6a23/packages/jest-cli/src/__tests__/test_scheduler.test.js#L30