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

Add options to resolveSnapshotPath #1620

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

hermanbanken
Copy link

My team wants to save our snapshots in different files, depending on some variables that are defined by a table (describe.each). Specifically we want to run the same test for various versions of clients. Storing all of these snapshots in the same file has some very annoying downsides, because the diff tools will confuse the addition and deletion of versions and display generally unhelpful outputs for human review.

Today the snapshot path depends only on the test file and the extension (.snap) and given that it runs in a different thread (using some RPC protocol) we can't pass down variables using AsyncLocalStorage or other mechanisms with global state.

This PR adds the easily customizable Test.Context object to the snapshot resolver, allowing users to conveniently specify a different path per context.

I'm not sure this works due to the nature of the RPC: can it send other parameters than strings? Does it fall apart if the object isn't JSON serialisable? I'm not sure. But I wanted to create this PR already, to make the proposal and gather some opinions.

@netlify
Copy link

netlify bot commented Jul 9, 2022

Deploy Preview for vitest-dev ready!

Built without sensitive environment variables

Name Link
🔨 Latest commit d82aa87
🔍 Latest deploy log https://app.netlify.com/sites/vitest-dev/deploys/62c953cbacaa9200087cad5d
😎 Deploy Preview https://deploy-preview-1620--vitest-dev.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@sheremet-va
Copy link
Member

sheremet-va commented Jul 9, 2022

I'm not sure this works due to the nature of the RPC: can it send other parameters than strings? Does it fall apart if the object isn't JSON serialisable? I'm not sure. But I wanted to create this PR already, to make the proposal and gather some opinions.

These are the rules for serialisation: https://developer.mozilla.org/en-US/docs/Web/API/Web_Workers_API/Structured_clone_algorithm

If it fails to clone, it will throw an error.

Context is actually a function, so it cannot be cloned.

@sheremet-va
Copy link
Member

sheremet-va commented Jul 31, 2022

I am not against the idea, but we will need to change how we resolve snapshot paths. If we change resolveSnapshotPath from function to a string (path to a module), then we can import it inside test thread, and pass whatever we want. This will be a breaking change:

resolveSnapshotPath?: (path: string, extension: string) => string

- resolveSnapshotPath?: (path: string, extension: string) => string
+ resolveSnapshotPath?: string

The module should return default as a function that we used previously.

@thomasmattheussen
Copy link

My team wants to save our snapshots in different files

We have the exact same problem. We'd like to use vitest but are now stuck using jest with https://github.com/igor-dv/jest-specific-snapshot

@unional
Copy link

unional commented Oct 21, 2024

I'm using workspace and browser mode and here is my 2 cents:

  • to work with workspace, the approach also needs projectRoot. For string approach, that means some templating ({projectRoot}/... or <projectRoot>/...)
  • page.screenshot() creates images with counter (a.spec.tsx/test-something-{1..n}.png). resolveSnapshotPath() does not have a way to do the same. Need that counter or additional templating.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Approved
Development

Successfully merging this pull request may close these issues.

4 participants