fix(browser): run toMatchScreenshot only once when used with expect.element
#9132
Conversation
| if (chai.util.flag(assertion, '_poll.assert_once')) { | ||
| clearTimeout(intervalId) | ||
| clearTimeout(timeoutId) | ||
|
|
||
| rejectWithCause(err) | ||
| } |
There was a problem hiding this comment.
Not entirely convinced by this solution, it adds yet-another exit point to a function that already has many...
✅ Deploy Preview for vitest-dev ready!Built without sensitive environment variables
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
Pushed a refactor of the poll assertion logic from the nested timer callbacks to a linear async/await loop. The control flow was quite hard to follow, there were multiple timeouts to track and exit points across different scopes. This also fixes a potential race condition where The behavior is essentially unchanged, the only difference is the last attempt now waits Happy to revert if there are concerns. |
macarie
force-pushed
the
fix/tomatchscreenshot-once-with-element
branch
from
9d8a543 to
39d89be
Compare
| const elapsed = performance.now() - start | ||
|
|
||
| // Allow some buffer for screenshot capture + diffing | ||
| expect(elapsed).toBeLessThan(renderDelay + 500) |
There was a problem hiding this comment.
Locally, 500ms are enough for all browsers to finish. Might be worth monitoring on CI to see if it's flaky.
There was a problem hiding this comment.
Looks like it's failing in CI. Maybe I can just check < 30_000 since that's poll's default timeout.
| errorMessage = error.message | ||
| } | ||
|
|
||
| expect(typeof errorMessage === 'string') |
There was a problem hiding this comment.
This doesn't do anything, did you mean expect.assert? We should maybe throw an error if the methods are not called 😄
There was a problem hiding this comment.
Editing fail on my part, it was an if before but forgot to change === with ).toBe( 😅
Good catch, thanks!
2 participants
Description
Fixes #8959
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:.