fix(browser): take failure screenshot if toMatchScreenshot can't capture a stable screenshot#9847
Conversation
| meta: { | ||
| assertion: 'toMatchScreenshot', | ||
| outcome: result.outcome, | ||
| }, |
There was a problem hiding this comment.
Not sure if this is the best way to accomplish this, but I didn't find other ways to check source/origin of an error and to attach additional data
There was a problem hiding this comment.
Looks good to me, but one question. Is assertion here and expectAssertionName available in JestExtendPlugin always same?
vitest/packages/expect/src/jest-extend.ts
Line 80 in fea46b3
We could always add assertion as error metadata in the plugin level, then move properties like JestExtendError.contenxt: { assertionName, meta }.
There was a problem hiding this comment.
Could definitely do that — I wasn't sure if it made sense to add it to all errors, but I guess it might be useful in the future 👍🏼
packages/expect/src/jest-extend.ts
Outdated
| setContext(assertionName: string, meta?: object): this { | ||
| this.context = { assertionName, meta } | ||
|
|
||
| return this | ||
| } |
There was a problem hiding this comment.
Should this be the 4th constructor parameter instead?
There was a problem hiding this comment.
I think so. I was wondering JestExtendError.constructor is somehow public, but probably not.
There was a problem hiding this comment.
Yep, it doesn't seem to be public. It's only used in this file
|
Is this testable? |
7510173 to
e272d74
Compare
✅ Deploy Preview for vitest-dev ready!Built without sensitive environment variables
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
|
||
| describe('failure screenshots', () => { | ||
| describe('`toMatchScreenshot`', () => { | ||
| test('usually does NOT produce a failure screenshot', async () => { |
There was a problem hiding this comment.
This should technically control all exit paths of the assertion, but I don't think it's worth the added time in each CI run. It might be easier to just test the cases that expect the opposite (like the one below) as the condition is quite straightforward.
Edit: since the line this comment is talking about is not clear in the UI, the subject is the test with the usually does NOT produce a failure screenshot description.
Description
Resolves #9690
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:.