[Feature]: Upstream jest-file-snapshot?

orta commented 2 years ago

🚀 Feature Proposal

https://www.npmjs.com/package/jest-file-snapshot

I tend to migrate my snapshots from inline to jest-file-snapshots in almost every project I write tests for (either once the snapshots become unreasonably sized for inline or there's too many).

Do others have feelings about whether this library might be worth thinking about migrating into Jest itself?

Motivation

It's not offensive for me to add:

import { toMatchFile } from 'jest-file-snapshot';

expect.extend({ toMatchFile });

Per each file to use the plugin/types, but it is friction. As is ensuring that the generated files do not trigger the watcher, if the matchers were upstreamed Jest can know to watchlist ignore them.

Example

N/A

Pitch

It's ~150 LOC, so probably not a big maintenance burden and does a good job of expanding the scope of how folks can use snapshots

/cc @satya164

satya164 commented 2 years ago

I'm happy to work on what's needed if we want to upstream it.

SimenB commented 2 years ago

I like it! @nicolo-ribaudo would this work for the stuff you do in Babel as well? (this stuff: https://github.com/babel/babel/blob/0b9fb13cb63e4aad375f2f54524871d3b6dd9314/packages/babel-parser/test/helpers/run-fixture-tests.js).

I ask because @orta's use case sounds like just toMatchSnapshot() without a huge benefit of separate files. But fixture tests are different

SimenB commented 2 years ago

@fisker might be something for prettier tests as well?

satya164 commented 2 years ago

speaking of Babel, I'd also like to mention https://github.com/satya164/babel-test where I used file snapshots, it was my main use case

fisker commented 2 years ago

Thanks for mentioning, looks like a good tool we can use. 👍

github-actions[bot] commented 2 years ago

This issue is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 30 days.

github-actions[bot] commented 2 years ago

This issue was closed because it has been stalled for 7 days with no activity. Please open a new issue if the issue is still relevant, linking to this one.

SimenB commented 2 years ago

Ooops, sorry. I think we wanna explore this, but I currently don't have the bandwidth

humanzz commented 1 year ago

I've been using https://github.com/igor-dv/jest-specific-snapshot - which sounds similar to jest-file-snapshot.

It'd be great if this type of specifying a snapshot file can be supported natively by jest.

The main use case for me is to be able to have multiple snapshot files, one for each test case, for a single test file

bradzacher commented 1 year ago

FWIW we've also been using jest-specific-snapshot in @typescript-eslint` for quite some time to do snapshots for our fixture tests.

Examples:

Scope analysis fixtures: https://github.com/typescript-eslint/typescript-eslint/tree/main/packages/scope-manager/tests/fixtures
AST Parser fixtures: https://github.com/typescript-eslint/typescript-eslint/tree/main/packages/ast-spec/src/declaration/ClassDeclaration/fixtures
More AST parser fixtures: https://github.com/typescript-eslint/typescript-eslint/tree/main/packages/typescript-estree/tests/snapshots

We have several thousand snapshots generated in this way!

It's been a really useful way for us to do our validation testing without building out custom infra that's almost the same as snapshot diffing.

SimenB commented 1 year ago

Oh, nice! Another use case for this is very nice 👍

I think I'm convinced this has enough utility to be in Jest core. Anybody up for sending a PR? 😀 I don't have any preference between jest-file-snapshot, jest-specific-snapshot or something else (like babel's (non-Jest, but) https://github.com/babel/babel/blob/156b608d48900b9489bd31aff03189a6c18f9040/packages/babel-parser/test/helpers/run-fixture-tests.js). But something which fits all of these use-cases would be sweet

bradzacher commented 1 year ago

The most complicated thing about implementing this properly in jest is keeping track of the random snapshots so you can assert that the untested ones are cleaned up properly.

Because jest-specific-snapshot allows you to pass any file path at all as the snapshot path - it simply cannot track the set of snapshots. Meaning you have to manually delete unnecessary snapshots.

If we can solve this concretely then it should be super simple to implement this feature natively!

seansfkelley commented 1 year ago

This ticket feels a lot like a duplicate of the broader request at #6383. I left some thoughts on what a potential in-core implementation of jest-file-snapshot could evolve to at https://github.com/jestjs/jest/issues/6383#issuecomment-1652106204, but the tl;dr is that I think there's a bit more work involved to make it actually mesh nicely with Jest core than just pulling the implementation into this repo. It's fundamentally awkward to use, because Jest doesn't expect it. (Pun intended.)

jestjs / jest