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

Tip

📝 Customizable high-level summaries are now available in beta!

You can now customize how CodeRabbit generates the high-level summary in your pull requests — including its content, structure, tone, and formatting.

  • Provide your own instructions using the high_level_summary_instructions setting.
  • Format the summary however you like (bullet lists, tables, multi-section layouts, contributor stats, etc.).
  • Use high_level_summary_in_walkthrough to move the summary from the description to the walkthrough section.

Example instruction:

"Divide the high-level summary into five sections:

  1. 📝 Description — Summarize the main change in 50–60 words, explaining what was done.
  2. 📓 References — List relevant issues, discussions, documentation, or related PRs.
  3. 📦 Dependencies & Requirements — Mention any new/updated dependencies, environment variable changes, or configuration updates.
  4. 📊 Contributor Summary — Include a Markdown table showing contributions:
    | Contributor | Lines Added | Lines Removed | Files Changed |
  5. ✔️ Additional Notes — Add any extra reviewer context.
    Keep each section concise (under 200 words) and use bullet or numbered lists for clarity."

Note: This feature is currently in beta for Pro-tier users, and pricing will be announced later.


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

github-actions bot commented Sep 28, 2025

Size Report

Bundles

File Size Gzip Brotli
compiler-dom.global.prod.js 85.2 kB 29.9 kB 26.4 kB
runtime-dom.global.prod.js 108 kB 40.5 kB 36.4 kB
vue.global.prod.js 166 kB 60.5 kB 53.8 kB

Usages

Name Size Gzip Brotli
createApp (CAPI only) 48 kB 18.8 kB 17.1 kB
createApp 57 kB 21.9 kB 20 kB
createApp + vaporInteropPlugin 88.5 kB 33.2 kB 30 kB
createVaporApp 31.7 kB 12.4 kB 11.3 kB
createSSRApp 61.4 kB 23.7 kB 21.6 kB
defineCustomElement 63.1 kB 23.8 kB 21.7 kB
overall 72.1 kB 27.3 kB 24.9 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: 8da4595

@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