Refactor addon-jest to use a parameter-based pattern #3678
Conversation
tmeasday
requested review from
alterx,
danielduan,
Hypnosphi,
igor-dv,
renaudtertrais and
usulpro
as code owners
Codecov Report
@@ Coverage Diff @@
## master #3678 +/- ##
=========================================
- Coverage 41.35% 41.3% -0.06%
=========================================
Files 455 455
Lines 5184 5191 +7
Branches 900 902 +2
=========================================
Hits 2144 2144
- Misses 2496 2501 +5
- Partials 544 546 +2
Continue to review full report at Codecov.
|
addons/jest/README.md
Outdated
| - **options.results**: OBJECT jest output results. *mandatory* | ||
| - **filesExt**: STRING test file extention. *optionnal*. This allow you to write "MyComponent" and not "MyComponent.test.js". It will be used as regex to find your file results. Default value is `((\\.specs?)|(\\.tests?))?(\\.js)?$`. That mean it will match: MyComponent.js, MyComponent.test.js, MyComponent.tests.js, MyComponent.spec.js, MyComponent.specs.js... | ||
| * **options.results**: OBJECT jest output results. _mandatory_ | ||
| * **filesExt**: STRING test file extention. _optionnal_. This allow you to write "MyComponent" and not "MyComponent.test.js". It will be used as regex to find your file results. Default value is `((\\.specs?)|(\\.tests?))?(\\.js)?$`. That mean it will match: MyComponent.js, MyComponent.test.js, MyComponent.tests.js, MyComponent.spec.js, MyComponent.specs.js... |
There was a problem hiding this comment.
That was an existing line ;)
addons/jest/src/index.js
Outdated
|
|
||
| const findTestResults = (testFiles, jestTestResults, jestTestFilesExt) => | ||
| testFiles.map(name => { | ||
| [].concat(testFiles).map(name => { |
There was a problem hiding this comment.
why ? map() is immutable no ?
There was a problem hiding this comment.
[].concat() is a way to "ensure array" -- [].concat('a') ~== [].concat(['a'])
There was a problem hiding this comment.
So it lets the user set { jest: 'filename' } or { jest: ['filename', 'other-filename'] } which is natural.
There was a problem hiding this comment.
Array.from(testFiles) is a more explicit way to do the same
There was a problem hiding this comment.
Ok, I didn't know. Good trick.
There was a problem hiding this comment.
@Hypnosphi -- good one, I'll make that change.
Arggh, looks like this bug has reared its head again. |
|
@renaudtertrais are you OK with the changed API/readme here? |
addons/jest/src/index.js
Outdated
|
|
||
| const findTestResults = (testFiles, jestTestResults, jestTestFilesExt) => | ||
| testFiles.map(name => { | ||
| [].concat(testFiles).map(name => { |
There was a problem hiding this comment.
Ok, I didn't know. Good trick.
|
@shilman I will look at the broken snapshot tomorrow but I don't think it is relevant to this branch, just flakiness on Chromatic's part wrt those iframe-based snapshots (as you saw on your own branch the other day). |
addons/jest/src/index.js
Outdated
| }, | ||
| ] = args; | ||
|
|
||
| if (testFiles && !testFiles.skip) { |
There was a problem hiding this comment.
Where does this testFiles.skip come from? Is this the same as the disable flag that we have in the other parameterized addons? It's not really documented anywhere.
There was a problem hiding this comment.
Poking @tmeasday just in case you missed this comment, since this is part of the 4.0 release. If you didn't, just ignore this message!
There was a problem hiding this comment.
Thanks @Keraito -- I have not forgotten, just getting around to this, sorry!
For #3625 -- this one was a little different as the current API doesn't quite fit the old pattern. But I think I've brought it as close as possible to fitting the parameters philosophy.