[jest-each] Fix each array title concatenation #6346
Conversation
mattphillips
force-pushed
the
jest-each-args
branch
from
f71d988
to
f610733
Compare
mattphillips
force-pushed
the
jest-each-args
branch
from
f610733
to
dd3f9fc
Compare
Codecov Report
@@ Coverage Diff @@
## master #6346 +/- ##
==========================================
+ Coverage 63.63% 63.65% +0.02%
==========================================
Files 226 226
Lines 8648 8654 +6
Branches 4 4
==========================================
+ Hits 5503 5509 +6
Misses 3144 3144
Partials 1 1
Continue to review full report at Codecov.
|
| const arrayFormat = (str, ...args) => { | ||
| const matches = (str.match(SUPPORTED_PLACEHOLDERS) || []).length; | ||
| return util.format(str, ...args.slice(0, matches)); | ||
| }; |
There was a problem hiding this comment.
what about matches.length > args.length?
There was a problem hiding this comment.
.slice is greedy and will take all of the args still, there will just be some placeholders that aren't replaced. Do we care about this case? I think it should be pretty obvious that the placeholders aren't replaced as there isn't a value for them 😆
There was a problem hiding this comment.
Not sure if it should fail the tests, I wouldn't say its an error case. Just a developer mistake, perhaps we could log a warning?
There was a problem hiding this comment.
Maybe we can add a lint rule for it?
|
Please rebase. |
|
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
Fixes #6342
Test plan
util.format