From 2e56fbc3a0e379904496b6f9efcc8f2286a38a31 Mon Sep 17 00:00:00 2001 From: Mark Wubben Date: Sat, 1 Apr 2017 16:05:36 +0100 Subject: [PATCH 1/2] Allow new assertions while waiting for pending ones 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 | 51 ++++++++++++++++++++++++++++++++++++--------------- 2 files changed, 49 insertions(+), 26 deletions(-) diff --git a/lib/test.js b/lib/test.js index 026285cb0..1e2969490 100644 --- a/lib/test.js +++ b/lib/test.js @@ -109,7 +109,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; @@ -152,7 +152,7 @@ class Test { } countPassedAssertion() { - if (this.finishing) { + if (this.finished) { this.saveFirstError(new Error('Assertion passed, but test has already ended')); } @@ -160,7 +160,7 @@ class Test { } addPendingAssertion(promise) { - if (this.finishing) { + if (this.finished) { this.saveFirstError(new Error('Assertion passed, but test has already ended')); } @@ -169,7 +169,7 @@ class Test { } addFailedAssertion(error) { - if (this.finishing) { + if (this.finished) { this.saveFirstError(new Error('Assertion failed, but test has already ended')); } @@ -365,21 +365,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() { @@ -389,6 +387,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 e8111d6b6..f7abcf595 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,32 @@ 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('contextRef', t => { new Test({ contextRef: {context: {foo: 'bar'}}, From d4bc502e9ec9e6a161874111ebbe0e786150ec4e Mon Sep 17 00:00:00 2001 From: Mark Wubben Date: Sun, 2 Apr 2017 14:37:11 +0100 Subject: [PATCH 2/2] fixup! Allow new assertions while waiting for pending ones --- test/test.js | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/test/test.js b/test/test.js index f7abcf595..a3337d9a9 100644 --- a/test/test.js +++ b/test/test.js @@ -667,6 +667,28 @@ test('no crash when adding assertions after the test has ended', t => { }).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'}},