fix-steps-snapshots

[goloveychuk]

Attempt to fix #35285

[goloveychuk]

@microsoft-github-policy-service agree

[github-actions]

Test results for "tests 1"

15 failed
❌ [playwright-test] › tests/ui-mode-test-progress.spec.ts:22:5 › should update trace live @macos-latest-node18-1
❌ [playwright-test] › tests/ui-mode-trace.spec.ts:121:5 › should show snapshots for sync assertions @macos-latest-node18-1
❌ [playwright-test] › tests/ui-mode-trace.spec.ts:341:5 › should work behind reverse proxy @macos-latest-node18-1
❌ [playwright-test] › tests/ui-mode-test-progress.spec.ts:22:5 › should update trace live @ubuntu-latest-node18-1
❌ [playwright-test] › tests/ui-mode-trace.spec.ts:121:5 › should show snapshots for sync assertions @ubuntu-latest-node18-1
❌ [playwright-test] › tests/ui-mode-trace.spec.ts:341:5 › should work behind reverse proxy @ubuntu-latest-node18-1
❌ [playwright-test] › tests/ui-mode-test-progress.spec.ts:22:5 › should update trace live @ubuntu-latest-node20-1
❌ [playwright-test] › tests/ui-mode-trace.spec.ts:121:5 › should show snapshots for sync assertions @ubuntu-latest-node20-1
❌ [playwright-test] › tests/ui-mode-trace.spec.ts:341:5 › should work behind reverse proxy @ubuntu-latest-node20-1
❌ [playwright-test] › tests/ui-mode-test-progress.spec.ts:22:5 › should update trace live @ubuntu-latest-node22-1
❌ [playwright-test] › tests/ui-mode-trace.spec.ts:121:5 › should show snapshots for sync assertions @ubuntu-latest-node22-1
❌ [playwright-test] › tests/ui-mode-trace.spec.ts:341:5 › should work behind reverse proxy @ubuntu-latest-node22-1
❌ [playwright-test] › tests/ui-mode-test-progress.spec.ts:22:5 › should update trace live @windows-latest-node18-1
❌ [playwright-test] › tests/ui-mode-trace.spec.ts:121:5 › should show snapshots for sync assertions @windows-latest-node18-1
❌ [playwright-test] › tests/ui-mode-trace.spec.ts:341:5 › should work behind reverse proxy @windows-latest-node18-1

1 flaky

⚠️ [firefox-page] › tests/page/page-evaluate.spec.ts:403:3 › should throw for too deep reference chain @firefox-ubuntu-22.04-node18

38772 passed, 808 skipped
✔️✔️✔️

Merge workflow run.

[yury-s]

We'll need a test for it.

[yury-s]

This is risky, we don't know whether the action change the state before or after the step end. I'd not do this.

[goloveychuk]

Yeah. It is. But if I'll remove it, if test step is last, we won't find "after" snapshot.

This (and all other inaccuracy) could be solved if additional snapshots will be made on step start and end.

[goloveychuk]

I'll write a test if solution is accepted. I'd still prefer to make additional screenshots, because debugging e2e is very painful, and precision in screenshots timing is very important.

[goloveychuk]

I see that you have screenshot content based deduplication

https://github.com/microsoft/playwright/blob/main/packages/playwright-core/src/server/trace/recorder/tracing.ts#L609

Which makes additional screenshots for steps not wasting more disk space (if screenshot is the same).
Although it will add runtime overhead for test (in tracing mode), it makes tracing very reliable for steps.

[goloveychuk]

Additional deduplication check could be added to take into account nested steps and immediate calls.
For this, nodejs behavior (microtasks, promises execution) should be discovered.
https://v8.dev/blog/fast-async
Some simple heuristics like several microseconds delta could be used. Or mb nodejs async store api to track promises. Or some other way to calc ticks.