Make snapshots usable again

* Make the Runner manage the snapshot state. Thread an accessor to the `t.snapshot()` assertion. * Save snapshots when runner has finished. Fixes #1218. * Use jest-snapshot directly, without serializing values. Use jest-diff to generate the diff if the snapshot doesn't match. This does mean the output is not colored and has other subtle differences with how other assertions format values, but that's OK for now. Fixes #1220, #1254. * Pin jest-snapshot and jest-diff versions. This isn't ideal but we're using private APIs and making other assumptions, so I'm not comfortable with using a loose SemVer range.
avajs · Apr 2, 2017 · 57fd051 · 57fd051
1 parent 48b892a
commit 57fd051
Show file tree

Hide file tree

Showing 13 changed files with 139 additions and 201 deletions.
diff --git a/lib/assert.js b/lib/assert.js
@@ -2,13 +2,11 @@
 const coreAssert = require('core-assert');
 const deepEqual = require('lodash.isequal');
 const observableToPromise = require('observable-to-promise');
-const indentString = require('indent-string');
 const isObservable = require('is-observable');
 const isPromise = require('is-promise');
-const jestSnapshot = require('jest-snapshot');
+const jestDiff = require('jest-diff');
 const enhanceAssert = require('./enhance-assert');
 const formatAssertError = require('./format-assert-error');
