Export SnapshotFile constructor (jest-snapshot)

[suchipi]

I'm working on a chai matcher for jest-snapshot. I pull in SnapshotFile from jest-snapshot/build/SnapshotFile and use it to create snapshots, check for matches, etc. It's pretty easy to use. However, the only publicly exposed way to create a SnapshotFile instance is via the forFile method. Using this method creates a SnapshotFile whose path points to ./__snapshots__/<filename>.snap. While this makes sense for jest, it's less flexible when trying to accommodate other test frameworks.

In order to work around this, currently my code does this:

import snapshotFileExport from "jest-snapshot/build/SnapshotFile";
const SnapshotFile = snapshotFileExport.forFile(__filename).constructor;
// and then later...
let snapshotFile = new SnapshotFile(snapshotFileName);

It'd be nice if the SnapshotFile constructor was exposed "officially", or maybe as a new method in SnapshotFile.js's exports (eg. withPath).

[cpojer]

Hey!

This is exciting. Would you mind sending a PR with the API updates you need? I think here is what we should do: expose SnapshotFile through jest-snapshot/src/index.js: you should be able to do require('jest-snapshot').SnapshotFile and never reach into jest-snapshot/build/…

We might want to make the snapshot cleanup function more generic too, as you'll probably need that to keep consistency. We might also want to move the reporter into jest-snasphot from jest-cli, but let's do this step-by-step :)

[suchipi]

I'll try to submit a PR sometime this week. For reference, here's the package I created: https://www.npmjs.com/package/chai-jest-snapshot

Some observations:

• You have to specify the snapshot file and snapshot name manually with my assertion helper, because chai is test framework-agnostic. I want to eventually create a babel plugin that will automatically fill in these arguments for mocha (and maybe other test frameworks, too).
• Because it's test framework-agnostic, I can't delete emptied snapshots, because I don't know about them. Not sure how to best handle this (right now I just don't offer this feature).
• Because it's framework-agnostic, you can only update one snapshot at a time (as opposed to jest -u which does the whole suite). I actually kind of prefer this, though.
• I had to duplicate a lot of the logic that was in the matcher, because it's written in a way that is dependent on jest and how you patched jasmine. It wasn't very difficult to do, though, because all the actual work is encapsulated in SnapshotFile. But I may have implemented slightly different functionality than what is present in jest as a result (I didn't see any tests around this behavior, and it doesn't seem like they would be very easy to write with how tightly snapshotting is integrated with your framework).
• Your matcher disallows using .not, but I didn't see a reason to disallow that in my assertion helper. I can't think of a reason someone would want to use it, but I didn't go out of my way to disallow it, either.

I've written tests documenting the expected behavior of the assertion helper in regards to when to create files, etc. They're available here. If you'd like, I'd appreciate a quick look to make sure I'm not doing anything drastically different from how jest uses jest-snapshot.

[cpojer]

This looks like a good start but I want to stress the importance of a good user experience which is why we spent so much time trying to get snapshot testing in Jest exactly right.

• I agree we should export the SnapshotFile constructor in Jest.
• Can you think of a way to make the matcher more general and not tied to patching jasmine so we can share as much code as possible?
• I recommend throwing if the update flag is passed and the snapshot matches. Otherwise people might keep the update flag in there, always overwriting the snapshot. It is probably going to be pretty annoying if you have 100 snapshots that all change if you modify a component that all snapshots rely on.
• Have you thought about CLI output or further test runner integration? Jest writes a nice summary every time a snapshot is modified, outdated or a snapshot or test file has been removed that has an associated test or snapshot file. This is to make sure the user is informed but also makes sure that the repo is always in a clean state.
• We didn't think .not is useful. How would it provide any value? Which snapshot would you test against?

We are hoping to further improve the UX and I feel like we are just at the beginning with Jest snapshot testing – if you'd ever like to use Jest itself and help us improve our snapshot feature, please let use know :)

[suchipi]

Can you think of a way to make the matcher more general and not tied to patching jasmine so we can share as much code as possible?

If jest-snapshot had something like processSnapshot(snapshotFilename, snapshotName, shouldUpdate) that returned something shaped similarly to a SnapshotState but with matches and pass/fail status in it too, then that could be exported, and the matcher could delegate most of it. That way, the "rules" about when snapshots are overwritten, changed, etc could be consistent across consumers. The hard part is knowing how much to return from processSnapshot.

It's worth noting that chai provides string diffing directly, so for chai, it is more helpful to get back the expected and actual than to receive a pre-diffed string. For other frameworks, though, it might be nice to have both the expected and actual as well as the diffed string, so that they don't need to replicate that behavior.

