From 22f7eddae970792d9bcaaa7fbad5c2b13f2f38de Mon Sep 17 00:00:00 2001
From: Mark Wubben <mark@novemberborn.net>
Date: Mon, 3 Apr 2017 14:02:18 +0100
Subject: [PATCH] Allow new assertions while waiting for pending ones (#1331)

Whilst not a great way to write tests, users may end up adding more
assertions from inside a promise chain that's passed to the `t.throws()`
or `t.notThrows()` assertions. These should be counted for the plan, and
not fail the test as being extraneous.

Fixes #1327
---
 lib/test.js  | 24 +++++++++--------
 test/test.js | 73 +++++++++++++++++++++++++++++++++++++++++-----------
 2 files changed, 71 insertions(+), 26 deletions(-)

diff --git a/lib/test.js b/lib/test.js
index 9d7f0623ab..18860f9c73 100644
--- a/lib/test.js
+++ b/lib/test.js
@@ -110,7 +110,7 @@ class Test {
 		this.endCallbackFinisher = null;
 		this.finishDueToAttributedError = null;
 		this.finishDueToInactivity = null;
-		this.finishing = false;
+		this.finished = false;
 		this.pendingAssertions = [];
 		this.pendingThrowsAssertion = null;
 		this.planCount = null;
@@ -153,7 +153,7 @@ class Test {
 	}
 
 	countPassedAssertion() {
-		if (this.finishing) {
+		if (this.finished) {
 			this.saveFirstError(new Error('Assertion passed, but test has already ended'));
 		}
 
@@ -161,7 +161,7 @@ class Test {
 	}
 
 	addPendingAssertion(promise) {
-		if (this.finishing) {
+		if (this.finished) {
 			this.saveFirstError(new Error('Assertion passed, but test has already ended'));
 		}
 
@@ -170,7 +170,7 @@ class Test {
 	}
 
 	addFailedAssertion(error) {
-		if (this.finishing) {
+		if (this.finished) {
 			this.saveFirstError(new Error('Assertion failed, but test has already ended'));
 		}
 
@@ -366,21 +366,19 @@ class Test {
 	}
 
 	finish() {
-		this.finishing = true;
-
 		if (!this.assertError && this.pendingThrowsAssertion) {
 			return this.waitForPendingThrowsAssertion();
 		}
 
-		this.verifyPlan();
-		this.verifyAssertions();
-
 		if (this.assertError || this.pendingAssertions.length === 0) {
 			return this.completeFinish();
 		}
 
-		// Finish after potential errors from pending assertions have been consumed.
-		return Promise.all(this.pendingAssertions).then(() => this.completeFinish());
+		// Retry finishing after potential errors from pending assertions have been consumed.
+		const consumedErrors = Promise.all(this.pendingAssertions);
+		// Note that more assertions may be added in the meantime.
+		this.pendingAssertions = [];
+		return consumedErrors.then(() => this.finish());
 	}
 
 	finishPromised() {
@@ -390,6 +388,10 @@ class Test {
 	}
 
 	completeFinish() {
+		this.verifyPlan();
+		this.verifyAssertions();
+
+		this.finished = true;
 		this.duration = globals.now() - this.startedAt;
 
 		let reason = this.assertError;
diff --git a/test/test.js b/test/test.js
index e8111d6b61..a3337d9a98 100644
--- a/test/test.js
+++ b/test/test.js
@@ -591,8 +591,8 @@ test('number of assertions matches t.plan when the test exits, but before all pe
 		result = r;
 	}).run().then(passed => {
 		t.is(passed, false);
-		t.match(result.reason.message, /Assertion passed, but test has already ended/);
-		t.is(result.reason.name, 'Error');
+		t.is(result.reason.assertion, 'plan');
+		t.is(result.reason.operator, '===');
 		t.end();
 	});
 });
@@ -604,35 +604,30 @@ test('number of assertions matches t.plan when the test exits, but before all pe
 		a.throws(delay.reject(10, new Error('foo')), 'foo');
 		a.notThrows(delay(10), 'foo');
 		setTimeout(() => {
-			a.fail();
+			a.pass();
 		}, 5);
 	}, null, r => {
 		result = r;
 	}).run().then(passed => {
 		t.is(passed, false);
-		t.match(result.reason.message, /Assertion failed, but test has already ended/);
-		t.is(result.reason.name, 'Error');
+		t.is(result.reason.assertion, 'plan');
+		t.is(result.reason.operator, '===');
 		t.end();
 	});
 });
 
 test('number of assertions doesn\'t match plan when the test exits, but before all promises resolve another is added', t => {
-	let result;
-	const passed = ava(a => {
+	ava(a => {
 		a.plan(3);
 		a.throws(delay.reject(10, new Error('foo')), 'foo');
 		a.notThrows(delay(10), 'foo');
 		setTimeout(() => {
 			a.throws(Promise.reject(new Error('foo')), 'foo');
 		}, 5);
-	}, null, r => {
-		result = r;
-	}).run();
-
-	t.is(passed, false);
-	t.is(result.reason.assertion, 'plan');
-	t.is(result.reason.operator, '===');
-	t.end();
+	}).run().then(passed => {
+		t.is(passed, true);
+		t.end();
+	});
 });
 
 test('assertions return promises', t => {
@@ -646,6 +641,54 @@ test('assertions return promises', t => {
 	});
 });
 
+// https://github.com/avajs/ava/issues/1330
+test('no crash when adding assertions after the test has ended', t => {
+	t.plan(3);
+
+	ava(a => {
+		a.pass();
+		setTimeout(() => {
+			t.doesNotThrow(() => a.pass());
+		}, 5);
+	}).run();
+
+	ava(a => {
+		a.pass();
+		setTimeout(() => {
+			t.doesNotThrow(() => a.fail());
+		}, 5);
+	}).run();
+
+	ava(a => {
+		a.pass();
+		setTimeout(() => {
+			t.doesNotThrow(() => a.notThrows(Promise.resolve()));
+		}, 5);
+	}).run();
+});
+
+test('can add more pending assertions while waiting for earlier assertions to complete', t => {
+	return ava(a => {
+		a.plan(4);
+
+		a.notThrows(new Promise(resolve => {
+			setImmediate(() => {
+				resolve();
+
+				a.pass();
+				a.throws(new Promise(resolve => {
+					setImmediate(() => {
+						a.pass();
+						resolve();
+					});
+				}));
+			});
+		}));
+	}).run().then(passed => {
+		t.is(passed, false);
+	});
+});
+
 test('contextRef', t => {
 	new Test({
 		contextRef: {context: {foo: 'bar'}},