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

Snapshots modify input by unexpectedly trimming and adjusting indentation #6107

Closed
6 tasks done
Bastian opened this issue Jul 12, 2024 · 3 comments · Fixed by #7156 or salesforce/lwc#5144
Closed
6 tasks done
Labels
p3-minor-bug An edge case that only affects very specific usage (priority)

Comments

@Bastian
Copy link

Bastian commented Jul 12, 2024

Describe the bug

When performing snapshot testing with strings using expect("some string").toMatchSnapshot(), I expect that any difference in the string should be detected. However, the input string is modified by trimming whitespace and adjusting indentation.

For instance, both of the following lines succeed for the same example.txt with the string "Whitespace gets trimmed", even though one might expect the second line to fail due to additional whitespace:

expect(`Whitespace gets trimmed`).toMatchFileSnapshot('example.txt');
expect(`   Whitespace gets trimmed   `).toMatchFileSnapshot('example.txt'); // Should fail, but succeeds

The code in Vitest responsible for modifying the string is located here:

export function prepareExpected(expected?: string) {
function findStartIndent() {
// Attempts to find indentation for objects.
// Matches the ending tag of the object.
const matchObject = /^( +)\}\s+$/m.exec(expected || '')
const objectIndent = matchObject?.[1]?.length
if (objectIndent) {
return objectIndent
}
// Attempts to find indentation for texts.
// Matches the quote of first line.
const matchText = /^\n( +)"/.exec(expected || '')
return matchText?.[1]?.length || 0
}
const startIndent = findStartIndent()
let expectedTrimmed = expected?.trim()
if (startIndent) {
expectedTrimmed = expectedTrimmed
?.replace(new RegExp(`^${' '.repeat(startIndent)}`, 'gm'), '')
.replace(/ +\}$/, '}')
}
return expectedTrimmed
}

This also causes problems with the diff view in the console output.
Given a file example.yml with a multiline string that contains a line with the string " } " (the space after } is important)

example: |
  {
    echo "Just some bash operation."
  } 
some:
  nesting:
    - "hello world"
even:
  more:
    nesting: true

the following test

const yml = fs.readFileSync('test/example.yml', {
  encoding: 'utf8',
  flag: 'r',
});

// This succeeds as expected
await expect(yml).toMatchFileSnapshot('example.yml');

const modifiedYml = yml.replaceAll('world', 'mars');
// This fails as expected, but the diff is broken
await expect(modifiedYml).toMatchFileSnapshot('example.yml');

// When not using file snapshots, the error diffs looks as expected
// (Uncomment previous expect to see output)
expect(modifiedYml).toEqual(yml);

produces the following console output:

image

Notice that the green "expected" part of the diff is "indentation-adjusted", while the red "received" part is not. This does not happen when using toEqual directly, where the diff looks as expected:

image

I ran into this problem when testing Kubernetes artifacts generated using cdk8s.

Reproduction

https://stackblitz.com/edit/vitest-dev-vitest-ftiqce?file=test%2Fbasic.test.ts

System Info

$npx envinfo --system --npmPackages '{vitest,@vitest/*,vite,@vitejs/*}' --binaries --browsers

  System:
    OS: Linux 5.0 undefined
    CPU: (8) x64 Intel(R) Core(TM) i9-9880H CPU @ 2.30GHz
    Memory: 0 Bytes / 0 Bytes
    Shell: 1.0 - /bin/jsh
  Binaries:
    Node: 18.20.3 - /usr/local/bin/node
    Yarn: 1.22.19 - /usr/local/bin/yarn
    npm: 10.2.3 - /usr/local/bin/npm
    pnpm: 8.15.6 - /usr/local/bin/pnpm
  npmPackages:
    @vitest/ui: latest => 2.0.2 
    vite: latest => 5.3.3 
    vitest: latest => 2.0.2 

(From Stackblitz, probably not relevant)

Used Package Manager

npm

Validations

@dangmai
Copy link

dangmai commented Jan 1, 2025

I am also seeing this behavior. For my project it's a bit of a deal breaker, because it's a Prettier plugin and every newline/indentation is important to check for regressions.
This is a very unexpected behavior, and as far as I can tell there's no way currently to avoid that code path in Vitest. Please correct me if I'm wrong.

@rassie
Copy link

rassie commented Jan 1, 2025

@dangmai if it helps, we are working around it right now using patch-package with the following patch for vitest 2.1.3. The patch will obviously will need to be updated for each vitest upgrade, but it's small enough that it's feasible until the fix arrives.

diff --git a/node_modules/@vitest/snapshot/dist/index.js b/node_modules/@vitest/snapshot/dist/index.js
index d467d8c..fe7de26 100644
--- a/node_modules/@vitest/snapshot/dist/index.js
+++ b/node_modules/@vitest/snapshot/dist/index.js
@@ -840,6 +840,7 @@ ${snapshots.join("\n\n")}
 function prepareExpected(expected) {
   function findStartIndent() {
     var _a, _b;
+    return 0;
     const matchObject = /^( +)\}\s+$/m.exec(expected || "");
     const objectIndent = (_a = matchObject == null ? void 0 : matchObject[1]) == null ? void 0 : _a.length;
     if (objectIndent) {

@dangmai
Copy link

dangmai commented Jan 1, 2025

@rassie Thank you for that workaround, I'll check that out and see how maintainable it is in the long run!

@hi-ogawa hi-ogawa added p3-minor-bug An edge case that only affects very specific usage (priority) and removed pending triage labels Jan 2, 2025
@github-actions github-actions bot locked and limited conversation to collaborators Jan 18, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
p3-minor-bug An edge case that only affects very specific usage (priority)
Projects
None yet
4 participants