Skip to content

Comments

fix(browser): scale iframe for non ui case#6512

Merged
sheremet-va merged 46 commits intovitest-dev:mainfrom
hi-ogawa:fix-browser-non-ui-viewport
Apr 17, 2025
Merged

fix(browser): scale iframe for non ui case#6512
sheremet-va merged 46 commits intovitest-dev:mainfrom
hi-ogawa:fix-browser-non-ui-viewport

Conversation

@hi-ogawa
Copy link
Contributor

@hi-ogawa hi-ogawa commented Sep 15, 2024 •
edited
Loading

Description

The test case I added is failing on webdriverio regardless of with or without UI. I couldn't figure out webdriverio, so this PR only fixes playwright where it has inconsistency with or without UI.

todo

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

@hi-ogawa hi-ogawa changed the title fix(browser): fix viewport for non ui case fix(browser): fix viewport for non ui case on playwright provider Sep 15, 2024
@hi-ogawa hi-ogawa marked this pull request as ready for review September 15, 2024 10:07
sheremet-va
sheremet-va requested changes Sep 15, 2024
View reviewed changes
Copy link
Member

@sheremet-va sheremet-va left a comment

Choose a reason for hiding this comment

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

This doesn't fix it when you call page.viewport

We already experimented with this before. Now we just need to keep the same iframe scaling that we have in ui

@hi-ogawa
Copy link
Contributor Author

hi-ogawa commented Sep 16, 2024

This doesn't fix it when you call page.viewport

Ah, I didn't think about that. I've read #6257 and scaling seems more involved but I'll see what I can do.

@hi-ogawa hi-ogawa marked this pull request as draft September 17, 2024 06:56
@netlify
Copy link

netlify bot commented Sep 18, 2024 •
edited
Loading

✅ Deploy Preview for vitest-dev ready!

Built without sensitive environment variables

Name Link
🔨 Latest commit a82ae78
🔍 Latest deploy log https://app.netlify.com/sites/vitest-dev/deploys/67107c43b2c76d000827d8c0
😎 Deploy Preview https://deploy-preview-6512--vitest-dev.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@hi-ogawa hi-ogawa changed the title fix(browser): fix viewport for non ui case on playwright provider fix(browser): scale iframe for non ui case Sep 18, 2024
hi-ogawa
hi-ogawa commented Sep 18, 2024
View reviewed changes
Copy link
Contributor Author

@hi-ogawa hi-ogawa left a comment

Choose a reason for hiding this comment

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

Only Mac chromium playwright is timing out with specs/runner.test.ts https://github.com/vitest-dev/vitest/actions/runs/10915841527/job/30296283296?pr=6512#step:9:24

I cannot reproduce it locally on my Linux chromium headless, so it's hard to diagnose what's wrong. I would appreciate if someone can check it on Mac machine locally.

@nstepien
Copy link
Contributor

nstepien commented Jan 21, 2025

Is this still blocked?

@hi-ogawa hi-ogawa mentioned this pull request Feb 11, 2025
Closed
6 tasks
@hi-ogawa
Copy link
Contributor Author

hi-ogawa commented Feb 15, 2025

I updated a branch but Mac seems still flaky. The last run passed but 2nd last run seemed stuck in timeout.

@hi-ogawa hi-ogawa mentioned this pull request Apr 3, 2025
Closed
6 tasks
@hi-ogawa hi-ogawa marked this pull request as ready for review April 3, 2025 04:50
@hi-ogawa
Copy link
Contributor Author

hi-ogawa commented Apr 3, 2025

Hopefully flakiness is finally gone...

