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

Why test.each table-variable for string data type, has double quotes? #7689

Closed
nicewaytodoit opened this issue Jan 23, 2019 · 8 comments · Fixed by #7694
Closed

Why test.each table-variable for string data type, has double quotes? #7689

nicewaytodoit opened this issue Jan 23, 2019 · 8 comments · Fixed by #7694
Milestone

Comments

@nicewaytodoit
Copy link

💬 Questions and Help

If I use code like this:

  test.each`
    input    | configObject  | expectedResult | configDescription
    ${"abc"} | ${undefined}  | ${undefined}   | ${"none"}
    ${5.1}   | ${undefined}  | ${"£5.10"}     | ${"none"}
  `(
    `converts $input to $expectedResult with config: $configDescription`,

test will generate following output:

    √ converts "abc" to undefined with config: "none" (6ms)
    √ converts 5.1 to "£5.10" with config: "none" (1ms)

Why is the string the only type that has double quotes, if we needed double quotes couldn't we add them on our own? (basically I am looking way remove those quotes which will give me more descriptive ability when I write tests)
Can you give me source code link/lines that does transformation?

Thank you,

@nicewaytodoit nicewaytodoit changed the title Why test each table variable for strings have double quotes? Why test.each table-variable for string data type, has double quotes? Jan 24, 2019
@SimenB
Copy link
Member

SimenB commented Jan 24, 2019

The reason is because we use pretty-format to format the values, and it adds the quotes

https://github.com/facebook/jest/blob/6de22dde9a10f775adc7b6f80080bdd224f6ae31/packages/jest-each/src/bind.js#L198

I agree it's suboptimal, though. @mattphillips maybe special case primitives and not pass them through pretty-format?

@mattphillips
Copy link
Contributor

@SimenB I agree it makes sense to just use primative’s .toString and only use pretty-format for objects etc 👍

I’ll try and take a look at this today, it’s probably a breaking change so might be good to get out with 24?

@SimenB
Copy link
Member

SimenB commented Jan 24, 2019

I wouldn't necessarily call it breaking (it'd just affect the string in reports, which I think we should reserver the right to change at will). However, it would be nice to get it into 24 😀

@mattphillips
Copy link
Contributor

Yeah I suppose it could be called a bug fix tbf, that said it could break snapshots 🤷‍♂️

@SimenB
Copy link
Member

SimenB commented Jan 24, 2019

I'll only break if people snapshot the output of Jest, which I think is fair? Who knows 😛

@jeysal
Copy link
Contributor

jeysal commented Jan 24, 2019

I think what Matt is referring to is the name of the snapshots when you're using snapshots in test.each? I'd also consider this breaking.

@SimenB
Copy link
Member

SimenB commented Jan 24, 2019

Ah! That's a good point, didn't think of that. Would be great to get in int time for 24, then

@github-actions
Copy link

This issue 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.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants