Deterministic snapshot files, declaration-ordered snapshot reports

Ensures that *.snap files are deterministic by sorting entry blocks by the hash of their test name or id.

Ensures that *.md snapshot report files are sorted in as close to declaration order as is reasonably possible.

Closes #2311
Closes #2324

Co-authored-by: Sindre Sorhus <sindresorhus@gmail.com>
Co-authored-by: Mark Wubben <mark@novemberborn.net>

lib/runner.js

@@ -31,6 +31,7 @@ class Runner extends Emittery {
 		this.boundCompareTestSnapshot = this.compareTestSnapshot.bind(this);
 		this.interrupted = false;
 		this.snapshots = null;
+		this.nextTaskIndex = 0;
 		this.tasks = {
 			after: [],
 			afterAlways: [],
@@ -76,6 +77,8 @@ class Runner extends Emittery {
 				});
 			}
 
+			metadata.taskIndex = this.nextTaskIndex++;
+
 			const {args, buildTitle, implementations, rawTitle} = parseTestArgs(testArgs);
 
 			if (this.checkSelectedByLineNumbers) {
@@ -285,7 +288,7 @@ class Runner extends Emittery {
 		return result;
 	}
 
-	async runHooks(tasks, contextRef, titleSuffix, testPassed) {
+	async runHooks(tasks, contextRef, {titleSuffix, testPassed, associatedTaskIndex} = {}) {
 		const hooks = tasks.map(task => new Runnable({
 			contextRef,
 			experiments: this.experiments,
@@ -295,7 +298,7 @@ class Runner extends Emittery {
 				t => task.implementation.apply(null, [t].concat(task.args)),
 			compareTestSnapshot: this.boundCompareTestSnapshot,
 			updateSnapshots: this.updateSnapshots,
-			metadata: task.metadata,
+			metadata: {...task.metadata, associatedTaskIndex},
 			powerAssert: this.powerAssert,
 			title: `${task.title}${titleSuffix || ''}`,
 			isHook: true,
@@ -326,7 +329,14 @@ class Runner extends Emittery {
 
 	async runTest(task, contextRef) {
 		const hookSuffix = ` for ${task.title}`;
-		let hooksOk = await this.runHooks(this.tasks.beforeEach, contextRef, hookSuffix);
+		let hooksOk = await this.runHooks(
+			this.tasks.beforeEach,
+			contextRef,
+			{
+				titleSuffix: hookSuffix,
+				associatedTaskIndex: task.metadata.taskIndex
+			}
+		);
 
 		let testOk = false;
 		if (hooksOk) {
@@ -358,7 +368,14 @@ class Runner extends Emittery {
 					logs: result.logs
 				});
 
-				hooksOk = await this.runHooks(this.tasks.afterEach, contextRef, hookSuffix, testOk);
+				hooksOk = await this.runHooks(
+					this.tasks.afterEach,
+					contextRef,
+					{
+						titleSuffix: hookSuffix,
+						testPassed: testOk,
+						associatedTaskIndex: task.metadata.taskIndex
+					});
 			} else {
 				this.emit('stateChange', {
 					type: 'test-failed',
@@ -372,7 +389,14 @@ class Runner extends Emittery {
 			}
 		}
 
-		const alwaysOk = await this.runHooks(this.tasks.afterEachAlways, contextRef, hookSuffix, testOk);
+		const alwaysOk = await this.runHooks(
+			this.tasks.afterEachAlways,
+			contextRef,
+			{
+				titleSuffix: hookSuffix,
+				testPassed: testOk,
+				associatedTaskIndex: task.metadata.taskIndex
+			});
 		return alwaysOk && hooksOk && testOk;
 	}
 

lib/snapshot-manager.js

@@ -104,13 +104,32 @@ function combineEntries(entries) {
 	const buffers = [];
 	let byteLength = 0;
 
-	const sortedKeys = [...entries.keys()].sort();
+	const sortedKeys = [...entries.keys()].sort((keyA, keyB) => {
+		const [a, b] = [entries.get(keyA), entries.get(keyB)];
+		const taskDifference = a.taskIndex - b.taskIndex;
+
+		if (taskDifference !== 0) {
+			return taskDifference;
+		}
+
+		const [assocA, assocB] = [a.associatedTaskIndex, b.associatedTaskIndex];
+		if (assocA !== undefined && assocB !== undefined) {
+			const assocDifference = assocA - assocB;
+
+			if (assocDifference !== 0) {
+				return assocDifference;
+			}
+		}
+
+		return a.snapIndex - b.snapIndex;
+	});
+
 	for (const key of sortedKeys) {
 		const keyBuffer = Buffer.from(`\n\n## ${key}\n\n`, 'utf8');
 		buffers.push(keyBuffer);
 		byteLength += keyBuffer.byteLength;
 
-		const formattedEntries = entries.get(key);
+		const formattedEntries = entries.get(key).buffers;
 		const last = formattedEntries[formattedEntries.length - 1];
 		for (const entry of formattedEntries) {
 			buffers.push(entry);
@@ -176,10 +195,11 @@ function encodeSnapshots(buffersByHash) {
 	byteOffset += 2;
 
 	const entries = [];
-	for (const pair of buffersByHash) {
-		const hash = pair[0];
-		const snapshotBuffers = pair[1];
-
+	// Maps can't have duplicate keys, so all items in [...buffersByHash.keys()]
+	// are unique, so sortedHashes should be deterministic.
+	const sortedHashes = [...buffersByHash.keys()].sort();
+	const sortedBuffersByHash = [...sortedHashes.map(hash => [hash, buffersByHash.get(hash)])];
+	for (const [hash, snapshotBuffers] of sortedBuffersByHash) {
 		buffers.push(Buffer.from(hash, 'hex'));
 		byteOffset += MD5_HASH_LENGTH;
 
@@ -332,6 +352,7 @@ class Manager {
 		const descriptor = concordance.describe(options.expected, concordanceOptions);
 		const snapshot = concordance.serialize(descriptor);
 		const entry = formatEntry(options.label, descriptor);
+		const {taskIndex, snapIndex, associatedTaskIndex} = options;
 
 		return () => { // Must be called in order!
 			this.hasChanges = true;
@@ -353,9 +374,9 @@ class Manager {
 			snapshots.push(snapshot);
 
 			if (this.reportEntries.has(options.belongsTo)) {
-				this.reportEntries.get(options.belongsTo).push(entry);
+				this.reportEntries.get(options.belongsTo).buffers.push(entry);
 			} else {
-				this.reportEntries.set(options.belongsTo, [entry]);
+				this.reportEntries.set(options.belongsTo, {buffers: [entry], taskIndex, snapIndex, associatedTaskIndex});
 			}
 		};
 	}

lib/test.js

@@ -230,7 +230,17 @@ class Test {
 			const index = id ? 0 : this.nextSnapshotIndex++;
 			const label = id ? '' : message || `Snapshot ${index + 1}`; // Human-readable labels start counting at 1.
 
-			const {record, ...result} = options.compareTestSnapshot({belongsTo, deferRecording, expected, index, label});
+			const {taskIndex, associatedTaskIndex} = this.metadata;
+			const {record, ...result} = options.compareTestSnapshot({
+				belongsTo,
+				deferRecording,
+				expected,
+				index,
+				label,
+				taskIndex,
+				snapIndex: this.snapshotCount,
+				associatedTaskIndex
+			});
 			if (record) {
 				this.deferredSnapshotRecordings.push(record);
 			}

test/snapshot-order/fixtures/intertest-order/package.json

@@ -0,0 +1 @@
+{}

test/snapshot-order/fixtures/intertest-order/test.js

@@ -0,0 +1,35 @@
+const test = require('ava');
+
+const reverse = process.env.INTERTEST_ORDER_REVERSE;
+
+// Functions which resolve the corresponding promise to undefined
+let resolveOne;
+let resolveTwo;
+
+// Promises with triggerable resolution
+const one = new Promise(resolve => {
+	resolveOne = resolve;
+});
+
+const two = new Promise(resolve => {
+	resolveTwo = resolve;
+});
+
+// Test cases which await the triggerable promises
+test('one', async t => {
+	await one;
+	t.snapshot('one');
+	resolveTwo();
+});
+test('two', async t => {
+	await two;
+	t.snapshot('two');
+	resolveOne();
+});
+
+// Start resolution
+if (reverse) {
+	resolveTwo();
+} else {
+	resolveOne();
+}

test/snapshot-order/fixtures/randomness/package.json

@@ -0,0 +1 @@
+{}

test/snapshot-order/fixtures/randomness/test.js

@@ -0,0 +1,67 @@
+const test = require('ava');
+
+const id = index => `index: ${index}`;
+const randomDelay = () => new Promise(resolve => {
+	setTimeout(resolve, Math.random() * 1000);
+});
+
+test.before(async t => {
+	await randomDelay();
+	t.snapshot(id(-2), 'in a before hook');
+});
+
+test.beforeEach(async t => {
+	await randomDelay();
+	t.snapshot(id(-1.5), 'in a beforeEach hook');
+});
+
+test.afterEach(async t => {
+	await randomDelay();
+	t.snapshot(id(-1), 'in an afterEach hook');
+});
+
+test.afterEach.always(async t => {
+	await randomDelay();
+	t.snapshot(id(-0.5), 'in an afterEachAlways hook');
+});
+
+test('B - declare some snapshots', async t => {
+	await randomDelay();
+	t.snapshot(id(0));
+	t.snapshot(id(1), 'has a message');
+	t.snapshot(id(2), 'also has a message');
+	t.snapshot(id(3), {id: 'has an ID'});
+});
+
+test('A - declare some more snapshots', async t => {
+	await randomDelay();
+	t.snapshot(id(4));
+});
+
+test('C - declare some snapshots in a try()', async t => {
+	await randomDelay();
+	t.snapshot(id(5), 'outer');
+	(await t.try('trying', t => {
+		t.snapshot(id(6), 'inner');
+	})).commit();
+	t.snapshot(id(7), 'outer again');
+});
+
+test('E - discard some snapshots in a try()', async t => {
+	await randomDelay();
+	t.snapshot(id(8), 'outer');
+	(await t.try('trying', t => {
+		t.snapshot(id(9), 'inner');
+	})).discard();
+	t.snapshot(id(10), 'outer again');
+});
+
+test('D - more snapshots with IDs', async t => {
+	await randomDelay();
+	t.snapshot(id(11), {id: 'the first in test D'});
+	t.snapshot(id(12));
+	// These have to be reported in reverse declaration order, because they can't
+	// be reported under the same header
+	t.snapshot(id(14), {id: 'the second-to-last in test D'});
+	t.snapshot(id(13));
+});