feat(ui-mode): update individual snapshots by dcrousso · Pull Request #42421 · microsoft/playwright

Devin Rousso (dcrousso) · 2026-08-26T16:47:28Z

reviewing image diffs currently requires rerunning tests with snapshot updates enabled

add a button for each diff that copies the actual image over the expected snapshot

github-actions · 2026-08-26T17:57:27Z

Hi, I'm the Playwright bot and I took a first look at the CI failures here.

🟢 CI is clear — the one failure is a pre-existing flake

The single failure, [firefox] › mcp/cli-core.spec.ts:123 › select, is a known flake: the same test failed on the same mcp-windows-latest-firefox bot on an unrelated push SHA (610977b), and timed out on chromium/chrome on other SHAs/PRs. This PR only touches UI-mode snapshot-update code, which the MCP select tool doesn't exercise.

Details

Overall: nothing in this diff reaches the failing paths. The PR adds a "Save actual as expected" button in trace-viewer plus an updateSnapshot test-server endpoint (testServer.ts, testServerInterface.ts, attachmentsTab.tsx, uiModeTraceView.tsx, workbench.tsx). None of that is loaded by the MCP CLI or the library/page tests below.

Pre-existing flake / infra

[firefox] › mcp/cli-core.spec.ts:123 › select — pre-existing flake. In the test-results DB this test has failed/timed out on SHAs this PR can't be responsible for: firefox on push SHA 610977b (run 30020951381), chromium timedOut on push SHA 3e14efd (run 30034115836), and chrome timedOut on PR feat(trace): load trace files independently, produce a single context entry #42350 (run 32489690957). Firefox alone is green in 722 of 724 runs. The MCP browser_select path is untouched by this PR.

Not failures (no action)

The "tests 1" report lists 3 flaky, 0 failed — within-run flakes rescued on retry, all with long flake histories (e.g. firefox-page › page-goto.spec.ts:90 › should work with Cross-Origin-Opener-Policy rescued 302 times; page-event-request.spec.ts:181 › ...COOP... rescued 373 times; chromium.spec.ts:436 › Service Worker (advanced) rescued 239 times). Nothing to triage.

I'm a first pass, not the final word — but I couldn't find any failure that points back at this change.

_{Triaged by the Playwright bot - agent run}

Dmitry Gozman (dgozman) · 2026-08-27T07:54:15Z

There have been multiple attempts at this functionality (some examples: #12707, #14883, #24310, #28415) that never landed, so I'd recommend to first research what went wrong with previous attempts.

Pavel Feldman (pavelfeldman) · 2026-08-27T18:12:30Z

Requesting a demo of this one, unclear from the code what is happening and which scenarios this supports.

Devin Rousso (dcrousso) · 2026-08-28T18:40:09Z

before

after

Dmitry Gozman (dgozman) · 2026-08-31T08:36:28Z

  </div>;
 };