sheremet-va
sheremet-va requested changes Apr 3, 2025
View reviewed changes
packages/browser/src/client/tester/utils.ts Outdated
throw new Error(`Cannot find Tester element. This is a bug in Vitest. Please, open a new issue with reproduction.`)
}
const scaleAttribute = testerUi.getAttribute('data-scale')
if (scaleAttribute === null) {
Copy link
Member

@sheremet-va sheremet-va Apr 3, 2025

Choose a reason for hiding this comment

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

We should always add a data-scale attribute and throw an error if it's incorrect. The isNaN check after that checks exactly that scenario

@sheremet-va sheremet-va merged commit c337480 into vitest-dev:main Apr 17, 2025
13 of 14 checks passed
@hi-ogawa hi-ogawa deleted the fix-browser-non-ui-viewport branch April 17, 2025 23:43
schogges pushed a commit to kumahq/kuma-gui that referenced this pull request Apr 23, 2025
@dependabot
chore(deps-dev): Bump vitest from 3.1.1 to 3.1.2 (#3781)
ecaab5d 
v3.1.2
   🐞 Bug Fixes
Add global chai variable in vitest/globals (fix: #7474)  -  by @​Jay-Karia in vitest-dev/vitest#7771 and vitest-dev/vitest#7474 (d9297)
Prevent modifying test.exclude when same object passed in coverage.exclude  -  by @​AriPerkkio in vitest-dev/vitest#7774 (c3751)
Fix already hoisted mock  -  by @​hi-ogawa in vitest-dev/vitest#7815 (773b1)
Fix test.scoped inheritance  -  by @​hi-ogawa in vitest-dev/vitest#7814 (db6c3)
Remove pointer-events-none after resizing the left panel  -  by @​alexprudhomme in vitest-dev/vitest#7811 (a7e77)
Default to run mode when stdin is not a TTY  -  by @​kentonv, @​hi-ogawa and @​sheremet-va in vitest-dev/vitest#7673 (6358f)
Use happy-dom/jsdom types for envionmentOptions  -  by @​hi-ogawa in vitest-dev/vitest#7795 (67430)
browser:
Fix transform error before browser server initialization  -  by @​hi-ogawa in vitest-dev/vitest#7783 (5f762)
Fix mocking from outside of root  -  by @​hi-ogawa in vitest-dev/vitest#7789 (03f55)
Scale iframe for non ui case  -  by @​hi-ogawa in vitest-dev/vitest#6512 (c3374)
coverage:
await profiler calls  -  by @​AriPerkkio in vitest-dev/vitest#7763 (795a6)
Expose profiling timers  -  by @​AriPerkkio in vitest-dev/vitest#7820 (5652b)
deps:
Update all non-major dependencies  -  in vitest-dev/vitest#7765 (7c3df)
Update all non-major dependencies  -  in vitest-dev/vitest#7831 (15701)
runner:
Correctly call test hooks and teardown functions  -  by @​sheremet-va in vitest-dev/vitest#7775 (3c00c)
Show stacktrace on test timeout error  -  by @​hi-ogawa in vitest-dev/vitest#7799 (df33b)
ui:
Load panel sizes from storage on initial load  -  by @​userquin in vitest-dev/vitest#7265 (6555d)
vite-node:
Named export should overwrite export all  -  by @​hi-ogawa in vitest-dev/vitest#7846 (5ba0d)
Add ERR_MODULE_NOT_FOUND code error if module cannot be loaded  -  by @​sheremet-va in vitest-dev/vitest#7776 (f9eac)
   🏎 Performance
browser: Improve browser parallelisation  -  by @​sheremet-va in vitest-dev/vitest#7665 (816a5)
@sheremet-va sheremet-va mentioned this pull request Apr 24, 2025
Open
6 tasks
@github-actions github-actions bot mentioned this pull request Apr 28, 2025
Merged
1 task
This was referenced May 15, 2025
Merged
Open
@unional unional mentioned this pull request Jun 1, 2025
Closed
@renovate renovate bot mentioned this pull request May 31, 2025
Merged
1 task
@renovate renovate bot mentioned this pull request Jun 17, 2025
Open
1 task
@renovate renovate bot mentioned this pull request Jun 29, 2025
Closed
1 task
@sheremet-va sheremet-va mentioned this pull request Oct 16, 2025
Open
6 tasks
@KaelWD KaelWD mentioned this pull request Nov 28, 2025
Open
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@sheremet-va sheremet-va sheremet-va approved these changes

Assignees

No one assigned

Labels

cr-tracked

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

bug: fixed positioning cannot be used in vitest browser headless mode Browser viewport does not match iframe viewport when the UI is disabled

3 participants

@hi-ogawa @nstepien @sheremet-va