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

\x in the snapshot data creates a corrupted snapshot #10868

Open
pragmatta opened this issue May 6, 2024 Discussed in #10402 · 6 comments
Open

\x in the snapshot data creates a corrupted snapshot #10868

pragmatta opened this issue May 6, 2024 Discussed in #10402 · 6 comments
Labels
bug Something isn't working bun:test Something related to the `bun test` runner

Comments

@pragmatta
Copy link

pragmatta commented May 6, 2024

Discussed in #10402

Having \x in the snapshot data creates a corrupted snapshot that can't be read. For example this test

test("dummy", () => {
        expect("\"x\\x\"").toMatchSnapshot()
})

generates snapshot

exports[`dummy 1`] = `""x\x""`;

Which is invalid Javascript (I assume the file gets evaluated) as the \x is treated as a hex string and parsing the snapshot fails. Instead running the test will write it in the snapshot over and over again (and creates a problem that is hard to diagnose).

@pragmatta
Copy link
Author

Also when the data does not result in an invalide escapepe sequence, it fails to properly parse out snapshots, resulting in test fails.

For example test:

    test("dummy", () => {
        expect({foo:"h\\i"}).toMatchSnapshot()
    })

results in snapshot:

exports[`dummy 1`] = `
{
  "foo": "h\i",
}
`;

that fails the test because of parsing mismatch:

37 |     test("dummy", () => {
38 |         expect({foo:"h\\i"}).toMatchSnapshot()
             ^
error: expect(received).toMatchSnapshot(expected)

Expected:
{
  "foo": "hi",
}

Received:
{
  "foo": "h\i",
}

@Electroid Electroid added bug Something isn't working bun:test Something related to the `bun test` runner labels May 6, 2024
@Jarred-Sumner
Copy link
Collaborator

The state of our snapshots implementation isn't great.

@pragmatta
Copy link
Author

pragmatta commented May 9, 2024

The state of our snapshots implementation isn't great.

Is there a way to read/parse the snapshots (easily), how does Bun do it? I want to cross-reference outputs of different implementations of same abstraction. I tried requiring the snapshot-file and reads an [object Module] but has no properties..

const snapshots = require("./__snapshots__/OINOApi.test.ts.snap")
const keys = Object.keys(snapshots)
for (const key of keys) {
  console.log(key, snapshots[key])
}

@pragmatta
Copy link
Author

pragmatta commented May 9, 2024

I tried requiring the snapshot-file and reads an [object Module] but has no properties..

Figured this one out, the extension of the file needs to be .js for it to work. Node seems to not care about this.

@pragmatta
Copy link
Author

@Jarred-Sumner this seems to have been solved in the latest version (there was a lot of test runner related commits even if couldn't find anything directly related).

@pragmatta
Copy link
Author

I was wrong, the problem did not go away, possibly a bug on my side made it disappear on my side. Ended up fixing this in my tests by adding extra layer of escaping like
expect(enc(foo)).toMatchSnapshot()
You need to escape at least

  • all JSON-sequences (\r, \t, etc)
  • all backticks

@pragmatta pragmatta reopened this Nov 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working bun:test Something related to the `bun test` runner
Projects
None yet
Development

No branches or pull requests

3 participants