+function UpdateSnapshotButton({ actualPath, expectedPath, onUpdateSnapshot }: {


I think we agreed to flash a green checkmark instead of updating the text?

Dmitry Gozman (dgozman) · 2026-08-31T08:43:14Z

  />;
 };

+function addLocalAttachmentPaths(model: TraceModel, result: reporterTypes.TestResult) {


What's going on here? Why don't we have correct attachment paths? Why do we only call before a single setModel(), and not before all the others?

ive reworked this

Unfortunately, that did not make it clear. My questions still stand - what's going on, why do we need to correct attachment paths, do we get wrong paths there? I'd like to understand and, hopefully, fix the root cause of wrong attachment paths.

ah my apologies yes i shouldve been clearer

the completed trace intentionally replaces each local path with a generic file reference when it embeds the attachment into trace.zip

UI Mode renders the attachment from that archived file, but saving the actual image still needs the original local source and destination paths

TestResult.attachments retains those local paths, so the new code keeps a separate map between each trace attachment and its corresponding local path without mutating TraceModel

note that this is only needed for handling a completed trace because live traces have not been archived yet and still contain their local paths

reviewing image diffs in UI Mode requires rerunning tests with snapshot updates enabled add a button beside each diff to save its actual image as the expected snapshot resolve paths from registered attachments and reject ambiguous or stale saves

Pavel Feldman (pavelfeldman) · 2026-09-10T20:56:05Z

I'll archive due to no activity

github-actions · 2026-09-14T22:16:09Z

Test results for "tests 1"

1 failed
❌ [firefox-library] › library/browsercontext-storage-state.spec.ts:453 › should work when service worker is intefering and the origin is not open @firefox-ubuntu-22.04-node20

8 flaky

⚠️ [chromium-library] › library/video.spec.ts:699 › screencast › should capture full viewport on hidpi `@chromium-ubuntu-22.04-arm-node20`
⚠️ [chromium-library] › library/chromium/chromium.spec.ts:373 › should produce network events, routing, and annotations for Service Worker `@realtime-time-library-chromium-linux`
⚠️ [chromium-library] › library/video.spec.ts:736 › screencast › should work with video+trace `@realtime-time-library-chromium-linux`
⚠️ [chromium-page] › page/workers.spec.ts:191 › should attribute network activity for worker inside iframe to the iframe `@realtime-time-library-chromium-linux`
⚠️ [chromium-library] › library/video.spec.ts:699 › screencast › should capture full viewport on hidpi `@chromium-ubuntu-22.04-node24`
⚠️ [firefox-library] › library/browsercontext-cookies-third-party.spec.ts:257 › third party 'Partitioned;' cookies `@firefox-ubuntu-22.04-node20`
⚠️ [firefox-library] › library/browsercontext-cookies-third-party.spec.ts:470 › top level 'Partitioned;' cookie and same origin iframe `@firefox-ubuntu-22.04-node20`
⚠️ [firefox-page] › page/page-emulate-media.spec.ts:144 › should keep reduced motion and color emulation after reload `@firefox-ubuntu-22.04-node20`

51761 passed, 1247 skipped

Merge workflow run.

github-actions · 2026-09-14T22:19:55Z

🟢 The one failure is broken on `main`, not caused by this PR

Hi, I'm the Playwright bot and I took a look at the CI failures here.

[firefox-library] › browsercontext-storage-state.spec.ts:453 is failing on main too — it has failed 44 of 44 runs across 12 different SHAs in the CI results DB, with zero passes on any Firefox bot. Nothing in this PR touches it.

Details

This PR only touches UI mode / trace viewer (attachmentsTab, uiModeTraceView, workbench) and the test server RPC for updating snapshots. None of that is loaded by library tests.

Pre-existing failure (not this PR)

[firefox-library] › library/browsercontext-storage-state.spec.ts:453 › should work when service worker is intefering and the origin is not open — not even a flake, it's a hard breakage on main. Run history for firefox-ubuntu-22.04-node20, oldest first:

🟥🟥🟥🟥🟥🟥🟥🟥🟥🟥🟥🟥

It also fails on firefox-windows-latest, firefox-macos-15-large, firefox-macos-15-xlarge and tracing-firefox on unrelated main SHAs (e.g. 500c9c82, 249916f0, 4dc79d4a, a9da2dbe) and on other PRs (fix(storage): preserve IndexedDB Map and Set values #42707, feat(evaluate): opt in to Map and Set serialization #42709). Every attempt fails, retries included. The assertion picks up an extra cookie written by the service worker:
```
expect(received).toEqual(expected)
+   Object { "name": "fromServiceWorker", "value": "yes" }
```
Looks like a Firefox service-worker cookie-visibility change, worth a separate issue.

Flaky, all unrelated

The 8 flaky results (video screencast, third-party partitioned cookies, worker network attribution, emulate-media) all passed on retry and are in browser-level code this PR doesn't touch.

_{Triaged by the Playwright bot - agent run}

github-actions · 2026-09-14T22:20:55Z

Test results for "MCP"

8564 passed, 1446 skipped

Merge workflow run.

Devin Rousso (dcrousso) requested review from Dmitry Gozman (dgozman) and Pavel Feldman (pavelfeldman) August 26, 2026 16:47

This comment was marked as outdated.

Sign in to view

This comment has been minimized.

Sign in to view

Devin Rousso (dcrousso) force-pushed the fix-42393 branch 2 times, most recently from c38f708 to 7308b9e Compare August 28, 2026 18:39

This comment has been minimized.

Sign in to view

Dmitry Gozman (dgozman) reviewed Aug 31, 2026

View reviewed changes

Devin Rousso (dcrousso) force-pushed the fix-42393 branch from 7308b9e to 739adf5 Compare August 31, 2026 21:01

This comment has been minimized.

Sign in to view

Devin Rousso (dcrousso) requested a review from Dmitry Gozman (dgozman) September 2, 2026 22:29

Devin Rousso (dcrousso) force-pushed the fix-42393 branch from 739adf5 to 1fc593f Compare September 4, 2026 22:52

This comment has been minimized.

Sign in to view

Pavel Feldman (pavelfeldman) closed this Sep 10, 2026

github-actions Bot mentioned this pull request Sep 11, 2026

feat(cli): shut idle headless sessions down after an hour #42676

Merged

Devin Rousso (dcrousso) reopened this Sep 14, 2026

github-actions Bot mentioned this pull request Sep 15, 2026

test: fix click tests on android #42726

Merged

Conversation

Devin Rousso (dcrousso) commented Aug 26, 2026

Uh oh!

This comment was marked as outdated.

This comment has been minimized.

This comment has been minimized.

github-actions Bot commented Aug 26, 2026

🟢 CI is clear — the one failure is a pre-existing flake

Uh oh!

Dmitry Gozman (dgozman) commented Aug 27, 2026

Uh oh!

Pavel Feldman (pavelfeldman) commented Aug 27, 2026

Uh oh!

Devin Rousso (dcrousso) commented Aug 28, 2026

Uh oh!

This comment has been minimized.

This comment has been minimized.

Dmitry Gozman (dgozman) Aug 31, 2026

Choose a reason for hiding this comment

Uh oh!

Dmitry Gozman (dgozman) Aug 31, 2026

Choose a reason for hiding this comment

Uh oh!

Devin Rousso (dcrousso) Aug 31, 2026

Choose a reason for hiding this comment

Uh oh!

Dmitry Gozman (dgozman) Sep 2, 2026

Choose a reason for hiding this comment

Uh oh!

Devin Rousso (dcrousso) Sep 2, 2026

Choose a reason for hiding this comment

Uh oh!

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

Pavel Feldman (pavelfeldman) commented Sep 10, 2026

Uh oh!

github-actions Bot commented Sep 14, 2026

Test results for "tests 1"

Uh oh!

github-actions Bot commented Sep 14, 2026

🟢 The one failure is broken on main, not caused by this PR

Uh oh!

github-actions Bot commented Sep 14, 2026

Test results for "MCP"

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

🟢 The one failure is broken on `main`, not caused by this PR