Skip to content

Deterministic snaphot files, declaration-ordered snapshot reports #2610

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 27 commits into from
Dec 6, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
47a0015
Test .snap is independent of test resolution order
ninevra Nov 19, 2020
6d6e211
Sort snapshots before encoding them
ninevra Nov 15, 2020
bec3485
Move intertest-order fixture to subdirectory
ninevra Nov 21, 2020
2b22330
Minor changes to intertest-order test
ninevra Nov 21, 2020
c2b7dff
Test that reports are sorted in declaration order
ninevra Nov 22, 2020
93bf925
Test report order of snapshots in hooks
ninevra Nov 22, 2020
342ec6b
Sort reports by approximate declaration order
ninevra Nov 20, 2020
69f4d7f
Pass through snapshotCount as a fallback sort key
ninevra Nov 22, 2020
52b4b29
Refactor to include ordering data in entries map
ninevra Nov 22, 2020
4585a11
Polyfill matchAll()
ninevra Nov 22, 2020
cd29d35
Stabilize sort of *Each* hook snapshot reports
ninevra Nov 23, 2020
d6b25db
Test more *Each hooks
ninevra Nov 23, 2020
72d78bb
Update package.json
sindresorhus Nov 23, 2020
5b78ddf
Update package.json
ninevra Nov 27, 2020
8fe455b
Expand abbreviations
ninevra Nov 25, 2020
f15a7a6
Prefer destructuring
ninevra Nov 25, 2020
244cdfc
Fix capitalization
ninevra Nov 25, 2020
c7f63c4
Force not-ci in snapshot-order tests
ninevra Nov 27, 2020
7dcb928
Test behavior with existing unsorted snapshots
ninevra Nov 28, 2020
67bb529
Clean up transient snapshots after tests
ninevra Nov 28, 2020
dcd84dc
Use t.teardown() for cleanup
ninevra Nov 28, 2020
5afacb4
Add a larger test case
ninevra Nov 28, 2020
241b170
Organize tests into separate files
ninevra Nov 28, 2020
ce77f43
Merge branch 'master' of https://github.com/avajs/ava into determinis…
ninevra Nov 30, 2020
9f05271
Prefer writing over copying when files are in memory
ninevra Nov 30, 2020
c2153e9
Add a snapshot message for clarity
ninevra Nov 30, 2020
49ff55f
Remove unmaintainable backwards-compatibility tests
ninevra Dec 1, 2020
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 29 additions & 5 deletions lib/runner.js
Original file line number Diff line number Diff line change
Expand Up @@ -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: [],
Expand Down Expand Up @@ -76,6 +77,8 @@ class Runner extends Emittery {
});
}

metadata.taskIndex = this.nextTaskIndex++;

const {args, buildTitle, implementations, rawTitle} = parseTestArgs(testArgs);

if (this.checkSelectedByLineNumbers) {
Expand Down Expand Up @@ -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,
Expand All @@ -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,
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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',
Expand All @@ -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;
}

Expand Down
37 changes: 29 additions & 8 deletions lib/snapshot-manager.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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;

Expand Down Expand Up @@ -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;
Expand All @@ -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});
}
};
}
Expand Down
12 changes: 11 additions & 1 deletion lib/test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down
1 change: 1 addition & 0 deletions test/snapshot-order/fixtures/intertest-order/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
{}
35 changes: 35 additions & 0 deletions test/snapshot-order/fixtures/intertest-order/test.js
Original file line number Diff line number Diff line change
@@ -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();
}
1 change: 1 addition & 0 deletions test/snapshot-order/fixtures/randomness/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
{}
67 changes: 67 additions & 0 deletions test/snapshot-order/fixtures/randomness/test.js
Original file line number Diff line number Diff line change
@@ -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));
});
Loading