I recommend throwing if the update flag is passed and the snapshot matches. Otherwise people might keep the update flag in there, always overwriting the snapshot. It is probably going to be pretty annoying if you have 100 snapshots that all change if you modify a component that all snapshots rely on.

This is good feedback. I think it makes sense to throw. Is this the current behavior in Jest?

I hadn't thought about how annoying it would be to add 100 temporary trues just to fix your known changes. I can definitely see how that would be a bad user experience. Chai is just an assertion library, so it can't receive flags passed to its test framework directly, but maybe I could use an environment variable? Something like UPDATE_CHAI_JEST_SNAPSHOTS=true mocha **/*.spec.js.

Have you thought about CLI output or further test runner integration? Jest writes a nice summary every time a snapshot is modified, outdated or a snapshot or test file has been removed that has an associated test or snapshot file. This is to make sure the user is informed but also makes sure that the repo is always in a clean state.

I want to look at deeper test framework integration next; there's not much reported to the user right now about what decisions are made regarding their snapshots, and short of console-logging, chai doesn't have a framework-agnostic way to show summary data like that. I'd like to look at mocha first, since a lot of people use mocha/chai/sinon/enzyme for React component testing.

If jest-snapshot had an interface something like startTrackingSnapshotSummary(), finishTrackingSnapshotSummary(), getSnapshotSummary() => { modified: [{ fileName, snapshotName }...], outdated: [...], removed: [...] }, then I think that would be pretty easy to hook into most test frameworks; most have something like beforeAll and afterAll.

We didn't think .not is useful. How would it provide any value? Which snapshot would you test against?

With the chai assertion helper, you have to specify the snapshot name (aka key) that you are comparing against, because that information isn't inferred from the framework (though I want to write a babel plugin for mocha to provide that eventually). So, it would test against whichever one you specified... but the semantics for what it means are confusing, as I think through it. If you expect it not to match snapshot x, but snapshot x is not present, does it create x so that it can "not match" it? If not, does the test pass?

I left it in because I was trying to keep the package unopinionated and flexible, but because the outcome of running a snapshot test is not truly binary (pass/fail is, but snapshotCreated, snapshotUpdated, snapshotRemoved, etc are not), negating it doesn't make as much sense as expect(a).to.not.eql(b) does. I may remove this functionality.

We are hoping to further improve the UX and I feel like we are just at the beginning with Jest snapshot testing – if you'd ever like to use Jest itself and help us improve our snapshot feature, please let use know :)

I tried Jest for the first time while writing this library in order to learn how the snapshot feature works. I had a really good experience; the whole thing was very polished, and babel "just working" was a huge plus. I feel like the tests and snapshots folders look kinda ugly, but I could get used to it without much coercion (I prefer to have something like someModule.js and someModule.spec.js next to each other in the same tree, so that the relative import paths are the same and so that I can see what isn't tested at all at a glance).

When Jest was first announced, my initial impressions were "I feel uncomfortable about this because automocking is very different". But as I've let the concept sit in my head, it's made more and more sense to use modules as a mocking boundary. I also can't say that I love all the steps needed to get babel, webpack, etc working in other more "modular" frameworks, especially if I have to run the specs in a browser.

Jest may become my preferred testing framework as I give it more attention; it's a really polished project. Do you have any roadmaps or milestones that I could contribute to?

[suchipi]

One other thing... I noticed that jest creates snapshot names with a number like 1 on the end of them. Is this the version of the snapshot? Or is this just to prevent namespace collision? I didn't implement this in my chai assertion helper, because I didn't understand the intended behavior. But I'd like to know more.

[cpojer]

Thanks for your response.

Integration

I think jest-snapshot should contain all the functionality necessary to integrate the feature into other runners. Usually the way we approach problem solving is that we aren't providing solutions to problems we don't have, which is why you are in the unfortunate position to have to go in and make it adaptable for your use-case. I'm not sure I understand your idea about processSnapshot. I feel like you could do:

const matcher = require('jest-snapshot').matcher;
const result = matcher(filePath, options, /* TODO: remove this arg; but it is a breaking change */ null, snapshotState)().compare(actual);

It looks like some indirection because Jasmine matchers kind of suck but we are planning to simplify this matcher in the next release (cc @DmitriiAbramov). The result variable should then contain a message and a pass flag which you can use in chai. I think your chai matcher should wrap around this matcher in Jest and it should already be possible.

I think you can actually move snapshot tracking into beforeAll/afterAll and implement it on top of the APIs provided by jest-snapshot. If it proves to be universally useful, we can integrate it into jest-snapshot but I think I'd like to see it work in "userland" first.

.not matcher

