Don't update snapshots when executing a subset of tests

Co-authored-by: Mark Wubben <mark@novemberborn.net>

docs/01-writing-tests.md

@@ -118,9 +118,11 @@ You can use the `.only` modifier with all tests. It cannot be used with hooks or
 
 *Note:* The `.only` modifier applies to the test file it's defined in, so if you run multiple test files, tests in other files will still run. If you want to only run the `test.only` test, provide just that test file to AVA.
 
+You cannot update snapshots when using `.only()`.
+
 ## Skipping tests
 
-Sometimes failing tests can be hard to fix. You can tell AVA to skip these tests using the `.skip` modifier. They'll still be shown in the output (as having been skipped) but are never run.
+Sometimes failing tests can be hard to fix. You can tell AVA to temporarily skip these tests using the `.skip` modifier. They'll still be shown in the output (as having been skipped) but are never run.
 
 ```js
 test.skip('will not be run', t => {
@@ -130,6 +132,8 @@ test.skip('will not be run', t => {
 
 You must specify the implementation function. You can use the `.skip` modifier with all tests and hooks, but not with `.todo()`. You can not apply further modifiers to `.skip`.
 
+You cannot update snapshots when using `.skip()`. If the test is likely to be failing for a while, use `.failing()` instead.
+
 ## Test placeholders ("todo")
 
 You can use the `.todo` modifier when you're planning to write a test. Like skipped tests these placeholders are shown in the output. They only require a title; you cannot specify the implementation function.

lib/cli.js

@@ -182,6 +182,10 @@ exports.run = async () => { // eslint-disable-line complexity
 	const chalkOptions = {level: combined.color === false ? 0 : require('chalk').level};
 	const chalk = require('./chalk').set(chalkOptions);
 
+	if (combined.updateSnapshots && combined.match) {
+		exit('Snapshots cannot be updated when matching specific tests.');
+	}
+
 	if (confError) {
 		if (confError.parent) {
 			exit(`${confError.message}\n\n${chalk.gray((confError.parent && confError.parent.stack) || confError.parent)}`);
@@ -356,6 +360,9 @@ exports.run = async () => { // eslint-disable-line complexity
 			pattern: normalizePattern(path.relative(projectDir, path.resolve(process.cwd(), pattern))),
 			...rest
 		}));
+	if (combined.updateSnapshots && filter.some(condition => condition.lineNumbers !== null)) {
+		exit('Snapshots cannot be updated when selecting specific tests by their line number.');
+	}
 
 	const api = new Api({
 		cacheEnabled: combined.cache !== false,

lib/reporters/default.js

@@ -153,6 +153,7 @@ class Reporter {
 		this.lineNumberErrors = [];
 		this.uncaughtExceptions = [];
 		this.unhandledRejections = [];
+		this.unsavedSnapshots = [];
 
 		this.previousFailures = 0;
 
@@ -295,6 +296,10 @@ class Reporter {
 				break;
 			}
 
+			case 'snapshot-error':
+				this.unsavedSnapshots.push(event);
+				break;
+
 			case 'uncaught-exception': {
 				this.uncaughtExceptions.push(event);
 
@@ -749,6 +754,16 @@ class Reporter {
 			}
 		}
 
+		if (this.unsavedSnapshots.length > 0) {
+			this.lineWriter.writeLine(colors.title('Could not update snapshots for the following test files:'));
+			this.lineWriter.writeLine();
+			for (const event of this.unsavedSnapshots) {
+				this.lineWriter.writeLine(`${figures.warning} ${this.relativeFile(event.testFile)}`);
+			}
+
+			this.lineWriter.writeLine();
+		}
+
 		if (this.failFastEnabled && (this.stats.remainingTests > 0 || this.stats.files > this.stats.finishedWorkers)) {
 			let remaining = '';
 			if (this.stats.remainingTests > 0) {

lib/reporters/tap.js

@@ -158,6 +158,9 @@ class TapReporter {
 					this.writeTest(evt, {passed: false, todo: true, skip: false});
 				}
 
+				break;
+			case 'snapshot-error':
+				this.writeComment(evt, {title: 'Could not update snapshots'});
 				break;
 			case 'stats':
 				this.stats = evt.stats;

lib/runner.js

@@ -23,6 +23,7 @@ class Runner extends Emittery {
 		this.recordNewSnapshots = options.recordNewSnapshots === true;
 		this.runOnlyExclusive = options.runOnlyExclusive === true;
 		this.serial = options.serial === true;
+		this.skippingTests = false;
 		this.snapshotDir = options.snapshotDir;
 		this.updateSnapshots = options.updateSnapshots;
 
@@ -147,6 +148,10 @@ class Runner extends Emittery {
 							task.metadata.exclusive = matcher([title], this.match).length === 1;
 						}
 
+						if (task.metadata.skipped) {
+							this.skippingTests = true;
+						}
+
 						if (task.metadata.exclusive) {
 							this.runOnlyExclusive = true;
 						}
@@ -182,7 +187,7 @@ class Runner extends Emittery {
 				fixedLocation: this.snapshotDir,
 				projectDir: this.projectDir,
 				recordNewSnapshots: this.recordNewSnapshots,
-				updating: this.updateSnapshots
+				updating: this.updateSnapshots && !this.runOnlyExclusive && !this.skippingTests
 			});
 			this.emit('dependency', this.snapshots.snapPath);
 		}
@@ -191,8 +196,12 @@ class Runner extends Emittery {
 	}
 
 	saveSnapshotState() {
+		if (this.updateSnapshots && (this.runOnlyExclusive || this.skippingTests)) {
+			return {cannotSave: true};
+		}
+
 		if (this.snapshots) {
-			return this.snapshots.save();
+			return {touchedFiles: this.snapshots.save()};
 		}
 
 		if (this.updateSnapshots) {
@@ -201,7 +210,7 @@ class Runner extends Emittery {
 			// were skipped. Perhaps emit a warning if this occurs?
 		}
 
-		return null;
+		return {};
 	}
 
 	onRun(runnable) {

lib/worker/subprocess.js

@@ -91,8 +91,10 @@ ipc.options.then(async options => {
 
 	runner.on('finish', () => {
 		try {
-			const touchedFiles = runner.saveSnapshotState();
-			if (touchedFiles) {
+			const {cannotSave, touchedFiles} = runner.saveSnapshotState();
+			if (cannotSave) {
+				ipc.send({type: 'snapshot-error'});
+			} else if (touchedFiles) {
 				ipc.send({type: 'touched-files', files: touchedFiles});
 			}
 		} catch (error) {

test/helpers/exec.js

@@ -7,6 +7,8 @@ const execa = require('execa');
 const cliPath = path.resolve(__dirname, '../../cli.js');
 const serialization = process.versions.node >= '12.16.0' ? 'advanced' : 'json';
 
+const normalizePath = (root, file) => path.posix.normalize(path.relative(root, file));
+
 exports.fixture = async (...args) => {
 	const cwd = path.join(path.dirname(test.meta.file), 'fixtures');
 	const running = execa.node(cliPath, args, {
@@ -18,6 +20,9 @@ exports.fixture = async (...args) => {
 	});
 
 	const stats = {
+		failed: [],
+		skipped: [],
+		unsavedSnapshots: [],
 		passed: []
 	};
 
@@ -27,9 +32,30 @@ exports.fixture = async (...args) => {
 		}
 
 		switch (message.type) {
+			case 'selected-test': {
+				if (message.skip) {
+					const {title, testFile} = message;
+					stats.skipped.push({title, file: normalizePath(cwd, testFile)});
+				}
+
+				break;
+			}
+
+			case 'snapshot-error': {
+				const {testFile} = message;
+				stats.unsavedSnapshots.push({file: normalizePath(cwd, testFile)});
+				break;
+			}
+
 			case 'test-passed': {
 				const {title, testFile} = message;
-				stats.passed.push({title, file: path.posix.relative(cwd, testFile)});
+				stats.passed.push({title, file: normalizePath(cwd, testFile)});
+				break;
+			}
+
+			case 'test-failed': {
+				const {title, testFile} = message;
+				stats.failed.push({title, file: normalizePath(cwd, testFile)});
 				break;
 			}
 

test/snapshot-updates/fixtures/contains-only.js

@@ -0,0 +1,10 @@
+const test = require('ava');
+
+test('always failing snapshot', t => {
+	t.snapshot(Date.now());
+});
+
+test.only('exclusive test', t => { // eslint-disable-line ava/no-only-test
+	t.pass();
+});
+

test/snapshot-updates/fixtures/contains-only.js.md

@@ -0,0 +1,11 @@
+# Snapshot report for `contains-only.js`
+
+The actual snapshot is saved in `contains-only.js.snap`.
+
+Generated by [AVA](https://avajs.dev).
+
+## always failing snapshot
+
+> Snapshot 1
+
+    1592749428423

test/snapshot-updates/fixtures/contains-skip.js

@@ -0,0 +1,10 @@
+const test = require('ava');
+
+test('always failing snapshot', t => {
+	t.snapshot(Date.now());
+});
+
+test.skip('skipped test', t => { // eslint-disable-line ava/no-skip-test
+	t.pass();
+});
+

test/snapshot-updates/fixtures/contains-skip.js.md

@@ -0,0 +1,11 @@
+# Snapshot report for `contains-skip.js`
+
+The actual snapshot is saved in `contains-skip.js.snap`.
+
+Generated by [AVA](https://avajs.dev).
+
+## always failing snapshot
+
+> Snapshot 1
+
+    1592749428431

test/snapshot-updates/fixtures/package.json

@@ -0,0 +1,10 @@
+{
+  "ava": {
+    "files": [
+      "*.js"
+    ]
+  },
+  "dependencies": {
+    "ava": "file:../../.."
+  }
+}

test/snapshot-updates/snapshots/test.js.md

@@ -0,0 +1,68 @@
+# Snapshot report for `test/snapshot-updates/test.js`
+
+The actual snapshot is saved in `test.js.snap`.
+
+Generated by [AVA](https://avajs.dev).
+
+## cannot update snapshots when file contains exclusive tests
+
+> failed tests
+
+    []
+
+> passed tests
+
+    [
+      {
+        file: 'contains-only.js',
+        title: 'exclusive test',
+      },
+    ]
+
+> files where snapshots could not be updated
+
+    [
+      {
+        file: 'contains-only.js',
+      },
+    ]
+
+## cannot update snapshots when file contains skipped tests
+
+> failed tests
+
+    [
+      {
+        file: 'contains-skip.js',
+        title: 'always failing snapshot',
+      },
+    ]
+
+> skipped tests
+
+    [
+      {
+        file: 'contains-skip.js',
+        title: 'skipped test',
+      },
+    ]
+
+> files where snapshots could not be updated
+
+    [
+      {
+        file: 'contains-skip.js',
+      },
+    ]
+
+## cannot update snapshots when matching test titles
+
+> Snapshot 1
+
+    'Snapshots cannot be updated when matching specific tests.'
+
+## cannot update snapshots when selecting tests by line number
+
+> Snapshot 1
+
+    'Snapshots cannot be updated when selecting specific tests by their line number.'

test/snapshot-updates/test.js

@@ -0,0 +1,28 @@
+const test = require('@ava/test');
+const exec = require('../helpers/exec');
+
+test('cannot update snapshots when file contains skipped tests', async t => {
+	const result = await t.throwsAsync(exec.fixture('contains-skip.js', '-u'));
+	t.snapshot(result.stats.failed, 'failed tests');
+	t.snapshot(result.stats.skipped, 'skipped tests');
+	t.snapshot(result.stats.unsavedSnapshots, 'files where snapshots could not be updated');
+});
+
+test('cannot update snapshots when file contains exclusive tests', async t => {
+	const result = await exec.fixture('contains-only.js', '-u');
+	t.snapshot(result.stats.failed, 'failed tests');
+	t.snapshot(result.stats.passed, 'passed tests');
+	t.snapshot(result.stats.unsavedSnapshots, 'files where snapshots could not be updated');
+});
+
+const stripLeadingFigures = string => string.replace(/^\W+/, '');
+
+test('cannot update snapshots when matching test titles', async t => {
+	const result = await t.throwsAsync(exec.fixture('contains-skip.js', '-u', '-m=snapshot'));
+	t.snapshot(stripLeadingFigures(result.stderr.trim()));
+});
+
+test('cannot update snapshots when selecting tests by line number', async t => {
+	const result = await t.throwsAsync(exec.fixture('contains-skip.js:4', '-u'));
+	t.snapshot(stripLeadingFigures(result.stderr.trim()));
+});