fix(browser): fix updating snapshot during watch mode by hi-ogawa · Pull Request #4867 · vitest-dev/vitest

hi-ogawa · 2024-01-04T08:06:12Z

Description

Closes #4865

It might be a little tough to automate a test for this but I'll try something.

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.

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:.

netlify · 2024-01-04T08:06:17Z

✅ Deploy Preview for fastidious-cascaron-4ded94 canceled.

Name	Link
🔨 Latest commit	`d55ac5e`
🔍 Latest deploy log	https://app.netlify.com/sites/fastidious-cascaron-4ded94/deploys/659d5b59ba0bbc0008aa607a

test/browser/specs/update-snapshot.test.mjs

sheremet-va · 2024-01-04T11:25:14Z

test/browser/specs/update-snapshot.test.mjs

+
+test.after(async () => {
+  // force exitCode mutated by vitest
+  process.exitCode = 0


This seems sketchy. What if exitCode was mutated by node:test? Why Vitest mutates exitCode?

I didn't investigate much but I meant to do something similar as runVitest test util here:

vitest/test/test-utils/index.ts

Lines 40 to 42 in 044c738

exitCode = process.exitCode

process.exitCode = 0

process.exit = exit

I'll look into more detail what exactly is happening with exit code.

I refined some assertions to be more precise about process.exitCode d2366e8

Why Vitest mutates exitCode?

Vitest sets process.exitCode = 1 whenever test failed during runtFiles:

vitest/packages/vitest/src/node/core.ts

Lines 505 to 506 in d2366e8

if (hasFailed(files))

process.exitCode = 1

EDIT: I further refactored by introducing wrapWithExitCode utility ccfa85a

hi-ogawa · 2024-01-05T02:58:50Z

test/browser/specs/fix-4686.test.mjs (added in #4692) became flaky on playwright/webkit. I don't see other PRs having problems, so most likely addition of new test here is affecting it. I'll debug and try something to make it less flaky.

hi-ogawa · 2024-01-05T09:33:50Z

Actually I also need to be careful with automatic process.exit by webdriverio:

vitest/packages/browser/src/node/providers/webdriver.ts

Lines 93 to 95 in 3d18efd

    
           // TODO: right now process can only exit with timeout, if we use browser 
        
           // needs investigating 
        
           process.exit()

Test flaky etc... there are a few things to work out, so I'll put my PRs back to draft.

sheremet-va · 2024-01-09T14:42:48Z

Thanks!

fix(browser): fix updating snapshot during watch mode

728e020

hi-ogawa added 7 commits January 4, 2024 17:23

test: add manual test

2c992b9

test: test update snapshot

5044910

Merge branch 'main' into fix-browser-pool-watch-update-snapshot

4b31b97

test: tweak cleanup

9a0a09a

test: tweak reporter

22fc5ea

test: minor refactor

582df43

chore: lint

044c738

hi-ogawa marked this pull request as ready for review January 4, 2024 10:09

hi-ogawa commented Jan 4, 2024

View reviewed changes

test/browser/specs/update-snapshot.test.mjs Show resolved Hide resolved

sheremet-va reviewed Jan 4, 2024

View reviewed changes

hi-ogawa added 3 commits January 5, 2024 08:40

Merge branch 'main' into fix-browser-pool-watch-update-snapshot

9e0daa8

test: refine exitCode assertion

d2366e8

test: refactor by wrapWithExitCode util

ccfa85a

test: wrong defaultExclude

9af06bd

hi-ogawa mentioned this pull request Jan 5, 2024

test: split fixtures for browser test #4878

Closed

6 tasks

hi-ogawa marked this pull request as draft January 5, 2024 09:33

motss mentioned this pull request Jan 6, 2024

Inline Snapshot won't be written into the source code #2327

Closed

6 tasks

hi-ogawa added 4 commits January 9, 2024 09:00

Merge branch 'main' into fix-browser-pool-watch-update-snapshot

b7a96e7

test: change cacheDir

192954c

chore: redundant exclude

3403ea1

test: spy process.exit

c500288

hi-ogawa marked this pull request as ready for review January 9, 2024 00:45

sheremet-va approved these changes Jan 9, 2024

View reviewed changes

Merge branch 'main' into fix-browser-pool-watch-update-snapshot

d55ac5e

sheremet-va merged commit 508fced into vitest-dev:main Jan 9, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix(browser): fix updating snapshot during watch mode#4867

fix(browser): fix updating snapshot during watch mode#4867
sheremet-va merged 17 commits intovitest-dev:mainfrom
hi-ogawa:fix-browser-pool-watch-update-snapshot

hi-ogawa commented Jan 4, 2024 •

edited

Loading

Uh oh!

netlify bot commented Jan 4, 2024 •

edited

Loading

Uh oh!

Uh oh!

sheremet-va Jan 4, 2024 •

edited

Loading

Uh oh!

hi-ogawa Jan 4, 2024

Uh oh!

hi-ogawa Jan 5, 2024 •

edited

Loading

Uh oh!

hi-ogawa commented Jan 5, 2024

Uh oh!

hi-ogawa commented Jan 5, 2024

Uh oh!

sheremet-va commented Jan 9, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

	exitCode = process.exitCode
	process.exitCode = 0
	process.exit = exit

Uh oh!

Conversation

hi-ogawa commented Jan 4, 2024 • 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!

netlify bot commented Jan 4, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ Deploy Preview for fastidious-cascaron-4ded94 canceled.

Uh oh!

Uh oh!

sheremet-va Jan 4, 2024 • 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!

hi-ogawa Jan 4, 2024

Choose a reason for hiding this comment

Uh oh!

hi-ogawa Jan 5, 2024 • 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!

hi-ogawa commented Jan 5, 2024

Uh oh!

hi-ogawa commented Jan 5, 2024

Uh oh!

sheremet-va commented Jan 9, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

hi-ogawa commented Jan 4, 2024 •

edited

Loading

netlify bot commented Jan 4, 2024 •

edited

Loading

sheremet-va Jan 4, 2024 •

edited

Loading

hi-ogawa Jan 5, 2024 •

edited

Loading