Use inline snapshots #117
Use inline snapshots #117
Conversation
__tests__/runtime.ts
Outdated
| const development = transform({code, options, env: 'development'}) | ||
| const production = transform({code, options, env: 'production'}) | ||
| const esm = transform({code, options, env: 'esm'}) | ||
| const cjs = transform({code, options, env: 'cjs'}) |
There was a problem hiding this comment.
Instead of defining variables for transforms, let's define a single variable for the snapshot being compared against. This would help structure the expect assertions below in the same way as they are being used in some of the other test files, by keeping the transform params close to the assertion.
There was a problem hiding this comment.
Alternatively, we could defined a meta-assertion that can accept a list of transforms and compare them against a single snapshot. This way we could avoid thinking about variables entirely
There was a problem hiding this comment.
Instead of defining variables for transforms, let's define a single variable for the snapshot being compared against.
Could you show a snippet of what you mean?
There was a problem hiding this comment.
I was thinking along the lines of
[
transform({code, options, env: 'development'}),
transform({code, options, env: 'production'}),
transform({code, options, env: 'esm'})
].forEach(snapshot => {
expect(snapshot).toBe(`
function _objectWithoutPropertiesLoose(source, excluded) {
if (source == null) return {}
var target = {}
var sourceKeys = Object.keys(source)
var key, i
for (i = 0; i < sourceKeys.length; i++) {
key = sourceKeys[i]
if (excluded.indexOf(key) >= 0) continue
target[key] = source[key]
}
return target
}
var _obj = obj,
foo = _obj.foo,
rest = _objectWithoutPropertiesLoose(_obj, ['foo'])
`)
})
when we assert that multiple transforms map to a single snapshot. This avoids having to trace which snapshot variable is the source of truth, and their relations
There was a problem hiding this comment.
I think maybe it wasn't clear in the PR body that the idea of inline snapshots is that you don't write the snapshot, Jest does. Unlike regular snapshots, it writes them inline in the assertion call instead of into a separate file.
I don't think it makes sense to consider solutions that can't auto-generate and update the output. (Each inline snapshot assertion can only be called once, by design.)
There was a problem hiding this comment.
Gotcha, wasn't aware of the limitations of inline snapshots. Is it still possible to compare the transform outputs against some source of truth without storing them as variables? Eg expect(transform(...)).toBe(development)
There was a problem hiding this comment.
I think I'm a bit confused by the line of questioning in this thread. Is development something like const development = transform(...)? If so, then you can definitely make that assertion because transform is a regular function that returns a string, so you're comparing two strings together. But then you've stored the transform output as a variable, which it seems like you're trying to avoid, although I don't necessarily understand why. Did you mean to write .toMatchInlineSnapshot? If so, then I don't understand what development would be.
Maybe it would be helpful to back up; could you explain broadly what you dislike about the current code, without getting into potential solutions?
That said, it seems like the consensus is otherwise to go with the #118 approach so maybe this is moot.
I like this. I think formatting with Prettier makes a developer more likely to actually read and comprehend a failed snapshot rather than just regenerating it.
I think I'm missing something. For this example wouldn't it be fine as is since |
Yeah sorry, I didn't explain that clearly. Consider something like this: const code = `
foo({a, b})
`
expect(transform({code})).toBe(code)There are two issues that will cause this assertion to fail even if the AST is identical:
I see two solutions. We could parse the input/output ASTs and compare those rather than code strings. Or, we could run the input string through Babel without the preset, then through Prettier, to get an input string that is more likely to have the same whitespace as the output. (The latter is what I meant in the original note.) Not sure whether this type of comparison is valuable enough to warrant the complexity of either option. |
|
Gotcha, I see what you mean now. I've never needed to do that type of assertion before so agreed that probably not worth adding the complexity until we run into a case that does require it. I would lean towards the latter solution though if/when we do. |
|
I'm partial to the format of #118 since it feels like less boilerplate to sift through to see the end result. In general though I prefer the inline snapshots since it's easier to see the context of the test coupled with the result. |
|
Seems like the preference is for #118 so I'm going to merge that PR into this branch, and then finish migrating the existing tests when I find the time. Thanks for the thoughts, everyone! |
* Group inline snapshot transform results by env * Prefix env list with BABEL_ENV
Closes #116.
WIP as it doesn't yet cover everything that the current test suite does.
Some notes:
expect(output).toBe(input)rather than taking a snapshot when we explicitly want Babel not to touch something. To do this, we'd have to format the input string with Prettier as well, so this would need to live intransformrather than the serializer.Putting this up for a review of the approach before I replicate the rest of the existing tests.