feat: interactively review failed snapshots by macarie · Pull Request #11325 · vitest-dev/vitest

macarie · 2026-09-22T15:10:12Z

Description

Resolves #2229
Closes #9297

UX largely inspired by Insta's review command.

Demo on a project with different types of snapshots:

interactive-review.webm

Possible follow-ups:

Handle snapshot removal/obsolete snapshots
Flag to run snapshots as soft assertions (?)

Please don't delete this checklist! Before submitting the PR, please make sure you do the following:

It's really useful if your PR references an issue where it is discussed ahead of time. If the feature is substantial or introduces breaking changes without a discussion, PR might be closed.
Ideally, include a test that fails without this PR but passes with it.
Please, don't make changes to pnpm-lock.yaml unless you introduce a new test example.
Please check Allow edits by maintainers to make review process faster. Note that this option is not available for repositories that are owned by Github organizations.

Tests

Run the tests with pnpm test:ci.

Documentation

If you introduce new functionality, document it. You can run documentation with pnpm run docs command.

Changesets

Changes in changelog are generated from PR name. Please, make sure that it explains your changes in an understandable manner. Please, prefix changeset messages with feat:, fix:, perf:, docs:, or chore:.

macarie · 2026-10-08T21:43:21Z

+  ) => Promise<void>,
+) => Promise<void>
+
+abstract class SnapshotReviewer {


This class is totally unnecessary now, but I think it could be used as a base for having this feature in UI mode by creating a custom UISnapshotReviewer (like NodeSnapshotReviewer) with a custom renderer.

I think we should avoid speculative abstraction for now.

macarie · 2026-10-08T21:46:11Z

+    try {
+      await this.collectReviews(this.#review)
+      await this.#updateSnapshots()
+      // @todo what should happen when there's no approved updates?


Should we rerun all tests if no updates were approved?

macarie · 2026-10-08T21:55:17Z

+export async function saveSnapshotUpdates(
+  snapshots: Map<SnapshotEnvironment, readonly SnapshotUpdate[]>,
+): Promise<void> {


Not sure if this is the best way to handle the updates or maybe change SnapshotState, but it seems to be working fine with all kinds of snapshots.

It should probably be marked as @internal tho 🤔

I think ideally we'd want get the failure/update information from SnapshotState since that's more like a ground truth source. I didn't think through what it would actually take for that though.

Code wise, the approach with the error meta and chai.ts stack fixup does look fairly clean, so it's not a big deal, but I'd like to see how the SnapshotState alternative approach looks like realistically.

I've created a poc for alternative approach #11790. It also discovered the independent bug #11791, which can be likely fixed on its own.

I thought that bug was just a weird interaction between soft, resolves/rejects, and the snapshot matchers. I should have checked 😅

macarie · 2026-10-13T11:48:09Z


 export interface RawSnapshot extends RawSnapshotInfo {
  snapshot: string
-  file: string


This was a redundant property since it's set in RawSnapshotInfo already

macarie · 2026-10-13T11:54:16Z

+    }
+  }
+
+  await Promise.all(fsEvents)


Not sure how I feel about waiting for these events to fire, since they might not be that reliable, but so far it seems it's working fine 👀

The watcher workaround looks quite a magic. For example, it looks like saveInlineSnapshots/saveSnapshotFile/saveRawSnapshots aren't awaited but is it also part of it?

From the code perspective, it looks more natural for watcher workaround to live on snapshot-review.ts than saveSnapshotUpdates. Also I wonder if it would be enough to just add some delay of setTimeout and pray watcher is settled for practice cases. (If it's CI issue, then we may find different surgical workaround for that specifically).

Yes, it tracks file updates (with a timeout) and it doesn't await the updates themselves.

I agree with you, it would make more sense for it to live in the review process rather than here, I'm going to do that 👍🏼

Also I wonder if it would be enough to just add some delay of setTimeout and pray watcher is settled for practice cases.

I tried but fs events seem to be quite "random" in timing, so it's easy to get a re-run after enabling the watcher unless using a high timeout.

Now the update waits for the saves and then the reviewer waits for the events only after running the tests, just in case some are still pending being dispatched.

macarie · 2026-10-21T14:24:02Z

+  review: {
+    shorthand: 'i',
+    // @todo implement
+    description:
+      'Run tests with soft snapshot assertions to collect all proposed changes, then review each one interactively to accept, reject, or skip it',
+  },


The idea here would be to (as the description says) run snapshot assertions as soft assertions. This way we can collect all the failures and have a single review phase, otherwise users would have to run → review → [run → review → [...]].

If we decide to go with it, it should probably be in a follow-up task tho.

macarie · 2026-10-21T14:34:53Z

+const DIFF_COL_WIDTH = 5
+const HUNK_RE = /^@@ -(\d+)(?:,\d+)? \+(\d+)(?:,\d+)? @@/
+
+function formatDiff(diff: string): string {


This might be unnecessary, but I like that it keeps the same layout throughout a review:

interactive-review.webm

netlify · 2026-10-22T10:58:31Z

✅ Deploy Preview for vitest-dev ready!

Built without sensitive environment variables

Name	Link
🔨 Latest commit	`68e358b`
🔍 Latest deploy log	https://app.netlify.com/projects/vitest-dev/deploys/6ae45f0392a337a7c0d9916b
😎 Deploy Preview	https://deploy-preview-11325--vitest-dev.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.
🤖 Make changes	Run an agent on this branch

To edit notification comments on pull requests, go to your Netlify project configuration.

hi-ogawa · 2026-10-29T09:50:44Z

          message: () => errorMessage || 'Snapshot properties mismatched',
          actual: received,
          expected: properties,
+          meta: createMetaObject({


This Snapshot properties mismatched case's meta isn't from snapshot failure but only matcher mismatch so it's not usable for review and seems to break the review update. Perhaps, we can remove meta thus exclude from review?

agent repro 🤖

import { appendFileSync, readFileSync } from 'node:fs' import { expect, test } from 'vitest' import { runInlineTests } from '../../../test-utils' test('property matcher failure', async () => { const { vitest, ctx, fs } = await runInlineTests( { 'basic.test.ts': ` import { expect, test } from 'vitest' test('props', () => { expect({ id: 123, name: 'x' }).toMatchSnapshot({ id: expect.any(String) }) }) `, '__snapshots__/basic.test.ts.snap': `// Vitest Snapshot v1, https://vitest.dev/guide/snapshot.html exports[\`props 1\`] = \` { "id": Any<String>, "name": "x", } \`; `, }, { update: 'none', watch: true }, ) const before = ctx!.vite.watcher.listenerCount('change') vitest.write('i') await vitest.waitForStdout('Reviewing [1/1]') appendFileSync('/tmp/probe-out.txt', String(['--- REVIEW SCREEN ---\n' + vitest.stdout].map(x => typeof x === 'string' ? x : JSON.stringify(x)).join(' ')) + '\n') vitest.write('a') await vitest.waitForStdout('Watching for file changes') await new Promise(r => setTimeout(r, 500)) const after = ctx!.vite.watcher.listenerCount('change') appendFileSync('/tmp/probe-out.txt', String(['--- SNAP FILE ---\n' + readFileSync(fs.resolveFile('__snapshots__/basic.test.ts.snap'), 'utf-8')].map(x => typeof x === 'string' ? x : JSON.stringify(x)).join(' ')) + '\n') appendFileSync('/tmp/probe-out.txt', String(['--- change listeners ---', { before, after }].map(x => typeof x === 'string' ? x : JSON.stringify(x)).join(' ')) + '\n') })

~~Does #11790 cover this bit too?~~

Yes, it's been removed 😄

hi-ogawa · 2026-10-29T10:08:59Z

+  ) => Promise<void>,
+) => Promise<void>
+
+abstract class SnapshotReviewer {


I think we should avoid speculative abstraction for now.

hi-ogawa · 2026-10-29T10:15:15Z

+    }
+  }
+
+  await Promise.all(fsEvents)


From the code perspective, it looks more natural for watcher workaround to live on snapshot-review.ts than saveSnapshotUpdates. Also I wonder if it would be enough to just add some delay of setTimeout and pray watcher is settled for practice cases. (If it's CI issue, then we may find different surgical workaround for that specifically).

macarie force-pushed the feat/snapshot-review branch 2 times, most recently from 1241c1a to dfac52e Compare October 8, 2026 21:29

feat: add mode to interactive review failed snapshots

15e35db

macarie force-pushed the feat/snapshot-review branch from dfac52e to 15e35db Compare October 8, 2026 21:39

macarie commented Oct 8, 2026

View reviewed changes

macarie added 6 commits October 10, 2026 22:05

chore: unslop watcher

bde8fc6

chore: undo unnecessary rename

b9f3511

chore: clear screen with built-in method

3cc9012

chore: remove unnecessary property from RawSnapshot

64ac38a

chore: track and wait for FS events created by snapshot updates

0793ccb

chore: undo stdin leftover changes

5a8b165

macarie commented Oct 13, 2026

View reviewed changes

macarie added 6 commits October 15, 2026 13:12

chore: track fs event before saving

e650c52

chore: improve reviewable snapshot detection

d547e4b

fix: snapshot error stack when used with async expect

1e59ead

chore: show diffs in an editor-like view

8cb1da4

chore: remove commented-out code

d82be44

chore: remove unnecessary diff check

398c9d3

macarie commented Oct 21, 2026

View reviewed changes

macarie force-pushed the feat/snapshot-review branch from d3a7a1c to fe628d9 Compare October 21, 2026 20:01

test: snapshot review functionality

92d45e5

macarie force-pushed the feat/snapshot-review branch from fe628d9 to 92d45e5 Compare October 21, 2026 20:14

macarie added 8 commits October 21, 2026 22:38

chore: use relative from pathe

26b6f74

chore: show key for raw snapshots too

8be4cfc

chore: check that interactive review is done

bf6a494

chore: normalize tracked paths on Windows

393bcbe

chore: set timeout to fs event tracker

4158db0

chore: fix test formatting

cde6f71

chore: use pathe in test

18683fb

chore: remove review CLI flag

b10cd58

macarie added 5 commits October 23, 2026 01:25

chore: reduce number of tests

53e8509

fix: custom domain snapshot render differencies

45eb9b4

chore: tests

7c3f415

chore: remove crlf test

6c2729e

chore: fix linting issue

1101f88

hi-ogawa added this to the 5.1.0 milestone Oct 23, 2026

macarie added 2 commits October 23, 2026 10:46

chore: mask source column number

64ca4ef

fix: TS error accessing error meta properties

8c9e83e

macarie marked this pull request as ready for review October 23, 2026 11:12

macarie requested review from hi-ogawa and sheremet-va October 23, 2026 11:13

hi-ogawa reviewed Oct 29, 2026

View reviewed changes

Comment thread packages/vitest/src/node/snapshot-review.ts Resolved

This was referenced Oct 30, 2026

refactor(snapshot): review pending updates from state #11790

Draft

expect.soft + .resolves/.rejects failures lose their location #11791

Closed

hi-ogawa and others added 4 commits October 30, 2026 18:10

Merge branch 'main' into feat/snapshot-review

68e358b

chore: apply review

8dc87a9

refactor: reuse files updates set

a7ee15a

chore: cleanup gitattributes

9688c49

Conversation

macarie commented Sep 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Please don't delete this checklist! Before submitting the PR, please make sure you do the following:

Tests

Documentation

Changesets

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

macarie Oct 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

macarie Oct 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

netlify Bot commented Oct 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ Deploy Preview for vitest-dev ready!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

macarie Oct 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

This branch has not been deployed

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

macarie commented Sep 22, 2026 •

edited

Loading

macarie Oct 8, 2026 •

edited

Loading

macarie Oct 13, 2026 •

edited

Loading

netlify Bot commented Oct 22, 2026 •

edited

Loading

macarie Oct 30, 2026 •

edited

Loading