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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Inline snapshots are indented at col 0 #11459

Closed
jasonlimantoro opened this issue May 27, 2021 · 12 comments 路 Fixed by #11560
Closed

Inline snapshots are indented at col 0 #11459

jasonlimantoro opened this issue May 27, 2021 · 12 comments 路 Fixed by #11560

Comments

@jasonlimantoro
Copy link

jasonlimantoro commented May 27, 2021

馃挜 Regression Report

Inline snapshots are indented at col 0, looks like #9477 resurfaced.

Last working version

Worked up to version: 26.6.3

Stopped working in version: 27.0.1

To Reproduce

Steps to reproduce the behavior: #9477

Run npx envinfo --preset jest

Paste the results here:

System:
    OS: Linux 5.10 Alpine Linux
    CPU: (2) x64 Intel(R) Core(TM) i5-7267U CPU @ 3.10GHz
  Binaries:
    Node: 12.22.1 - /usr/local/bin/node
    Yarn: 2.4.1 - /usr/local/bin/yarn
    npm: 6.14.12 - /usr/local/bin/npm
@G-Rath
Copy link
Contributor

G-Rath commented Jun 10, 2021

Interesting - my test seems to still be passing, so that's odd 馃

@jasonlimantoro
Copy link
Author

Have you tried adding a new inline snapshot after the upgrade? The bug seems to appear after you add a new inline snapshot expectation in any test case file that has already had some inline snapshots.

So, If u didn鈥檛 touch your old tests (the tests that work for Jest 26), then they will not break and therefore, the bug is not reproduced.

@G-Rath
Copy link
Contributor

G-Rath commented Jun 10, 2021

That's the conditions my test was meant to test for, since that was the bug the first time around :)

@G-Rath
Copy link
Contributor

G-Rath commented Jun 10, 2021

(sorry to be clear, when I say "my test", I mean the test I added to the jest test suite when I did the fix for #9477, not tests in my local projects 馃槄)

@jasonlimantoro
Copy link
Author

That's the conditions my test was meant to test for, since that was the bug the first time around :)

Your first comment was mind-boggling to me. After reading your next comment, now it makes sense LOL 馃槀.

Perhaps I could try to isolate the issue in a reproducible repository, see if the bug persists.

@jasonlimantoro
Copy link
Author

jasonlimantoro commented Jun 10, 2021

Here is a repo:
https://github.com/jasonlimantoro/jest-27-inlinesnapshots (using jest 27.0.4, but the bug is still there).

  • There is one test file in test/index.test.js, with an inline snapshot indented at col 0.
describe("Index", () => {
    it("should work", () => {
        expect({ name: "john" }).toMatchInlineSnapshot(`
Object {
  "name": "john",
}
`);
    });
});
  • Remove the string inside the backtick and then run the test (yarn test) to verify this behavior.

  • If you remove the snapshots, downgrade to version 26, and then run the test again

    yarn add jest@26
    yarn test
    

    the inline snapshots will be indented correctly again.

describe("Index", () => {
    it("should work", () => {
        expect({ name: "john" }).toMatchInlineSnapshot(`
            Object {
              "name": "john",
            }
        `);
    });
});

@G-Rath
Copy link
Contributor

G-Rath commented Jun 10, 2021

I strongly suspect this is related to the prettier change.

I found that if I have a formatting "error" (i.e a missing semicolon), then that is corrected and the snapshots are updated with correct indenting, but if there's nothing for prettier to do then we get the lack of indenting behaviour.

@SimenB
Copy link
Member

SimenB commented Jun 14, 2021

/cc @jeysal any idea? Seems like a regression from the "uglier snapshots" thing, but weird a test didn't catch it

@jeysal
Copy link
Contributor

jeysal commented Jun 14, 2021

No, I don't, sorry. Tbf I didn't write most of the actual impl for non-Prettier inline snapshots, was more involved in resolving the massive performance problems that prevented it from landing and then getting it over the line.

@G-Rath
Copy link
Contributor

G-Rath commented Jun 17, 2021

I was able to create a test for this, so #11560 is now good to go - I've updated the description to detail what the problem is and why it wasn't caught by the existing test suite.

@SimenB
Copy link
Member

SimenB commented Sep 29, 2021

@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 Oct 30, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants