Jest: Question: Snapshot testing, environments and -u

[emirotin]

Hi there.

I hope it's a good place to ask questions.
Snapshot testing is great, but I have 2 concerns / questions:

1) Snapshots are static. Sometimes component rendering depends on the environment / config.
For example:

import config from '~/config'
const Avatar = ({ username, size }) => <img src={`${config.apiUrl}/avatar/${username}?s=${size}`} />

While developing I may have localhost:8080 as my API. It will record it in the snapshots. Then on the CI server, it can be set to production and the tests will fail.

A valid comment would be that this component could take the base URL as a prop. But then somebody higher in the hierarchy will have to pass it, so either that component will fail the snapshot testing, or the config will have to be passed from the very top, from the root App component (in which case I can provide stable testing config).

But then passing it all the way down will be tedious.

So is it that context is the only way to organize it properly? Taking into account it's considered experimental and unstable is there any good alternative?

2) When I make some changes and run tests there's a -u option to update the snapshots. Now imagine I've mad changes that caused two snapshots mismatches. One is because the things have to change, but the other being the real bug.

I suppose -u will update both snapshots. And If I have 10 test cases that need to update (say an online indicator is added to the avatar) and 1 bug it becomes easy to miss the bug and accidentally update the snapshot for it.

What is the proper way to manage it? Only use -u with --bail?

[cpojer]

Thanks for bringing up these questions. I think it's important we have these discussions and think about where we can take snapshot testing from this basic version we have right now.

To prevent against environment specific configuration: This is where Jest's mocking system can really shiny. I'm assuming you aren't making actual API calls in your test run (if you are, you should probably record their outputs) so you can mock out the module that provides the API url:

jest.mock('~/config', () => {
  const config = require.requireActual('~/config');
  config.apiUrl = 'http://my-app.com';
  return config;
});

import config from '~/config';

// config.apiUrl will always be the mocked version

This is basically a form of "dependency injection" with modules as boundaries.

To answer your second question: --bail actually only works on a per-test-file basis, not within a test but I can see how we might want to change that eventually. Currently we also have a problem when fit is used to skip other tests or when a runtime error occurs: Jest will yell at you for having obsolete snapshots and that you should run with -u to remove them. I think we need to change Jest to be a bit more conservative and skip snapshot validation in those cases.

One thing we tried to do is to make snapshot files independent to test file changes as long as you are only moving around it calls within a file. For example:

it('renders a snapshot', () => expect('foo').toMatchSnasphot());

it('renders another snapshot', () => expect('foo').toMatchSnasphot());

should produce identical output to

it('renders another snapshot', () => expect('foo').toMatchSnasphot());

it('renders a snapshot', () => expect('foo').toMatchSnasphot());

To make the system as seamless as possible, we had to pick a barrier that we can use to create names for our snapshots. Logically it seemed like it was the most granular we could go without requiring a babel-transform or something. To guard against the specific issue you are running into I'd propose to separate individual tests into more it calls.

Does this make sense?

[emirotin]

I'm assuming you aren't making actual API calls in your test run

I'm not. It's a normal module that gets populated during the build step (and has some defaults for development).

This is where Jest's mocking system can really shiny.

Makes sense, thanks.

---

Does this make sense?

I'm not sure it if answers my question. Here's my case: imagine I have about 10 tests for the Avatar component. I've changed it (added the online indicator, or wrapped it in a DIV), so all the tests are failing. As there are many of them I'm eager to auto-update the snapshots (no way I'm going to edit the HTML strings by hand, right?).

Now if during my changes and refactoring I've introduced some real bug in some edge case it's getting masked, and set in stone as the new expected snapshot.

[cpojer]

I see what you mean. Yes, it is suggested that you review your changes carefully and that your code reviewers do the same. Snapshots should be considered additional signal during code review, as well.

[emirotin]

OK makes sense indeed — after the update is applied I can obviously review
it (TBH I didn't think about it :).
I think it answers my question and the issue can be closed.

Thanks for your support, going to give it a try soon.

On Thu, Jul 28, 2016 at 2:28 PM Christoph Pojer [email protected]
wrote:

I see what you mean. Yes, it is suggested that you review your changes
carefully and that your code reviewers do the same. Snapshots should be
considered additional signal during code review, as well.

—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
https://github.com/facebook/jest/issues/1328#issuecomment-235869241, or mute
the thread
https://github.com/notifications/unsubscribe-auth/AAgGCE5P-fBrSvpxV2cxh4RbzWFVcECiks5qaJJYgaJpZM4JXDhB
.

[aaronabramov]

also as we talked yesterday. It would be nice to have an interactive mode for updating snapshot (similar to git add -i)

Snapshot mismatch:
-expected +actual

- missing string
+ added string

Do you want to update this snapshot? Y\N
Y
...

that would be very helpful for going through test failures one by one after a big refactoring and filtering out legitimate failures from just formatting updates

[emirotin]

Exactly ^

On Thu, Jul 28, 2016 at 7:55 PM Dmitrii Abramov [email protected]
wrote:

also as we talked yesterday. It would be nice to have an interactive mode
for updating snapshot (similar to git add -i)

Snapshot mismatch:
-expected +actual

• missing string
• added string

Do you want to update this snapshot? YN
Y
...

that would be very helpful for going through test failures one by one
after a big refactoring and filtering out legitimate failures from just
formatting updates

—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
https://github.com/facebook/jest/issues/1328#issuecomment-235956554, or mute
the thread
https://github.com/notifications/unsubscribe-auth/AAgGCMDtMpGNGUVM8YoLHG-utWOjKX8tks5qaN8IgaJpZM4JXDhB
.

[cpojer]

Ohh that's a great idea @dmitriiabramov.

The initial idea for snapshot tests was to not be intrusive. If people have to stop and pause every time something changes, that would be bad. Adding it as an option is a good idea however.

[gaearon]

cc @keyanzhang who also wanted to explore this:

It would be nice to have an interactive mode for updating snapshot

[aaronjensen]

The initial idea for snapshot tests was to not be intrusive. If people have to stop and pause every time something changes, that would be bad. Adding it as an option is a good idea however.

Prompting for y/n in a normal flow has the potential to be less intrusive than forcing someone to run another command w/ -u. Consider running tests from your editor or running tests with --watch.

Imagine running with --watch and every time snapshot tests fail you're given the option of reviewing the failure(s) and approving or not. It does not need to interrupt, it can require the press of a key to enter into approval mode. There could be a command line flag to give the same prompt when running tests from the command line manually.

Another thing to consider is allowing an option to require initial approval. The way the feature is designed now, the initial test run always passes. This is not very good for TDD, but I suppose it is good for HMRDD. It'd be nice to get a view of the initial version and be asked to approve it with y/n.

[aaronjensen]

The new u feature in watch is awesome, thanks for that!

I think we still need an initial approval step for the first time a new snapshot is recorded. As it is, I have to open up the snapshot file manually to confirm that it looks ok. Of course, I can also confirm visually on the actual site or styleguide, but it'd be nice to have that step.

[cpojer]

@aaronjensen are you willing to work on an experiment/pull request for that? I agree it would be helpful as I need to look at this stuff manually too.

[aaronjensen]

@cpojer I can't commit to that just yet, I've got a couple other OSS to dos at the moment. Maybe in a few weeks.

[cpojer]

I'm gonna close this one in favor of #1798 which is a more focused discussion about just this feature we are discussing now.