-const snapshotState = require('./snapshot-state');
 
 class AssertionError extends Error {
 	constructor(opts) {
@@ -251,21 +249,21 @@ function wrapAssertions(callbacks) {
 			}
 		},
 
-		snapshot(actual, optionalMessage) {
-			const result = snapshot(this, actual, optionalMessage);
+		snapshot(actual, message) {
+			const state = this._test.getSnapshotState();
+			const result = state.match(this.title, actual);
 			if (result.pass) {
 				pass(this);
 			} else {
-				const diff = formatAssertError.formatDiff(actual, result.expected);
-				const values = diff ? [diff] : [
-					formatAssertError.formatWithLabel('Actual:', actual),
-					formatAssertError.formatWithLabel('Must be deeply equal to:', result.expected)
-				];
-
+				const diff = jestDiff(result.expected.trim(), result.actual.trim(), {expand: true})
+					// Remove annotation
+					.split('\n')
+					.slice(3)
+					.join('\n');
 				fail(this, new AssertionError({
 					assertion: 'snapshot',
-					message: result.message,
-					values
+					message: message || 'Did not match snapshot',
+					values: [{label: 'Difference:', formatted: diff}]
 				}));
 			}
 		}
@@ -378,50 +376,3 @@ function wrapAssertions(callbacks) {
 	return Object.assign(assertions, enhancedAssertions);
 }
 exports.wrapAssertions = wrapAssertions;
-
-function snapshot(executionContext, tree, optionalMessage, match, snapshotStateGetter) {
-	// Set defaults - this allows tests to mock deps easily
-	const toMatchSnapshot = match || jestSnapshot.toMatchSnapshot;
-	const getState = snapshotStateGetter || snapshotState.get;
-
-	const state = getState();
-
-	const context = {
-		dontThrow() {},
-		currentTestName: executionContext.title,
-		snapshotState: state
-	};
-
-	// Symbols can't be serialized and saved in a snapshot, that's why tree
-	// is saved in the `__ava_react_jsx` prop, so that JSX can be detected later
-	const serializedTree = tree.$$typeof === Symbol.for('react.test.json') ? {__ava_react_jsx: tree} : tree; // eslint-disable-line camelcase
-	const result = toMatchSnapshot.call(context, JSON.stringify(serializedTree));
-
-	let message = 'Please check your code or --update-snapshots';
-
-	if (optionalMessage) {
-		message += '\n\n' + indentString(optionalMessage, 2);
-	}
-
-	state.save();
-
-	let expected;
-
-	if (result.expected) {
-		// JSON in a snapshot is surrounded with `"`, because jest-snapshot
-		// serializes snapshot values too, so it ends up double JSON encoded
-		expected = JSON.parse(result.expected.slice(1).slice(0, -1));
-		// Define a `$$typeof` symbol, so that pretty-format detects it as React tree
-		if (expected.__ava_react_jsx) { // eslint-disable-line camelcase
-			expected = expected.__ava_react_jsx; // eslint-disable-line camelcase
-			Object.defineProperty(expected, '$$typeof', {value: Symbol.for('react.test.json')});
-		}
-	}
-
-	return {
-		pass: result.pass,
-		expected,
-		message
-	};
-}
-exports.snapshot = snapshot;
diff --git a/lib/main.js b/lib/main.js
@@ -7,10 +7,12 @@ const Runner = require('./runner');
 
 const opts = globals.options;
 const runner = new Runner({
+	bail: opts.failFast,
 	failWithoutAssertions: opts.failWithoutAssertions,
+	file: opts.file,
+	match: opts.match,
 	serial: opts.serial,
-	bail: opts.failFast,
-	match: opts.match
+	updateSnapshots: opts.updateSnapshots
 });
 
 worker.setRunner(runner);
@@ -78,7 +80,11 @@ globals.setImmediate(() => {
 		adapter.ipcChannel.unref();
 
 		runner.run(options)
-			.then(exit)
+			.then(() => {
+				runner.saveSnapshotState();
+
+				return exit();
+			})
 			.catch(err => {
 				process.emit('uncaughtException', err);
 			});

diff --git a/lib/runner.js b/lib/runner.js
@@ -1,6 +1,8 @@
 'use strict';
 const EventEmitter = require('events');
+const path = require('path');
 const Bluebird = require('bluebird');
+const jestSnapshot = require('jest-snapshot');
 const optionChain = require('option-chain');
 const matcher = require('matcher');
 const TestCollection = require('./test-collection');
@@ -45,14 +47,18 @@ class Runner extends EventEmitter {
 
 		options = options || {};
 
+		this.file = options.file;
 		this.match = options.match || [];
 		this.serial = options.serial;
+		this.updateSnapshots = options.updateSnapshots;
 
 		this.hasStarted = false;
 		this.results = [];
+		this.snapshotState = null;
 		this.tests = new TestCollection({
 			bail: options.bail,
-			failWithoutAssertions: options.failWithoutAssertions
+			failWithoutAssertions: options.failWithoutAssertions,
+			getSnapshotState: () => this.getSnapshotState()
 		});
 
 		this.chain = optionChain(chainableMethods, (opts, args) => {
@@ -173,6 +179,29 @@ class Runner extends EventEmitter {
 		return stats;
 	}
 
+	getSnapshotState() {
+		if (this.snapshotState) {
+			return this.snapshotState;
+		}
+
+		const name = path.basename(this.file) + '.snap';
+		const dir = path.dirname(this.file);
+
+		const snapshotPath = path.join(dir, '__snapshots__', name);
+		const testPath = this.file;
+		const update = this.updateSnapshots;
+
+		const state = jestSnapshot.initializeSnapshotState(testPath, update, snapshotPath);
+		this.snapshotState = state;
+		return state;
+	}
+
+	saveSnapshotState() {
+		if (this.snapshotState) {
+			this.snapshotState.save(this.updateSnapshots);
+		}
+	}
+
 	run(options) {
 		if (options.runOnlyExclusive && !this.tests.hasExclusive) {
 			return Promise.resolve(null);

diff --git a/lib/snapshot-state.js b/lib/snapshot-state.js
diff --git a/lib/test-collection.js b/lib/test-collection.js
@@ -11,6 +11,7 @@ class TestCollection extends EventEmitter {
 
 		this.bail = options.bail;
 		this.failWithoutAssertions = options.failWithoutAssertions;
+		this.getSnapshotState = options.getSnapshotState;
 		this.hasExclusive = false;
 		this.testCount = 0;
 
@@ -132,6 +133,7 @@ class TestCollection extends EventEmitter {
 			contextRef,
 			failWithoutAssertions: false,
 			fn: hook.fn,
+			getSnapshotState: this.getSnapshotState,
 			metadata: hook.metadata,
 			onResult: this._emitTestResult,
 			title
@@ -148,6 +150,7 @@ class TestCollection extends EventEmitter {
 			contextRef,
 			failWithoutAssertions: this.failWithoutAssertions,
 			fn: test.fn,
+			getSnapshotState: this.getSnapshotState,
 			metadata: test.metadata,
 			onResult: this._emitTestResult,
 			title: test.title

diff --git a/lib/test.js b/lib/test.js
@@ -98,6 +98,7 @@ class Test {
 		this.contextRef = options.contextRef;
 		this.failWithoutAssertions = options.failWithoutAssertions;
 		this.fn = isGeneratorFn(options.fn) ? co.wrap(options.fn) : options.fn;
+		this.getSnapshotState = options.getSnapshotState;
 		this.metadata = options.metadata;
 		this.onResult = options.onResult;
 		this.title = options.title;

diff --git a/package.json b/package.json
@@ -133,7 +133,8 @@
     "is-obj": "^1.0.0",
     "is-observable": "^0.2.0",
     "is-promise": "^2.1.0",
-    "jest-snapshot": "^18.1.0",
+    "jest-diff": "19.0.0",
+    "jest-snapshot": "19.0.2",
     "js-yaml": "^3.8.2",
     "last-line-stream": "^1.0.0",
     "lodash.debounce": "^4.0.3",

diff --git a/test/assert.js b/test/assert.js
@@ -1,6 +1,7 @@
 'use strict';
+const path = require('path');
+const jestSnapshot = require('jest-snapshot');
 const test = require('tap').test;
-const sinon = require('sinon');
 const assert = require('../lib/assert');
 const formatValue = require('../lib/format-assert-error').formatValue;
 
@@ -573,74 +574,50 @@ test('.ifError()', t => {
 	t.end();
 });
 
-test('snapshot makes a snapshot using a library and global options', t => {
-	const saveSpy = sinon.spy();
-	const state = {save: saveSpy};
-	const stateGetter = sinon.stub().returns(state);
-	const matchStub = sinon.stub().returns({pass: true});
-
-	const test = {
-		title: 'Test name'
+test('.snapshot()', t => {
+	// Set to `true` to update the snapshot, then run:
+	// "$(npm bin)"/tap --no-cov -R spec test/assert.js
+	//
+	// Ignore errors and make sure not to run tests with the `-b` (bail) option.
+	const update = false;
+
+	const state = jestSnapshot.initializeSnapshotState(__filename, update, path.join(__dirname, 'fixture', 'assert.snap'));
+	const executionContext = {
+		_test: {
+			getSnapshotState() {
+				return state;
+			}
+		},
+		title: ''
 	};
 
-	t.plan(4);
-
-	t.doesNotThrow(() => {
-		assert.snapshot(test, 'tree', undefined, matchStub, stateGetter);
-	});
-
-	t.ok(stateGetter.called);
-
-	t.match(matchStub.firstCall.thisValue, {
-		currentTestName: 'Test name',
-		snapshotState: state
+	passes(t, () => {
+		executionContext.title = 'passes';
+		assertions.snapshot.call(executionContext, {foo: 'bar'});
 	});
 
-	t.ok(saveSpy.calledOnce);
-	t.end();
-});
-
-test('snapshot handles jsx tree', t => {
-	const saveSpy = sinon.spy();
-	const state = {save: saveSpy};
-	const stateGetter = sinon.stub().returns(state);
-	const matchStub = sinon.stub().returns({pass: true});
-
-	const test = {
-		title: 'Test name'
-	};
-
-	t.plan(5);
-
-	t.doesNotThrow(() => {
-		const tree = {
-			type: 'h1',
-			children: ['Hello'],
-			props: {}
-		};
-
-		Object.defineProperty(tree, '$$typeof', {value: Symbol.for('react.test.json')});
-
-		assert.snapshot(test, tree, undefined, matchStub, stateGetter);
+	failsWith(t, () => {
+		executionContext.title = 'fails';
+		assertions.snapshot.call(executionContext, {foo: update ? 'bar' : 'not bar'});
+	}, {
+		assertion: 'snapshot',
+		message: 'Did not match snapshot',
+		values: [{label: 'Difference:', formatted: 'Object {\n-   "foo": "bar",\n+   "foo": "not bar",\n  }'}]
 	});
 
-	t.ok(stateGetter.called);
-
-	const savedTree = JSON.parse(matchStub.firstCall.args[0]);
-	t.deepEqual(savedTree, {
-		__ava_react_jsx: { // eslint-disable-line camelcase
-			type: 'h1',
-			children: ['Hello'],
-			props: {}
-		}
+	failsWith(t, () => {
+		executionContext.title = 'fails';
+		assertions.snapshot.call(executionContext, {foo: update ? 'bar' : 'not bar'}, 'my message');
+	}, {
+		assertion: 'snapshot',
+		message: 'my message',
+		values: [{label: 'Difference:', formatted: 'Object {\n-   "foo": "bar",\n+   "foo": "not bar",\n  }'}]
 	});
 
-	t.match(matchStub.firstCall.thisValue, {
-		currentTestName: 'Test name',
-		snapshotState: state
-	});
+	if (update) {
+		state.save(true);
+	}
 
-	t.ok(saveSpy.calledOnce);
 	t.end();
 });