search

gkampitakis / go-snaps

Jest-like snapshot testing in Go 📸
https://pkg.go.dev/github.com/gkampitakis/go-snaps
MIT License
174 stars 6 forks source link

[Bug]: Cryptic error message for snaps saved using "CRLF" instead of "LF" line ending #63

Closed JBongars closed 1 year ago

JBongars commented 1 year ago

Description

Pulling the snaps on a Windows and running the tests on a Docker (Linux) environment produces the following error:-

--- FAIL: TestFoo (0.01s)
  --- FAIL: TestFoo/test_snapshot (0.01s)
      foo_test.go:34: snapshot not found

This error is cryptic because suggests that the file doesn't exist when it is actually the wrong format. This can happen when developer clones a repository from Github and attempts to run the tests in a Docker container.

Steps to Reproduce

  1. Go to snapshot folder.
  2. Select a snap
  3. Change the line ending from LF to CRLF on a Windows machine
  4. Attempt to run the tests in a Docker container or some kind of Linux environement

Expected Behavior

Proposal: Update the docs to highlight this issue and maybe change the error message to be less cryptic. i.e.

--- FAIL: TestFoo (0.01s)
  --- FAIL: TestFoo/test_snapshot (0.01s)
      foo_test.go:34: snapshot not found or wrong format

Proposed changes to docs under notes. (Tried to create a PR but my branch was not accepted):

5. If you are running this package on both a Windows and Linux environment, you may encounter the following error:-

\```bash
--- FAIL: TestFoo (0.01s)
  --- FAIL: TestFoo/test_snapshot (0.01s)
      foo_test.go:34: snapshot not found
\```

this is because the snapshots are saved with a different line ending on each platform (LF on Linux, CRLF on Windows). To fix this, you can use the [dos2unix](https://pkgs.alpinelinux.org/package/edge/community/x86/dos2unix) tool to convert the line endings to LF on Linux
gkampitakis commented 1 year ago

Hey 👋 first of all thanks for opening this issue and spending time on this detailed description.

Secondly I would like to know why your pr was not accepted 🤔 If you want to contribute you need to fork the repo, and then create a pull request here with your work. If you did this and didn't work happy to help with this.

As for the issue you mentioned, I can definitely see that when changing a file from LF to CRLF and running the tests it leads to snapshot errors but not on foo_test.go:34: snapshot not found.

snapshot not found is only returned in this case here https://github.com/gkampitakis/go-snaps/blob/main/snaps/matchSnapshot.go#L34-L38 where a snapshot doesn't exist and the go-snaps "detects" that's running on CI so it fails instead of creating the snapshot. So I think the message is not cryptic but does exactly what it says.

As for the LF to CRLF diff errors, I want to spend some time to investigate a bit more on how the library behaves and if it's possible to mitigate or inform the user, else we can add a disclaimer on the docs.

JBongars commented 1 year ago

@gkampitakis just saw the new release, thanks for fixing this issue :)

gkampitakis commented 1 year ago

Thanks for reporting the issue. Looks like my handwritten regex could not handle CRLF so it was not able to find snasphots inside the file and was reporting snapshot not found or wrong format. Tell me if now works for you 😄