[jest-each] Template table heading validation and extraction

[mattphillips]

Summary

Currently any additional words in a template will break jest-each's expected behaviour of injecting the template headings as variables to the test function.

For example:

test.only.each`
  a    | b    | expected
  This test demonstrates jest-each
  ${1} | ${2} | ${3}
`('$a + $b == $expected', ({a, b, expected}) => {
  expect(a + b).toBe(expected);
});

This test will fail as expected equals undefined.

That is because currently jest-each splits on | and removes whitespace so the expected key is actually deserialised to expectedThistestdemonstratesjest-each which is not correct.

---

The proposed solution to this is to validate the headings to make sure there are no additional words in the first row (heading row). Any words appearing after the first row are simply ignored by jest-each.

By ignoring the additional rows user's are able to add whatever they like - currently though prettier does strip out all additional text in the template after the first interpolation (${variable}), perhaps it is worth investigating how to prevent that.

This PR relates to point 2 in this comment

Test plan

See tests

EDIT: the above example will also now pass

[jeysal]

Okay now I see what you mean. Just ignoring lines that do not match the expected format gives the possibility to put headlines in between, unrelated to any comment/skip API though.
It seems kinda magic and not like something I'd recommend people to do though. And doesn't work if there's just one parameter I guess.

[jeysal]

Possibly breaking BTW if someone relies on multiple words between pipes

[mattphillips]

@jeysal It should work fine even if there is only one heading/param (see b64b642) 😄

I'm not sure I agree that it's a breaking change, I doubt anyone does depend on this behaviour and it definitely is a bug (well is an unintended behaviour) and needs fixing.

I agree about not recommending people to add subheadings/additional text to the template - at least this way we do not put semantics (and code) around // syntax.

I suspect if we want to support a convention around this then it will need a little more thinking about. By fixing this behaviour it doesn't prevent people adding custom text should they want to in the meantime.

[jeysal]

I'm not sure I agree that it's a breaking change, I doubt anyone does depend on this behaviour

Not with

a | b
some thing

'$a => $bsomething'

perhaps, but quite possibly with

a | some thing
${1} | ${2}

'$a => $something'

[chrisblossom]

I don't think this should be considered a breaking change. Although it is true that someone's test would break with a b | c, this is a bug and (most likely) unintended by the user.

[chrisblossom]

Are there any updates on this (and/or #8717)?

[SimenB]

@mattphillips ping 🙂

[jeysal]

I think this is still good to go, but I would still say it's breaking because the example I gave in my last comment (using the behavior in headings) is actually not that far-fetched. Also, it's been lying around so long, might as well play it safe and put it into a major.

[SimenB]

@jeysal rebase and land?

[SimenB]

@jeysal would you be able to take a look at CI?

[jeysal]

I think the new snapshots it's trying to write look appropriate. pushed them

[github-actions]

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.