Yeah, it doesn't make so much sense to me so I'd just prohibit it.

The number/name of snapshots

We made it so that Jest keeps track of the current test name and for each test (call to it) it keeps a counter. Consider this example:

describe('abc', () => it('def', () => expect(foo).toMatchSnapshot());
it('test', expect(bar).toMatchSnapshot());

will generate "abc def 1" as first snapshot name and "test 1" as second. If you add a second snapshot in the "def"-it-call, it's name will be "abc def 2". We then alphabetically sort all the snapshots from a test and save them in a file. This is so that people can have as many snapshots per file as they want and when they reorder their assertions the snapshot file won't change.

Jest and contributions

Thank you for your kind words. I agree automocking is often counter-productive and we now recommend to disable automocking and are likely going to ship with automocking disabled by default in the next major release of Jest. We also dropped the ball initially when we shipped Jest but it has improved a lot since.

You can find a high-level overview of our plans in the Jest 14.0 blog post and I'm happy to share more details. We literally want to redo everything about Jest that sucks and one project that I'm excited about and am hoping that someone from the community could take on is to rewrite the reporter code and what Jest outputs when run from the CLI – it could be so much prettier than it is. However, if you'd like to start using Jest, I'd be happy to listen to your pain points and what you'd like to change to better suit your needs. I'm sure we can make it work well for you.

[suchipi]

Regarding using the matcher: There are a few reasons that I can not use it in its current state, detailed below.

Counter

Because the matcher handles the counter logic, I cannot create snapshots whose names do not use the counter. In order to remain relatively unopinionated (as I feel this is one of the reasons people like chai), my assertion helper asks for the filename and snapshot name of the snapshot the user would like to compare against.

expect(object).to.matchSnapshot(snapshotFileName, snapshotName)

If I use the matcher, then I can't avoid adding a counter to the snapshotName:

// assuming my assertion helper uses the matcher
expect(object).to.matchSnapshot(path.join(__dirname, "object.spec.js.snap"), "object")
// creates a snapshot in object.spec.js.snap named "object 1"

I think adding the counter number makes sense in jest because the matcher interface is implicit and jest keeps track of the snapshots. However, because I would like my chai assertion helper to be more configurable, the interface is explicit. It would be confusing for users if they told the assertion helper to create a snapshot named "My snapshot" and it was actually saved as "My snapshot 1".

I tried to workaround this issue by "stubbing out" incrementCounter in the snapshotState I pass in:

  let snapshotState = {
    ...
    incrementCounter: () => "",
    ...
  };

But the matcher would still add a space to the end of the string in this case.

Eventually, when I start looking at mocha integration, I may use the same counter rules that you have implemented in jest. However, I want the counter to be a responsibility of the mocha integration instead of the chai integration; I'm thinking that the mocha integration will probably be a babel plugin that fills in the arguments to the chai assertion helper. So it could add the counter this way:

it("works", function() {
  expect(object).to.matchSnapshot();
});

- becomes ->

it("works", function() {
  expect(object).to.matchSnapshot(/* filename injected based on mocha integration convention */, "works 1");
});

Message

The message returned by the matcher is a string with a hardcoded newline and then the snapshot diff.

Chai has a builtin way of providing diffs to be shown at test failure (you provide the expected and actual, and it handles the diffing for you). The reporter is configurable and the user can choose not to show diffs across the board for all chai assertions.

Because the matcher diffs the expected and actual itself and only returns the diff as a part of the error message, if I used the message provided by the matcher, then I would show diffs in a nonstandard format compared to other chai diffs, and I may be showing a diff when the user has requested that chai never show diffs.

In addition, the message provided on failure uses the snapshot count generated by the matcher. Because I do not wish to use the snapshot count feature directly in the chai integration, this message format is not very useful.

[cpojer]

Message

I recommend to send a pull request that refactors the matcher into two parts: one that returns {pass, actual, expected} and one (the jasmine matcher) that wraps the raw matcher and transforms it into {pass, message}. This way you can do whatever you'd like with the actual and expected value.

Counter

If you create a PR as suggested above, the raw matcher will receive a snapshot state. You can then provide your own snapshot state. We should also make getSnapshotState separate from patching jasmine.

Concrete suggestions:

Add:

• jest-snapshot.createSnapshotState(filePath)
• jest-snapshot.patchJasmine(jasmine, snapshotState)
• Split up the matchers into a raw matcher and a jasmine matcher.

How does this sound?

[suchipi]

I think the suggested changes should work. I plan to take a look at this (and implement those changes) at some point; haven't had a lot of time lately.

[github-actions]

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.
Please note this issue tracker is not a help forum. We recommend using StackOverflow or our discord channel for questions.