Skip to content

Conversation

@sheremet-va
Copy link

@sheremet-va sheremet-va commented Sep 26, 2025

This PR rewrites the tests for Vue Vapor mode to use Vitest Browser Mode. The logic of the test stayed the same, although it maybe also good to rewrite some of it or split it into separate tests. There are no big noticeable changes in speed.

Happy to get any feedback.

If there is desire, I can also update older e2e tests.

Running tests with --browser.headless=false also shows Vitest UI:

Screenshot 2025-09-26 at 17 29 01

@coderabbitai
Copy link

coderabbitai bot commented Sep 26, 2025

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

test('vapor', { timeout: E2E_TIMEOUT }, async () => {
createVaporApp(App).mount('#app')

expect(css('.main')).not.toBeVisible()
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Technically it's always better to use expect.element(locator).not.toBeVisible(), but prettier changes this 1 line into 3, so I kept more error prone version for esthetic reasons. I wonder if we can improve this on Vitest side.

.use(sirv(path.resolve(import.meta.dirname, '../dist')))
.listen(port)
process.on('SIGTERM', () => server && server.close())
test('vapor', { timeout: E2E_TIMEOUT }, async () => {
Copy link
Author

@sheremet-va sheremet-va Sep 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Putting timeout here makes prettier format the test body with indentation of 2 instead of 4, although maybe it's even better to move the timeout to the config.

},
{
extends: true,
extends: './packages-private/vapor-e2e-test/vite.config.ts',
Copy link
Author

@sheremet-va sheremet-va Sep 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The test config reuses the Vite config from a private repo instead of the main one (so, aliases and defines are not actually inherited, but tests seem to run ok...)

@github-actions
Copy link

Size Report

Bundles

File Size Gzip Brotli
compiler-dom.global.prod.js 84 kB 29.8 kB 26.3 kB
runtime-dom.global.prod.js 104 kB 39.1 kB 35.3 kB
vue.global.prod.js 162 kB 59.3 kB 52.9 kB

Usages

Name Size Gzip Brotli
createApp (CAPI only) 47.2 kB 18.4 kB 16.9 kB
createApp 56 kB 21.6 kB 19.7 kB
createApp + vaporInteropPlugin 68.6 kB 25.9 kB 23.6 kB
createVaporApp 21 kB 8.33 kB 7.62 kB
createSSRApp 60.3 kB 23.3 kB 21.3 kB
defineCustomElement 61 kB 23.1 kB 21.1 kB
overall 70.5 kB 26.8 kB 24.4 kB

@pkg-pr-new
Copy link

pkg-pr-new bot commented Sep 28, 2025

Open in StackBlitz

@vue/compiler-core

npm i https://pkg.pr.new/@vue/compiler-core@13934

@vue/compiler-dom

npm i https://pkg.pr.new/@vue/compiler-dom@13934

@vue/compiler-sfc

npm i https://pkg.pr.new/@vue/compiler-sfc@13934

@vue/compiler-ssr

npm i https://pkg.pr.new/@vue/compiler-ssr@13934

@vue/compiler-vapor

npm i https://pkg.pr.new/@vue/compiler-vapor@13934

@vue/reactivity

npm i https://pkg.pr.new/@vue/reactivity@13934

@vue/runtime-core

npm i https://pkg.pr.new/@vue/runtime-core@13934

@vue/runtime-dom

npm i https://pkg.pr.new/@vue/runtime-dom@13934

@vue/runtime-vapor

npm i https://pkg.pr.new/@vue/runtime-vapor@13934

@vue/server-renderer

npm i https://pkg.pr.new/@vue/server-renderer@13934

@vue/shared

npm i https://pkg.pr.new/@vue/shared@13934

vue

npm i https://pkg.pr.new/vue@13934

@vue/compat

npm i https://pkg.pr.new/@vue/compat@13934

commit: d9db524

@edison1105 edison1105 added version: minor scope: vapor related to vapor mode labels Sep 28, 2025
@edison1105
Copy link
Member

I've actually been planning to switch e2e tests to vitest browser mode too, but I haven't done it yet since vitest browser mode is still experimental. I have a question: after switching to vitest browser mode, does the e2e test execution time increase or decrease? If the difference isn't too significant, I think this PR is worthwhile.

@sheremet-va
Copy link
Author

have a question: after switching to vitest browser mode, does the e2e test execution time increase or decrease? If the difference isn't too significant, I think this PR is worthwhile.

The time is unpredictable with the previous approach for me, the first time it's ~2s, the second time ~1,5s. The browser mode runs for 2s (the MVC test is always 1,5s unlike with pupeteer where it's either 1,5s or 700ms). I think it might be because the previous setup was modifying the DOM directly (by setting input.value or doing a element.click() instead of using CDP which is slower). If we keep the previous implementation, I think the time will be the same, if not a little bit faster.

@edison1105
Copy link
Member

@sheremet-va
Sounds good to me. Let's move all e2e tests to vitest browser mode.

@sheremet-va
Copy link
Author

I will come back to this PR at some point. @edison1105 right now I am struggling with running unit tests in Vitest beta. It seems like BaseTransition.spec.ts is failing with the latest version (it's not the only test that fails) and I am not sure why. It seems like something was introduce in Vitest (probably related to spying) that is causing this. Can you help me debug it?

@edison1105
Copy link
Member

@sheremet-va
I'll take a look later

export function nextFrame() {
// this page is not same as Playwright's page
// how to wait for the next frame?
return page.evaluate(() => {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sheremet-va
I merged the latest code of minor into the current branch, but I encountered a problem and don't know how to solve it. this page is not same as Playwright's page. how to wait for the next frame? I couldn't find a solution in the vitest documentation.

)
// comp leave
await css(btnSelector).click()
expect(css(containerSelector).element().innerHTML).toBe(
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Accessing element() is also not a good idea, checking toContainHTML is more resilient

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's better to leave the remaining test case refactoring work to you. I'll go and handle other PRS first.

@sheremet-va
Copy link
Author

The main issue I am having with VItest 4 is that unit tests are failing. Did you have a loot at them yet, @edison1105?

@edison1105
Copy link
Member

edison1105 commented Oct 23, 2025

@sheremet-va

// in vitest 3.2.4
const hook = vi.fn((el, done) => {})
test('hook length', () => {
  expect(hook.length).toBe(2) // true
})

// in vitest 4.x
const hook = vi.fn((el, done) => {})
test('hook length', () => {
  expect(hook.length).toBe(2) // should be 2 but got 0
})

The above difference is the cause of the BaseTransition.spec.ts failed. Internally, Transition uses hook.length to determine whether the animation has ended. If hook.length <= 1, done will be called.

const callAsyncHook = (
hook: Hook<(el: any, done: () => void) => void>,
args: [TransitionElement, () => void],
) => {
const done = args[1]
callHook(hook, args)
if (isArray(hook)) {
if (hook.every(hook => hook.length <= 1)) done()
} else if (hook.length <= 1) {
done()
}
}


Another test case (componentPublicInstance.spec.ts:348:28) failed. Simply change the 4 to 3. Refer to the comments in L344. (It equals 3 in jest) - You already fixed it via 7e56f8d.

// vitest does not cache the spy like jest do
const v3 = instanceProxy.toggle()
expect(v3).toEqual('b')
expect(spy).toHaveBeenCalled()
expect(getCalledTimes).toEqual(4)

@sheremet-va
Copy link
Author

sheremet-va commented Oct 23, 2025

Another test case (componentPublicInstance.spec.ts:348:28) failed. Simply change the 4 to 3. Refer to the comments in L344. (It equals 3 in jest) - You already fixed it via 7e56f8d.

That one is an expected breaking change, yeah.

The above difference is the cause of the BaseTransition.spec.ts failed. Internally, Transition uses hook.length to determine whether the animation has ended. If hook.length <= 1, done will be called.

Thank you so much for investigating, this is indeed a bug in Vitest!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

scope: vapor related to vapor mode version: minor

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants