Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Draft
wants to merge 16 commits into
base: main
Choose a base branch
from
15 changes: 15 additions & 0 deletions packages/browser/src/client/orchestrator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -273,6 +273,21 @@ async function setIframeViewport(
if (ui) {
await ui.setIframeViewport(width, height)
}
else if (getBrowserState().provider === 'playwright') {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I couldn't figure out what's wrong with webdriverio, so I added scaling only for playwright and CI seems mostly happy.

const scale = Math.min(
1,
iframe.parentElement!.parentElement!.clientWidth / width,
iframe.parentElement!.parentElement!.clientHeight / height,
)
iframe.parentElement!.style.cssText = `
width: ${width}px;
height: ${height}px;
transform: scale(${scale});
transform-origin: left top;
`
iframe.parentElement?.setAttribute('data-scale', String(scale))
await new Promise(r => requestAnimationFrame(r))
}
else {
iframe.style.width = `${width}px`
iframe.style.height = `${height}px`
Expand Down
15 changes: 7 additions & 8 deletions packages/browser/src/client/tester/context.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ import { convertElementToCssSelector, getBrowserState, getWorkerState } from '..

// this file should not import anything directly, only types and utils

const state = () => getWorkerState()
// @ts-expect-error not typed global
const provider = __vitest_browser_runner__.provider
function filepath() {
Expand Down Expand Up @@ -222,8 +221,7 @@ function getTaskFullName(task: RunnerTask): string {
}

function processClickOptions(options_?: UserEventClickOptions) {
// only ui scales the iframe, so we need to adjust the position
if (!options_ || !state().config.browser.ui) {
if (!options_) {
return options_
}
if (provider === 'playwright') {
Expand All @@ -250,8 +248,7 @@ function processClickOptions(options_?: UserEventClickOptions) {
}

function processHoverOptions(options_?: UserEventHoverOptions) {
// only ui scales the iframe, so we need to adjust the position
if (!options_ || !state().config.browser.ui) {
if (!options_) {
return options_
}

Expand All @@ -277,8 +274,7 @@ function processHoverOptions(options_?: UserEventHoverOptions) {
}

function processDragAndDropOptions(options_?: UserEventDragAndDropOptions) {
// only ui scales the iframe, so we need to adjust the position
if (!options_ || !state().config.browser.ui) {
if (!options_) {
return options_
}
if (provider === 'playwright') {
Expand Down Expand Up @@ -336,11 +332,14 @@ function processPlaywrightPosition(position: { x: number; y: number }) {
}

function getIframeScale() {
const testerUi = window.parent.document.querySelector('#tester-ui') as HTMLElement | null
const testerUi = window.parent.document.querySelector(`iframe[data-vitest]`)?.parentElement
if (!testerUi) {
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) {
return 1
}
const scale = Number(scaleAttribute)
if (Number.isNaN(scale)) {
throw new TypeError(`Cannot parse scale value from Tester element (${scaleAttribute}). This is a bug in Vitest. Please, open a new issue with reproduction.`)
Expand Down
53 changes: 53 additions & 0 deletions test/browser/fixtures/viewport/basic.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
import { page, userEvent } from "@vitest/browser/context";
import { expect, test } from "vitest";

test("drag and drop over large viewport", async () => {
// put boxes horizontally [1] [2] ... [30]
// then drag-and-drop from [1] to [30]

const wrapper = document.createElement("div");
wrapper.style.cssText = "display: flex; width: 3000px;";
document.body.appendChild(wrapper);

const events: { i: number; type: string }[] = [];

for (let i = 1; i <= 30; i++) {
const el = document.createElement("div");
el.textContent = `[${i}]`;
el.style.cssText = `
flex: none;
width: 100px;
height: 100px;
border: 1px solid black;
box-sizing: border-box;
display: flex;
justify-content: center;
align-items: center;
`;
el.draggable = true;
wrapper.append(el);

el.addEventListener("dragstart", (ev) => {
ev.dataTransfer.effectAllowed = "move";
events.push({ type: "dragstart", i });
});
el.addEventListener("dragover", (ev) => {
ev.preventDefault();
ev.dataTransfer.dropEffect = "move";
events.push({ type: "dragover", i });
});
el.addEventListener("drop", (ev) => {
ev.preventDefault();
events.push({ type: "drop", i });
});
}

await userEvent.dragAndDrop(page.getByText("[1]"), page.getByText("[30]"));

expect(events).toMatchObject(
expect.arrayContaining([
{ type: "dragstart", i: 1 },
{ type: "drop", i: 30 },
]),
);
});
21 changes: 21 additions & 0 deletions test/browser/fixtures/viewport/vitest.config.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
import { fileURLToPath } from 'node:url'
import { defineConfig } from 'vitest/config'

// pnpm -C test/browser test-fixtures --root fixtures/viewport --browser.ui=false
// pnpm -C test/browser test-fixtures --root fixtures/viewport --browser.headless=true

const provider = process.env.PROVIDER || 'playwright'
const name =
process.env.BROWSER || (provider === 'playwright' ? 'chromium' : 'chrome')

export default defineConfig({
test: {
browser: {
enabled: true,
name,
provider,
viewport: { width: 3000, height: 400 }
},
},
cacheDir: fileURLToPath(new URL("./node_modules/.vite", import.meta.url)),
})
2 changes: 1 addition & 1 deletion test/browser/specs/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
import type { UserConfig } from 'vitest'
import { runVitest } from '../../test-utils'

const provider = process.env.PROVIDER || 'playwright'
export const provider = process.env.PROVIDER || 'playwright'
export const browser = process.env.BROWSER || (provider !== 'playwright' ? 'chromium' : 'chrome')

export async function runBrowserTests(
Expand All @@ -21,7 +21,7 @@
} as UserConfig['browser'],
}, include, 'test', viteOverrides)

const browserResult = await readFile('./browser.json', 'utf-8')

Check failure on line 24 in test/browser/specs/utils.ts

View workflow job for this annotation

GitHub Actions / Browser: chromium, macos-latest

specs/server-url.test.ts > server-url http

Error: ENOENT: no such file or directory, open './browser.json' ❯ Module.runBrowserTests specs/utils.ts:24:25 ❯ specs/server-url.test.ts:9:40 ⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯ Serialized Error: { errno: -2, code: 'ENOENT', syscall: 'open', path: './browser.json' }

Check failure on line 24 in test/browser/specs/utils.ts

View workflow job for this annotation

GitHub Actions / Browser: chromium, macos-latest

specs/server-url.test.ts > server-url https

Error: ENOENT: no such file or directory, open './browser.json' ❯ Module.runBrowserTests specs/utils.ts:24:25 ❯ specs/server-url.test.ts:18:40 ⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯ Serialized Error: { errno: -2, code: 'ENOENT', syscall: 'open', path: './browser.json' }

Check failure on line 24 in test/browser/specs/utils.ts

View workflow job for this annotation

GitHub Actions / Browser: chromium, macos-latest

specs/update-snapshot.test.ts > update snapshot

Error: ENOENT: no such file or directory, open './browser.json' ❯ Module.runBrowserTests specs/utils.ts:24:25 ❯ specs/update-snapshot.test.ts:15:15 ⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯ Serialized Error: { errno: -2, code: 'ENOENT', syscall: 'open', path: './browser.json' }
const browserResultJson = JSON.parse(browserResult)

const getPassed = results => results.filter(result => result.status === 'passed' && !result.mesage)
Expand Down
12 changes: 12 additions & 0 deletions test/browser/specs/viewport.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
import { expect, test } from 'vitest'
import { provider, runBrowserTests } from './utils'

test.runIf(provider === 'playwright')('viewport', async () => {
const { stderr, stdout } = await runBrowserTests({
root: './fixtures/viewport',
reporters: [['verbose', { isTTY: false }]],
})

expect(stderr).toBe('')
expect(stdout).toContain('✓ basic.test.ts')
})
5 changes: 3 additions & 2 deletions test/browser/test/userEvent.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -135,9 +135,10 @@ describe('userEvent.click', () => {
},
})

// not exact due to scaling and rounding
expect(spy).toHaveBeenCalledWith({
x: 200,
y: 150,
x: expect.closeTo(200, -1),
y: expect.closeTo(150, -1),
Comment on lines +138 to +141
Copy link
Contributor Author

@hi-ogawa hi-ogawa Sep 18, 2024

Choose a reason for hiding this comment

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

Scaling round issue seems to already exist for UI case. For example, I can reproduce it on main branch with firefox.

$ PROVIDER=playwright BROWSER=firefox pnpm -C test/browser test-fixtures /userEvent. -t 'x/y'

 FAIL  test/userEvent.test.ts > userEvent.click > clicks with x/y coords
AssertionError: expected "spy" to be called with arguments: [ { x: 200, y: 150 } ]

Received: 

  1st spy call:

  Array [
    Object {
-     "x": 200,
+     "x": 199,
      "y": 150,
    },
  ]

I'm not sure if there's a way to fix this, so I loosened the assertion.

})
})
})
Expand Down
Loading