Consider not updating snapshots automatically

[novemberborn]

It may be considered odd that AVA automatically updates snapshots. From @billyjanitsch #1585 (comment):

I've never liked the behavior of auto-creating new snapshots without -u: I imagine most users think of running ava as a pure function which returns a boolean describing whether their tests passed or failed. So it's odd for ava to have side effects by default, especially those which modify VCS-tracked files.

Then from @novemberborn #1585 (comment):

what if we'd write snapshot updates to a temporary location, exit with an instructive failure and exit code of 1, and then have a npx ava --commit-snapshots command (pending bike-shedding) to write them into [the] source tree?

[ahukkanen]

I am currently writing a modification to a library that uses AVA for the tests and I found the current behavior very confusing before I bumped into the original issue (#1585) comments. There is also no mention about this behavior in the documentation:
https://github.com/avajs/ava/blob/master/docs/04-snapshot-testing.md

This change would be highly welcomed for newcomers. This is my first time ever using AVA.

[sramam]

Given my extensive commenting on #1585, I'd like to argue against this.

ava's current behavior is consistent with other js-land test frameworks.
While I'm not against doing different, it should not be done without sufficient cause.

Many of the issues raised by @billyjanitsch, while real, belong outside a test runner.

Pragmatically, it complicates scenarios like running a test suite as a commit/push/prepublish hook. I've often successfully pushed commits (relying on my git hook to fail if the tests do anything other than succeed) only to realize that the test suite passed but dirtied the repo, so my (incorrect) commit shouldn't have gone through.

IMHO the pre-commit/push/publish hooks should include a step to check for uncommitted modifications. That's not to deny @billyjanitsch's concern that it complicates things, it does.

It'll either require ava to include isomorphic-git or have an implied dependency on a git executable being available. Either of these dependencies seems too onerous a price.

I think @novemberborn's instinct on calling this bike-shedding is exactly right.

[ahukkanen]

Either way, document how it works so that also people not familiar with AVA will understand how it works. Or at least when they bump into this behavior, they get the information from the documentation that it is how it is designed to work.

[novemberborn]

Yea I'm ambivalent on this. If we get the experience right, this may be worthwhile. But equally perhaps the solution is to have improved reporter output.

[ninevra]

The --commit-snapshots workflow described in #1585 (comment) sounds like the workflow used by insta and trybuild (both rust projects); so, it's attested, though maybe not in javascript. It makes comparing and updating snapshots very convenient, because one doesn't have to re-run all the tests.

This workflow also solves #2636: since committing snapshots is separate from the test run, it's natural for snapshot assertions to fail on a mismatch, and the user expects to re-run tests after committing snapshots anyway. The test runner then doesn't have to predict whether the snapshots will end up committed or not.

trybuild helpfully saves the temporary snapshots in a wip directory containing a .gitignore with pattern * so that the temporary values don't clutter git status or accidentally end up in version control.

[novemberborn]

@ninevra that sounds great.

[conartist6]

Why not just introduce a flag called, say, --no-initial-snapshots that refuses to create initial snapshots? Then tests which would otherwise result in the creation of a new snapshot would instead fail, which is the desirable behavior in CI and also does not involve any coupling to version control.