Hacker News Reader

Comment by cpojer | Hacker News Reader

Hi! I work on JavaScript Tools at FB.

There is a fantastic post by Ben McCormick about the up and downsides of this system: http://benmccormick.org/2016/09/19/testing-with-jest-snapsho...

Snapshots aren't the only feature of Jest; it is simply one assertion in our assertion library. There is a ton of other stuff in the framework that makes setting up a test environment and writing tests easier.

My philosophy for building a test framework is to build a good feature set to help you out in any situation but the user should be in total control. This allows you to find the best way to test your code which I've found to be extremely subjective. The choice of test framework and test methodology seems almost religious at times and I've deliberately tried to stay away from these conversations.

View on HN

Indeed, I'm not criticising Jest which most of our developers really like, merely criticising snapshot testing, which is one of the ways we are using jest.

The Ben McCormick post does cover some of the issues with snapshot testing, particularly around lack of communication of developer intent.

If you wrote a unit test that asserted that a tree of data looked exactly a particular way, when the only correctness/incorrectness criteria was whether or not a particular prop was present on one of the branches, then you've written a bad test.

Using a snapshot means that your test fails when your code changes in ways that are still correct. It's a test that invites lots of false negatives, and the reason that is considered acceptable is because it makes it easy to update the test when it predictably gives you the false negative.

Normally writing tests that depend on the exact implementation details of the code under test is considered a bad thing. I can't help but think that there are better solutions to this problem.

Almost immediately after one of our teams started using snapshot testing, we had test breakages because an entirely different component that we used in an ancillary way had a semver minor change (added a new prop that was defaulted) and that broke tests (but not the app) in our component. Perhaps we're doing it wrong, but adding tests to packages that can trivially be incorrectly broken by your dependencies changing is fairly painful (and makes a mockery of semver).

I implemented the snapshot functionality in Jest and I work on Jest at FB.

> Almost immediately after one of our teams started using snapshot testing, we had test breakages because an entirely different component that we used in an ancillary way had a semver minor change (added a new prop that was defaulted) and that broke tests (but not the app) in our component. Perhaps we're doing it wrong, but adding tests to packages that can trivially be incorrectly broken by your dependencies changing is fairly painful (and makes a mockery of semver).

Snapshots should make you more confident when making changes.

In the case you just described they correctly caught that something changed.

Imagine a minor semver update that works in 99% of the cases but has a very tricky edge case scenario they didn't consider when bumping to the new version, and you're super lucky and you have that scenario in your codebase, but only in a single page of your app under a certain condition.

Snapshot failures highlights subtle dependencies between seemingly unrelated components, making you aware of it. I think at snapshot failures as warnings: those are potential breakages and I can now check that everything works, and with a good coverage I know exactly what changed and where, so I can go and check case by case. Once I'm absolutely sure everything is fine I can safely update the snapshots, which as you correctly stated is easy to do, I've just to pass a `-u` flag.

I realise that it can occasionally be useful to have a canary that says 'something has changed' as long as its easy to reset. Personally, I'd rather such a canary operated on the visual representation plus interactions, since that is what the user experiences, rather than error on tree differences that produce no effective difference to the end user.

I do think though that neither of these should replace tests that test the actual postconditions of your code, and that the preponderance of false negatives and ease of resetting the canary discourages developers from spending the effort required to properly understand what is going on.

Almost all programming is about erecting firewalls to contain changes, so that as they ripple out from the change site, they hit boundaries beyond which things no longer need to be modified. As part of that, modules document what you can and cannot rely on. If you have a test that breaks because it's relying on some feature of a modules output that is not considered public behaviour, the bug is in your test, not the module that causes the failure.

> In the case you just described they correctly caught that something changed.

It is not the job of tests to tell you that something changed, but that something broke.

Perhaps I'd have less problem if people talked about them as canaries rather than as tests. 'Test' implies that there's something wrong if you fail.

Reply on news.ycombinator.com