-
-
Notifications
You must be signed in to change notification settings - Fork 32.7k
test_runner: add inlineSnapshot #59463
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
base: main
Are you sure you want to change the base?
Conversation
Review requested:
|
bd403c8
to
475eca6
Compare
added: REPLACEME | ||
--> | ||
|
||
> Stability: 1.0 - Early development |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These docs rock!
}); | ||
``` | ||
|
||
When first run without the `--test-update-snapshots` flag, the test will fail |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In my opinion, the ideal DX is to always assume a missing second parameter "wants to be updated" whether or not the --test-update-snapshots
flag is passed. You can then get a really nice micro workflow on a given test without ever needing to pass the flag just by repeatedly deleting the param (in my IDE, I often delete the parameter just by hitting undo: ctrl-z, save, test / update, ctrl-z, save, test / update, ... until I get the shape I want).
const user = { name: 'Alice', age: 30 }; | ||
|
||
// Initially call without second argument | ||
t.assert.inlineSnapshot(user); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great name! I think this aligns really well to the current TestContext.assert.snapshot() API. Also, I'm pleasantly surprised I don't need to await here.
test('inline snapshot test', (t) => { | ||
const user = { name: 'Alice', age: 30 }; | ||
|
||
t.assert.inlineSnapshot(user, ` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be really helpful if inlineSnapshot() supported wrapping. For example:
/**
* Asserts bitmap is the expected hex value. The `expectedHex` arg is replaced
* automatically when tests are run with `--test-update-snapshots`.
*/
function assertBitmapHex(ctx: TestContext, bmp: Uint8Array, expectedHex: string): string {
const actualHex = [...bmp]
.map(v => v.toString(16).padStart(2, '0'))
.join('')
ctx.assert.inlineSnapshot(actualHex, expectedHex) // <-- don't update me inline here!
}
test('', () => {
// ...
assertBitmapHex(bmp, '000000000000000000000000') // <-- I get updated inline here since I'm called at the top level test function.
})
It's possible to not use wrapping but then it gets pretty clumsy trying to fill out all the args in every test.
This is a draft implementation related to this issue: #59231.
I'm opening this PR mainly to collect some feedback regarding this feature and potential implementations, solutions, or ideas!
Note: The proposed solution is working, but it still needs refinement.