Conversation
✅ Deploy Preview for vitest-dev ready!Built without sensitive environment variables
To edit notification comments on pull requests, go to your Netlify project configuration. |
22cdbd6 to
b1916ae
Compare
|
Any news here? 👀 |
|
From my testing seems to be working fairly well with Playwright, but I've had no time to implement the WebdriverIO version yet. Hopefully this week 🤞🏼 |
77883ff to
6e15663
Compare
4182a00 to
5ea6c4c
Compare
| /> | ||
| <span class="pointer-events-none" text-sm> | ||
| {{ viewport[0] }}x{{ viewport[1] }}px | ||
| <span v-if="scale < 1">({{ (scale * 100).toFixed(0) }}%)</span> |
There was a problem hiding this comment.
I think I can add this back now if necessary.
There was a problem hiding this comment.
Why would we remove it? The window is still scaled
There was a problem hiding this comment.
I removed it initially when I wasn't sure about what approach to go with. I'll add it back 👍🏼
|
I moved the scaling part to be CSS-only, which allowed removing some JS. I think resizing the browser window and the panels now feels more fluid and less janky, especially on old(er) hardware. When taking screenshots a bit of CSS is injected to remove the scaling from the iframe and making it escape the pane. This should fix the "low resolution" issues, removing the scaling also leads to not having to deal with different sub pixel sizes and GPU rendering differences. In headless mode (with no UI), Playwright now uses the set viewport, same as WebdriverIO, this should allow capturing screenshots larger than the default viewport size. In UI mode this is not done to prevent having flashing windows (more than we already have), so it's up to the user to make sure the rendered elements fit in the window. |
| iframe { | ||
| position: absolute !important; | ||
| inset: 0 !important; | ||
| z-index: ${Number.MAX_SAFE_INTEGER} !important; | ||
| transform: none !important; | ||
| } |
There was a problem hiding this comment.
We might have to opacity: 0 everything else. When the iframe doesn't have a background, the UI peeks through.
There was a problem hiding this comment.
This can be fixed by setting a background color. I'm not sure if it should change with the theme or if it should stay white tho 🤔
| } | ||
|
|
||
| const SCREENSHOT_STYLES = /* css */` | ||
| iframe { |
There was a problem hiding this comment.
Shouldn't this be [data-vitest=true]?
There was a problem hiding this comment.
Yes, should be more specific. Do you prefer data-vitest or its name (or both)?
There was a problem hiding this comment.
I think we use data-vitest everywhere in locators), but I’m not sure
There was a problem hiding this comment.
Used data-vitest, looks like it's the one that's used in some other places too.
| /> | ||
| <span class="pointer-events-none" text-sm> | ||
| {{ viewport[0] }}x{{ viewport[1] }}px | ||
| <span v-if="scale < 1">({{ (scale * 100).toFixed(0) }}%)</span> |
There was a problem hiding this comment.
Why would we remove it? The window is still scaled
| enabled: true, | ||
| screenshotFailures: false, | ||
| provider: playwright(), | ||
| provider: ${provider.name}(), |
There was a problem hiding this comment.
runInlineTests can inject these automatically if you just pass down an object instead of a file content string (so passing down provider in the test will correctly specify it in the config)
| } | ||
|
|
||
| // we can safely use `a` as both vertical and horizontal scale are the same | ||
| const scale = new DOMMatrix(getComputedStyle(iframe).transform).a |
There was a problem hiding this comment.
Can you add an explanation (as a comment) of what is going on here? I've never seen this usage before
There was a problem hiding this comment.
I've updated the comment, does this version explain it better? 🤔
|
|
||
| expect(vitest.stdout).toContain('✓ |chromium| basic.test.ts > screenshot-snapshot') | ||
| for (const instance of instances) { | ||
| expect(`✓ |${instance.browser}| basic.test.ts > screenshot-snapshot`) |
There was a problem hiding this comment.
I wonder if we should throw an error if expect was called without a matcher
There was a problem hiding this comment.
Fixed it, messed up with the multi-cursor (again) 😅
There was a problem hiding this comment.
I wonder if we should throw an error if expect was called without a matcher
That would be nice, or maybe a lint issue 👀
Description
Highly WIP and needs a lot of testing, but so far seems to work with Playwright with
chromium,firefox, andwebkit.Have to look into WebdriverIO (🫣)
Fixes #9124
Fixes #9363
Please don't delete this checklist! Before submitting the PR, please make sure you do the following:
pnpm-lock.yamlunless you introduce a new test example.Tests
pnpm test:ci.Documentation
pnpm run docscommand.Changesets
feat:,fix:,perf:,docs:, orchore:.