Vitest always writes snapshot files on Windows 10 with Node 18.17.1

[christoph-schlumpf]

Describe the bug

On Windows 10 with Node 18.17.1, vitest always writes the snapshot files when running the tests, even if there are no changes in the snapshot. This leads to unnecessary writes and changed files in the repo.

Reason for this issue is this code:

vitest/packages/snapshot/src/env/node.ts

Line 39 in eca25dc

async readSnapshotFile(filepath: string): Promise<string | null> {

readSnapshotFile always reads an empty string ("") on Windows. Additionally, many repos on windows have git config autocrlf enabled.

I fixed it with the following patch:

const readSnapshotFile = async (filepath) => {
    if (!existsSync(filepath)) {
        return null;
    }
    return promises
        .readFile(filepath, { encoding: "utf-8" })
        .then((content) => (process.platform === "win32" ? content.replace(/\r\n|\r/g, "\n") : content));
}; 

Reproduction

Can be reproced on any Windows machine with Node 18.17.1 and vitest with snapshots.

System Info

System:
    OS: Windows 10 10.0.19045
    CPU: (8) x64 11th Gen Intel(R) Core(TM) i7-1185G7 @ 3.00GHz
    Memory: 25.88 GB / 47.71 GB
  Binaries:
    Node: 18.17.1 - C:\eplatform\mytools\node\node.EXE
    npm: 9.6.7 - C:\eplatform\mytools\node\npm.CMD
  Browsers:
    Edge: Spartan (44.19041.3636.0), Chromium (119.0.2151.58)
    Internet Explorer: 11.0.19041.3636
  npmPackages:
    @vitejs/plugin-react: ~4.0.0 => 4.0.4
    @vitest/coverage-istanbul: ^0.34.6 => 0.34.6
    @vitest/ui: ~0.32.0 => 0.32.4
    vite: ~4.3.9 => 4.3.9
    vitest: 0.34.6 => 0.34.6

Used Package Manager

npm

Validations

• Follow our Code of Conduct
• Read the Contributing Guidelines.
• Read the docs.
• Check that there isn't already an issue that reports the same bug to avoid creating a duplicate.
• Check that this is a concrete bug. For Q&A open a GitHub Discussion or join our Discord Chat Server.
• The provided reproduction is a minimal reproducible example of the bug.

[sheremet-va]

We don't think this should be built-in, but we can expose an option to override the default behaviour: #5449

[christoph-schlumpf]

Hi @sheremet-va Thank you for the information. we will try out the override solution.