Skip to content
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

Merged
merged 5 commits into from
May 30, 2018

Conversation

mattphillips
Copy link
Contributor

Summary

Fixes #6348

Test plan

  • Add unit test
  • Add e2e tests

@mattphillips
Copy link
Contributor Author

/cc @captbaritone

@mattphillips mattphillips changed the title [jest-each] handle 1d array [jest-each] Feature: handle 1d array May 29, 2018
@captbaritone
Copy link
Contributor

LGTM! Thanks for the quick turnaround.

@SimenB
Copy link
Member

SimenB commented May 30, 2018

Failing on circus. You probably have to keep the describe at the bottom

const table: Table = args[0];
const table: Table = args[0].every(Array.isArray)
? args[0]
: args[0].map(entry => [entry]);
Copy link
Member

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?

Copy link
Contributor Author

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 😄

Copy link
Member

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 => {});

Copy link
Member

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

Copy link
Contributor Author

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

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, throwing on mismatching lengths sounds good

@georgejamesjr

This comment has been minimized.

@codecov-io
Copy link

Codecov Report

Merging #6351 into master will increase coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            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
Impacted Files Coverage Δ
packages/jest-each/src/bind.js 100% <100%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2307643...88f6a66. Read the comment docs.

@SimenB
Copy link
Member

SimenB commented Jun 12, 2018

@mattphillips mind adding some docs on this?

@mattphillips
Copy link
Contributor Author

@SimenB see #6444

@github-actions
Copy link

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.
Please note this issue tracker is not a help forum. We recommend using StackOverflow or our discord channel for questions.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 12, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support shallow arrays in jest-each
7 participants