-
-
Notifications
You must be signed in to change notification settings - Fork 6.5k
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
[jest-each] Feature: handle 1d array #6351
Conversation
/cc @captbaritone |
LGTM! Thanks for the quick turnaround. |
Failing on circus. You probably have to keep the |
packages/jest-each/src/bind.js
Outdated
const table: Table = args[0]; | ||
const table: Table = args[0].every(Array.isArray) | ||
? args[0] | ||
: args[0].map(entry => [entry]); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should we be clever and only wrap those not already arrays? or would that never happen in practice?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it would look strange:
test.each([
['foo'],
'bar',
['baz'],
'qux'
])('do something %s', word => {});
But in saying that it would be pretty trivial to just wrap everything that isn't an array in an array. I'm happy to update if you think it makes more sense 😄
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Imagine something like this:
test.each([
['foo', 'bar'],
'foobar',
['baz', 'zzz', 'woke', 'blazingmeansgood'],
'qux'
])('do something %s', words => {});
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Again, not sure if it's useful at all or if we should just throw
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that all rows should be the same length, i.e. the test callback should have the same number of args. So maybe we should throw an error like the tagged template string? Inconsistent number of arguments in rows, 0, 1, 3. Test function expected X
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, throwing on mismatching length
s sounds good
This comment has been minimized.
This comment has been minimized.
Codecov Report
@@ Coverage Diff @@
## master #6351 +/- ##
==========================================
+ Coverage 63.63% 63.63% +<.01%
==========================================
Files 226 226
Lines 8648 8649 +1
Branches 4 3 -1
==========================================
+ Hits 5503 5504 +1
Misses 3144 3144
Partials 1 1
Continue to review full report at Codecov.
|
@mattphillips mind adding some docs on this? |
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 #6348
Test plan