From c02536fda8ea4882723839912427be5d52e2159b Mon Sep 17 00:00:00 2001 From: Mark Wubben Date: Fri, 17 Mar 2017 10:30:26 +0000 Subject: [PATCH 01/15] Anticipate timers firing early Don't make the duration comparison so precise, since timers may fire early. --- test/test.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/test.js b/test/test.js index 792fde050..93412f917 100644 --- a/test/test.js +++ b/test/test.js @@ -323,7 +323,7 @@ test('record test duration', t => { }, 1234); }).run().then(result => { t.is(result.passed, true); - t.true(result.result.duration >= 1234); + t.true(result.result.duration >= 1000); t.end(); }); }); @@ -339,7 +339,7 @@ test('wait for test to end', t => { t.is(result.passed, true); t.is(result.result.planCount, 1); t.is(result.result.assertCount, 1); - t.true(result.result.duration >= 1234); + t.true(result.result.duration >= 1000); t.end(); }); From 264c816e521b398f6eec72761d58f65eb58a1aa2 Mon Sep 17 00:00:00 2001 From: Mark Wubben Date: Thu, 16 Mar 2017 18:40:35 +0000 Subject: [PATCH 02/15] Remove unused hook class --- lib/hook.js | 22 ---------------------- 1 file changed, 22 deletions(-) delete mode 100644 lib/hook.js diff --git a/lib/hook.js b/lib/hook.js deleted file mode 100644 index 0429df387..000000000 --- a/lib/hook.js +++ /dev/null @@ -1,22 +0,0 @@ -'use strict'; -const Test = require('./test'); - -class Hook { - constructor(title, fn) { - if (typeof title === 'function') { - fn = title; - title = null; - } - - this.title = title; - this.fn = fn; - } - test(testTitle) { - const title = this.title || `${this.metadata.type} for "${testTitle}"`; - const test = new Test(title, this.fn); - test.metadata = this.metadata; - return test; - } -} - -module.exports = Hook; From 24fb3ab3ef3a6e7cf4514be9761f86a603dfdb17 Mon Sep 17 00:00:00 2001 From: Mark Wubben Date: Fri, 17 Mar 2017 10:29:46 +0000 Subject: [PATCH 03/15] Always skip todo tests --- lib/runner.js | 6 ------ lib/test-collection.js | 2 +- 2 files changed, 1 insertion(+), 7 deletions(-) diff --git a/lib/runner.js b/lib/runner.js index fd8e2debf..1e22bb75a 100644 --- a/lib/runner.js +++ b/lib/runner.js @@ -6,8 +6,6 @@ const matcher = require('matcher'); const TestCollection = require('./test-collection'); const validateTest = require('./validate-test'); -function noop() {} - const chainableMethods = { defaults: { type: 'test', @@ -74,10 +72,6 @@ class Runner extends EventEmitter { throw new TypeError(validationError); } - if (opts.todo) { - fn = noop; - } - this.tests.add({ metadata: opts, fn, diff --git a/lib/test-collection.js b/lib/test-collection.js index 4ce5a4aee..8bd39612e 100644 --- a/lib/test-collection.js +++ b/lib/test-collection.js @@ -145,7 +145,7 @@ class TestCollection extends EventEmitter { return test; } _buildTestWithHooks(test) { - if (test.metadata.skipped) { + if (test.metadata.skipped || test.metadata.todo) { return new Sequence([this._skippedTest(this._buildTest(test))], true); } From 0fc8e4d5b972a2aea112527724ca12854f1a4525 Mon Sep 17 00:00:00 2001 From: Mark Wubben Date: Sat, 18 Mar 2017 14:27:23 +0000 Subject: [PATCH 04/15] Clean up test-worker and process-adapter * Clarify responsibilities * Consistently import dependencies * Clarify significance of `exports.avaRequired` * Stop mimicking process with process-adapter. Reference it using `adapter` instead. Use the `process` global where applicable. Masking the process global with a mimicked object is unnecessarily confusing. * Remove superstitious delays in exiting workers The worker now only exits when told by the main process. This means the IPC channel must have drained before the main process can send the instruction. There's no need to wait before sending the message that teardown has completed. The AppVeyor workaround was introduced to solve Node.js 0.10 issues. We're no longer supporting that version. In theory, issues around flushing I/O exist regardless of whether AVA is running in AppVeyor. There is no clear way around this though, so let's assume it's not actually an issue. --- lib/main.js | 14 +-- lib/process-adapter.js | 26 ++-- lib/test-worker.js | 117 ++++++++---------- test/fixture/trigger-worker-exception/hack.js | 11 +- 4 files changed, 74 insertions(+), 94 deletions(-) diff --git a/lib/main.js b/lib/main.js index e56b94d8c..9a5da627e 100644 --- a/lib/main.js +++ b/lib/main.js @@ -1,9 +1,9 @@ 'use strict'; -const process = require('./process-adapter'); +const worker = require('./test-worker'); +const adapter = require('./process-adapter'); const serializeError = require('./serialize-error'); const globals = require('./globals'); const Runner = require('./runner'); -const send = process.send; const opts = globals.options; const runner = new Runner({ @@ -13,7 +13,7 @@ const runner = new Runner({ }); // Note that test files have require('ava') -require('./test-worker').avaRequired = true; +worker.avaRequired = true; // If fail-fast is enabled, use this variable to detect // that no more tests should be logged @@ -39,7 +39,7 @@ function test(props) { props.error = null; } - send('test', props); + adapter.send('test', props); if (hasError && opts.failFast) { isFailed = true; @@ -50,7 +50,7 @@ function test(props) { function exit() { const stats = runner._buildStats(); - send('results', {stats}); + adapter.send('results', {stats}); } globals.setImmediate(() => { @@ -58,11 +58,11 @@ globals.setImmediate(() => { const numberOfTests = runner.tests.testCount; if (numberOfTests === 0) { - send('no-tests', {avaRequired: true}); + adapter.send('no-tests', {avaRequired: true}); return; } - send('stats', { + adapter.send('stats', { testCount: numberOfTests, hasExclusive }); diff --git a/lib/process-adapter.js b/lib/process-adapter.js index 17babf98d..2959c3c36 100644 --- a/lib/process-adapter.js +++ b/lib/process-adapter.js @@ -1,23 +1,18 @@ 'use strict'; const fs = require('fs'); const path = require('path'); -const chalk = require('chalk'); +const debug = require('debug')('ava'); const sourceMapSupport = require('source-map-support'); const installPrecompiler = require('require-precompiled'); -const debug = require('debug')('ava'); - -// Check if the test is being run without AVA cli -const isForked = typeof process.send === 'function'; - -if (!isForked) { - const fp = path.relative('.', process.argv[1]); - - console.log(); - console.error('Test files must be run with the AVA CLI:\n\n ' + chalk.grey.dim('$') + ' ' + chalk.cyan('ava ' + fp) + '\n'); +// Parse and re-emit AVA messages +process.on('message', message => { + if (!message.ava) { + return; + } - process.exit(1); // eslint-disable-line unicorn/no-process-exit -} + process.emit(message.name, message.data); +}); exports.send = (name, data) => { process.send({ @@ -27,11 +22,6 @@ exports.send = (name, data) => { }); }; -exports.on = process.on.bind(process); -exports.emit = process.emit.bind(process); -exports.exit = process.exit.bind(process); -exports.env = process.env; - const opts = JSON.parse(process.argv[2]); exports.opts = opts; diff --git a/lib/test-worker.js b/lib/test-worker.js index d9c7309b2..823dfbd2f 100644 --- a/lib/test-worker.js +++ b/lib/test-worker.js @@ -1,38 +1,56 @@ 'use strict'; -/* eslint-disable import/order */ -const isObj = require('is-obj'); -const process = require('./process-adapter'); -const opts = process.opts; -const testPath = opts.file; +// Check if the test is being run without AVA cli +{ + /* eslint-disable import/order */ + const path = require('path'); + const chalk = require('chalk'); + + const isForked = typeof process.send === 'function'; + if (!isForked) { + const fp = path.relative('.', process.argv[1]); + + console.log(); + console.error('Test files must be run with the AVA CLI:\n\n ' + chalk.grey.dim('$') + ' ' + chalk.cyan('ava ' + fp) + '\n'); + + process.exit(1); // eslint-disable-line unicorn/no-process-exit + } +} -// Bind globals first before anything has a chance to interfere +/* eslint-enable import/order */ +const Bluebird = require('bluebird'); +const currentlyUnhandled = require('currently-unhandled')(); +const isObj = require('is-obj'); +const adapter = require('./process-adapter'); const globals = require('./globals'); +const serializeError = require('./serialize-error'); +const throwsHelper = require('./throws-helper'); + +const opts = adapter.opts; +const testPath = opts.file; globals.options = opts; -const Promise = require('bluebird'); // Bluebird specific -Promise.longStackTraces(); +Bluebird.longStackTraces(); (opts.require || []).forEach(require); -process.installSourceMapSupport(); +adapter.installSourceMapSupport(); +adapter.installPrecompilerHook(); -const currentlyUnhandled = require('currently-unhandled')(); -const serializeError = require('./serialize-error'); -const send = process.send; -const throwsHelper = require('./throws-helper'); +const dependencies = []; +adapter.installDependencyTracking(dependencies, testPath); // Check if test files required ava and show error, when they didn't exports.avaRequired = false; -process.installPrecompilerHook(); - -const dependencies = []; -process.installDependencyTracking(dependencies, testPath); - require(testPath); // eslint-disable-line import/no-dynamic-require +// If AVA was not required, show an error +if (!exports.avaRequired) { + adapter.send('no-tests', {avaRequired: false}); +} + process.on('unhandledRejection', throwsHelper); process.on('uncaughtException', exception => { @@ -51,30 +69,7 @@ process.on('uncaughtException', exception => { stack: err.stack }; } - send('uncaughtException', {exception: serialized}); -}); - -// If AVA was not required, show an error -if (!exports.avaRequired) { - send('no-tests', {avaRequired: false}); -} - -// Parse and re-emit AVA messages -process.on('message', message => { - if (!message.ava) { - return; - } - - process.emit(message.name, message.data); -}); - -process.on('ava-exit', () => { - // Use a little delay when running on AppVeyor (because it's shit) - const delay = process.env.AVA_APPVEYOR ? 100 : 0; - - globals.setTimeout(() => { - process.exit(0); // eslint-disable-line xo/no-process-exit - }, delay); + adapter.send('uncaughtException', {exception: serialized}); }); let tearingDown = false; @@ -87,28 +82,26 @@ process.on('ava-teardown', () => { let rejections = currentlyUnhandled(); - if (rejections.length === 0) { - exit(); - return; + if (rejections.length > 0) { + rejections = rejections.map(rejection => { + let reason = rejection.reason; + if (!isObj(reason) || typeof reason.message !== 'string') { + reason = { + message: String(reason) + }; + } + return serializeError(reason); + }); + + adapter.send('unhandledRejections', {rejections}); } - rejections = rejections.map(rejection => { - let reason = rejection.reason; - if (!isObj(reason) || typeof reason.message !== 'string') { - reason = { - message: String(reason) - }; - } - return serializeError(reason); - }); - - send('unhandledRejections', {rejections}); - globals.setTimeout(exit, 100); -}); - -function exit() { // Include dependencies in the final teardown message. This ensures the full // set of dependencies is included no matter how the process exits, unless // it flat out crashes. - send('teardown', {dependencies}); -} + adapter.send('teardown', {dependencies}); +}); + +process.on('ava-exit', () => { + process.exit(0); // eslint-disable-line xo/no-process-exit +}); diff --git a/test/fixture/trigger-worker-exception/hack.js b/test/fixture/trigger-worker-exception/hack.js index 5db3daf4b..8ca645c37 100644 --- a/test/fixture/trigger-worker-exception/hack.js +++ b/test/fixture/trigger-worker-exception/hack.js @@ -1,20 +1,17 @@ 'use strict'; -require('../../../lib/serialize-error'); +const StackUtils = require('stack-utils'); -const serializeModule = require.cache[require.resolve('../../../lib/serialize-error')]; - -const original = serializeModule.exports; +const original = StackUtils.prototype.parseLine; let restored = false; let restoreAfterFirstCall = false; -serializeModule.exports = error => { +StackUtils.prototype.parseLine = function(line) { if (restored) { - return original(error); + return original.call(this, line); } if (restoreAfterFirstCall) { restored = true; } - throw new Error('Forced error'); }; From 602d16f5fe01c8788bb5617647af6055ad457bc8 Mon Sep 17 00:00:00 2001 From: Mark Wubben Date: Mon, 20 Mar 2017 12:54:03 +0000 Subject: [PATCH 05/15] Don't leak internal AssertionErrors to user code Asynchronous `t.throws()` / `t.notThrows()` was the only case where internal AssertionErrors were leaked to user code. Return `undefined` instead. --- lib/assert.js | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/lib/assert.js b/lib/assert.js index ffe803b05..640e6c4d7 100644 --- a/lib/assert.js +++ b/lib/assert.js @@ -166,9 +166,10 @@ function wrapAssertions(callbacks) { }; if (promise) { - const result = promise.then(makeNoop, makeRethrow).then(test); - pending(this, result); - return result; + const intermediate = promise.then(makeNoop, makeRethrow).then(test); + pending(this, intermediate); + // Don't reject the returned promise, even if the assertion fails. + return intermediate.catch(noop); } try { @@ -208,10 +209,10 @@ function wrapAssertions(callbacks) { }; if (promise) { - const result = promise - .then(noop, reason => test(makeRethrow(reason))); - pending(this, result); - return result; + const intermediate = promise.then(noop, reason => test(makeRethrow(reason))); + pending(this, intermediate); + // Don't reject the returned promise, even if the assertion fails. + return intermediate.catch(noop); } try { From 702fd5db24775336355b51ac9808d96fd6f9e888 Mon Sep 17 00:00:00 2001 From: Mark Wubben Date: Mon, 20 Mar 2017 14:36:55 +0000 Subject: [PATCH 06/15] Pass bail option to TestCollection constructor Simplify Runner by passing options to TestCollection. --- lib/runner.js | 7 ++++--- lib/test-collection.js | 9 +++++---- test/test-collection.js | 32 ++++++++++++++++---------------- 3 files changed, 25 insertions(+), 23 deletions(-) diff --git a/lib/runner.js b/lib/runner.js index 1e22bb75a..839100813 100644 --- a/lib/runner.js +++ b/lib/runner.js @@ -46,9 +46,10 @@ class Runner extends EventEmitter { options = options || {}; this.results = []; - this.tests = new TestCollection(); + this.tests = new TestCollection({ + bail: options.bail + }); this.hasStarted = false; - this._bail = options.bail; this._serial = options.serial; this._match = options.match || []; this._addTestResult = this._addTestResult.bind(this); @@ -144,7 +145,7 @@ class Runner extends EventEmitter { this.hasStarted = true; - return Promise.resolve(this.tests.build(this._bail).run()).then(this._buildStats); + return Promise.resolve(this.tests.build().run()).then(this._buildStats); } } diff --git a/lib/test-collection.js b/lib/test-collection.js index 8bd39612e..c0ac1a771 100644 --- a/lib/test-collection.js +++ b/lib/test-collection.js @@ -6,9 +6,10 @@ const Sequence = require('./sequence'); const Test = require('./test'); class TestCollection extends EventEmitter { - constructor() { + constructor(options) { super(); + this.bail = options.bail; this.hasExclusive = false; this.testCount = 0; @@ -164,12 +165,12 @@ class TestCollection extends EventEmitter { _buildTests(tests) { return tests.map(test => this._buildTestWithHooks(test)); } - build(bail) { + build() { const beforeHooks = new Sequence(this._buildHooks(this.hooks.before)); const afterHooks = new Sequence(this._buildHooks(this.hooks.after)); - const serialTests = new Sequence(this._buildTests(this.tests.serial), bail); - const concurrentTests = new Concurrent(this._buildTests(this.tests.concurrent), bail); + const serialTests = new Sequence(this._buildTests(this.tests.serial), this.bail); + const concurrentTests = new Concurrent(this._buildTests(this.tests.concurrent), this.bail); const allTests = new Sequence([serialTests, concurrentTests]); let finalTests = new Sequence([beforeHooks, allTests, afterHooks], true); diff --git a/test/test-collection.js b/test/test-collection.js index b68657242..ad3566727 100644 --- a/test/test-collection.js +++ b/test/test-collection.js @@ -78,7 +78,7 @@ function serialize(collection) { } test('throws if no type is supplied', t => { - const collection = new TestCollection(); + const collection = new TestCollection({}); t.throws(() => { collection.add({ title: 'someTitle', @@ -89,7 +89,7 @@ test('throws if no type is supplied', t => { }); test('throws if you try to set a hook as exclusive', t => { - const collection = new TestCollection(); + const collection = new TestCollection({}); t.throws(() => { collection.add(mockTest({ type: 'beforeEach', @@ -100,7 +100,7 @@ test('throws if you try to set a hook as exclusive', t => { }); test('throws if you try to set a before hook as always', t => { - const collection = new TestCollection(); + const collection = new TestCollection({}); t.throws(() => { collection.add(mockTest({ type: 'before', @@ -111,7 +111,7 @@ test('throws if you try to set a before hook as always', t => { }); test('throws if you try to set a test as always', t => { - const collection = new TestCollection(); + const collection = new TestCollection({}); t.throws(() => { collection.add(mockTest({always: true})); }, {message: '"always" can only be used with after and afterEach hooks'}); @@ -119,7 +119,7 @@ test('throws if you try to set a test as always', t => { }); test('hasExclusive is set when an exclusive test is added', t => { - const collection = new TestCollection(); + const collection = new TestCollection({}); t.false(collection.hasExclusive); collection.add(mockTest({exclusive: true}, 'foo')); t.true(collection.hasExclusive); @@ -127,7 +127,7 @@ test('hasExclusive is set when an exclusive test is added', t => { }); test('adding a concurrent test', t => { - const collection = new TestCollection(); + const collection = new TestCollection({}); collection.add(mockTest({}, 'foo')); t.strictDeepEqual(serialize(collection), { tests: { @@ -138,7 +138,7 @@ test('adding a concurrent test', t => { }); test('adding a serial test', t => { - const collection = new TestCollection(); + const collection = new TestCollection({}); collection.add(mockTest({serial: true}, 'bar')); t.strictDeepEqual(serialize(collection), { tests: { @@ -149,7 +149,7 @@ test('adding a serial test', t => { }); test('adding a before test', t => { - const collection = new TestCollection(); + const collection = new TestCollection({}); collection.add(mockTest({type: 'before'}, 'baz')); t.strictDeepEqual(serialize(collection), { hooks: { @@ -160,7 +160,7 @@ test('adding a before test', t => { }); test('adding a beforeEach test', t => { - const collection = new TestCollection(); + const collection = new TestCollection({}); collection.add(mockTest({type: 'beforeEach'}, 'foo')); t.strictDeepEqual(serialize(collection), { hooks: { @@ -171,7 +171,7 @@ test('adding a beforeEach test', t => { }); test('adding a after test', t => { - const collection = new TestCollection(); + const collection = new TestCollection({}); collection.add(mockTest({type: 'after'}, 'bar')); t.strictDeepEqual(serialize(collection), { hooks: { @@ -182,7 +182,7 @@ test('adding a after test', t => { }); test('adding a after.always test', t => { - const collection = new TestCollection(); + const collection = new TestCollection({}); collection.add(mockTest({ type: 'after', always: true @@ -196,7 +196,7 @@ test('adding a after.always test', t => { }); test('adding a afterEach test', t => { - const collection = new TestCollection(); + const collection = new TestCollection({}); collection.add(mockTest({type: 'afterEach'}, 'baz')); t.strictDeepEqual(serialize(collection), { hooks: { @@ -207,7 +207,7 @@ test('adding a afterEach test', t => { }); test('adding a afterEach.always test', t => { - const collection = new TestCollection(); + const collection = new TestCollection({}); collection.add(mockTest({ type: 'afterEach', always: true @@ -221,7 +221,7 @@ test('adding a afterEach.always test', t => { }); test('adding a bunch of different types', t => { - const collection = new TestCollection(); + const collection = new TestCollection({}); collection.add(mockTest({}, 'a')); collection.add(mockTest({}, 'b')); collection.add(mockTest({serial: true}, 'c')); @@ -240,7 +240,7 @@ test('adding a bunch of different types', t => { }); test('foo', t => { - const collection = new TestCollection(); + const collection = new TestCollection({}); const log = []; function logger(a) { @@ -302,7 +302,7 @@ test('foo', t => { }); test('foo', t => { - const collection = new TestCollection(); + const collection = new TestCollection({}); const log = []; function logger(result) { From c9c45983fc2ed5996350114a6e39cc9dc987c5f6 Mon Sep 17 00:00:00 2001 From: Mark Wubben Date: Thu, 16 Mar 2017 15:56:10 +0000 Subject: [PATCH 07/15] Simplify Concurrent and Sequence There's no need to collect all results. Tests emit them directly. --- lib/concurrent.js | 100 +++--- lib/sequence.js | 99 ++---- test/concurrent.js | 840 +++++++++++++++++++++------------------------ test/sequence.js | 826 ++++++++++++++++++++------------------------ 4 files changed, 846 insertions(+), 1019 deletions(-) diff --git a/lib/concurrent.js b/lib/concurrent.js index 70d0b40f6..872931025 100644 --- a/lib/concurrent.js +++ b/lib/concurrent.js @@ -1,82 +1,64 @@ 'use strict'; -const Promise = require('bluebird'); const isPromise = require('is-promise'); -const autoBind = require('auto-bind'); -const AvaError = require('./ava-error'); class Concurrent { - constructor(tests, bail) { - if (!Array.isArray(tests)) { - throw new TypeError('Expected an array of tests'); + constructor(runnables, bail) { + if (!Array.isArray(runnables)) { + throw new TypeError('Expected an array of runnables'); } - this.results = []; - this.passed = true; - this.reason = null; - this.tests = tests; + this.runnables = runnables; this.bail = bail || false; - - autoBind(this); } + run() { - let results; + let allPassed = true; - try { - results = this.tests.map(this._runTest); - } catch (err) { - if (err instanceof AvaError) { - return this._results(); + let pending; + let rejectPending; + let resolvePending; + const allPromises = []; + const handlePromise = promise => { + if (!pending) { + pending = new Promise((resolve, reject) => { + rejectPending = reject; + resolvePending = resolve; + }); } - throw err; - } - - const isAsync = results.some(isPromise); - - if (isAsync) { - return Promise.all(results) - .catch(AvaError, () => {}) - .then(this._results); - } - - return this._results(); - } - _runTest(test, index) { - const result = test.run(); + allPromises.push(promise.then(result => { + if (!result.passed) { + allPassed = false; - if (isPromise(result)) { - return result.then(result => this._addResult(result, index)); - } + if (this.bail) { + // Stop if the test failed and bail mode is on. + resolvePending(); + } + } + }, rejectPending)); + }; - return this._addResult(result, index); - } - _addResult(result, index) { - // Always save result when not in bail mode or all previous tests pass - if ((this.bail && this.passed) || !this.bail) { - this.results[index] = result; - } + for (const runnable of this.runnables) { + const result = runnable.run(); - if (result.passed === false) { - this.passed = false; + if (isPromise(result)) { + handlePromise(result); + } else if (!result.passed) { + if (this.bail) { + // Stop if the test failed and bail mode is on. + return {passed: false}; + } - // Only set reason once - if (!this.reason) { - this.reason = result.reason; + allPassed = false; } + } - if (this.bail) { - throw new AvaError('Error in Concurrent while in bail mode'); - } + if (pending) { + Promise.all(allPromises).then(resolvePending); + return pending.then(() => ({passed: allPassed})); } - return result; - } - _results() { - return { - passed: this.passed, - reason: this.reason, - result: this.results - }; + return {passed: allPassed}; } } diff --git a/lib/sequence.js b/lib/sequence.js index 2dacbdf18..75eca2da7 100644 --- a/lib/sequence.js +++ b/lib/sequence.js @@ -1,86 +1,59 @@ 'use strict'; const isPromise = require('is-promise'); -const autoBind = require('auto-bind'); -const AvaError = require('./ava-error'); class Sequence { - constructor(tests, bail) { - if (!tests) { - throw new Error('Sequence items can\'t be undefined'); + constructor(runnables, bail) { + if (!Array.isArray(runnables)) { + throw new TypeError('Expected an array of runnables'); } - this.results = []; - this.passed = true; - this.reason = null; - this.tests = tests; + this.runnables = runnables; this.bail = bail || false; - - autoBind(this); } + run() { - const length = this.tests.length; + const iterator = this.runnables[Symbol.iterator](); - for (let i = 0; i < length; i++) { - // If last item failed and we should bail, return results and stop - if (this.bail && !this.passed) { - return this._results(); - } + let allPassed = true; + const runNext = () => { + let promise; - const result = this.tests[i].run(); + for (let next = iterator.next(); !next.done; next = iterator.next()) { + const result = next.value.run(); + if (isPromise(result)) { + promise = result; + break; + } - // If a Promise returned, we don't need to check for Promises after this test - // so we can just use Promise.each() on the rest of the tests - if (isPromise(result)) { - return result - .then(this._addResult) - .return(this.tests.slice(i + 1)) - .each(this._runTest) - .catch(AvaError, () => {}) - .then(this._results); - } + if (!result.passed) { + allPassed = false; - try { - this._addResult(result); - } catch (err) { - // In bail mode, don't execute the next tests - if (err instanceof AvaError) { - return this._results(); + if (this.bail) { + // Stop if the test failed and bail mode is on. + break; + } } - - throw err; } - } - return this._results(); - } - _runTest(test) { - const result = test.run(); - return isPromise(result) ? result.then(this._addResult) : this._addResult(result); - } - _addResult(result) { - this.results.push(result); - - if (result.passed === false) { - this.passed = false; - - // Only set reason once - if (!this.reason) { - this.reason = result.reason; + if (!promise) { + return {passed: allPassed}; } - if (this.bail) { - throw new AvaError('Error in Sequence while in bail mode'); - } - } + return promise.then(result => { + if (!result.passed) { + allPassed = false; - return result; - } - _results() { - return { - passed: this.passed, - reason: this.reason, - result: this.results + if (this.bail) { + // Stop if the test failed and bail mode is on. + return {passed: false}; + } + } + + return runNext(); + }); }; + + return runNext(); } } diff --git a/test/concurrent.js b/test/concurrent.js index aa71c0f1e..5da32354c 100644 --- a/test/concurrent.js +++ b/test/concurrent.js @@ -1,14 +1,31 @@ 'use strict'; -const test = require('tap').test; +const tap = require('tap'); +const isPromise = require('is-promise'); const Concurrent = require('../lib/concurrent'); +let results = []; +const test = (name, fn) => { + tap.test(name, t => { + results = []; + return fn(t); + }); +}; +function collect(result) { + if (isPromise(result)) { + return result.then(collect); + } + + results.push(result); + return result; +} + function pass(val) { return { run() { - return { + return collect({ passed: true, result: val - }; + }); } }; } @@ -16,10 +33,10 @@ function pass(val) { function fail(val) { return { run() { - return { + return collect({ passed: false, reason: val - }; + }); } }; } @@ -35,10 +52,10 @@ function failWithTypeError() { function passAsync(val) { return { run() { - return Promise.resolve({ + return collect(Promise.resolve({ passed: true, result: val - }); + })); } }; } @@ -46,10 +63,10 @@ function passAsync(val) { function failAsync(err) { return { run() { - return Promise.resolve({ + return collect(Promise.resolve({ // eslint-disable-line promise/no-promise-in-callback passed: false, reason: err - }); + })); } }; } @@ -72,24 +89,21 @@ test('all sync - all pass - no bail', t => { false ).run(); - t.strictDeepEqual(result, { - passed: true, - reason: null, - result: [ - { - passed: true, - result: 'a' - }, - { - passed: true, - result: 'b' - }, - { - passed: true, - result: 'c' - } - ] - }); + t.strictDeepEqual(result, {passed: true}); + t.strictDeepEqual(results, [ + { + passed: true, + result: 'a' + }, + { + passed: true, + result: 'b' + }, + { + passed: true, + result: 'c' + } + ]); t.end(); }); @@ -103,24 +117,21 @@ test('all sync - no failure - bail', t => { true ).run(); - t.strictDeepEqual(result, { - passed: true, - reason: null, - result: [ - { - passed: true, - result: 'a' - }, - { - passed: true, - result: 'b' - }, - { - passed: true, - result: 'c' - } - ] - }); + t.strictDeepEqual(result, {passed: true}); + t.strictDeepEqual(results, [ + { + passed: true, + result: 'a' + }, + { + passed: true, + result: 'b' + }, + { + passed: true, + result: 'c' + } + ]); t.end(); }); @@ -134,24 +145,21 @@ test('all sync - begin failure - no bail', t => { false ).run(); - t.strictDeepEqual(result, { - passed: false, - reason: 'a', - result: [ - { - passed: false, - reason: 'a' - }, - { - passed: true, - result: 'b' - }, - { - passed: true, - result: 'c' - } - ] - }); + t.strictDeepEqual(result, {passed: false}); + t.strictDeepEqual(results, [ + { + passed: false, + reason: 'a' + }, + { + passed: true, + result: 'b' + }, + { + passed: true, + result: 'c' + } + ]); t.end(); }); @@ -165,24 +173,21 @@ test('all sync - mid failure - no bail', t => { false ).run(); - t.strictDeepEqual(result, { - passed: false, - reason: 'b', - result: [ - { - passed: true, - result: 'a' - }, - { - passed: false, - reason: 'b' - }, - { - passed: true, - result: 'c' - } - ] - }); + t.strictDeepEqual(result, {passed: false}); + t.strictDeepEqual(results, [ + { + passed: true, + result: 'a' + }, + { + passed: false, + reason: 'b' + }, + { + passed: true, + result: 'c' + } + ]); t.end(); }); @@ -196,24 +201,21 @@ test('all sync - end failure - no bail', t => { false ).run(); - t.strictDeepEqual(result, { - passed: false, - reason: 'c', - result: [ - { - passed: true, - result: 'a' - }, - { - passed: true, - result: 'b' - }, - { - passed: false, - reason: 'c' - } - ] - }); + t.strictDeepEqual(result, {passed: false}); + t.strictDeepEqual(results, [ + { + passed: true, + result: 'a' + }, + { + passed: true, + result: 'b' + }, + { + passed: false, + reason: 'c' + } + ]); t.end(); }); @@ -227,24 +229,21 @@ test('all sync - multiple failure - no bail', t => { false ).run(); - t.strictDeepEqual(result, { - passed: false, - reason: 'a', - result: [ - { - passed: false, - reason: 'a' - }, - { - passed: true, - result: 'b' - }, - { - passed: false, - reason: 'c' - } - ] - }); + t.strictDeepEqual(result, {passed: false}); + t.strictDeepEqual(results, [ + { + passed: false, + reason: 'a' + }, + { + passed: true, + result: 'b' + }, + { + passed: false, + reason: 'c' + } + ]); t.end(); }); @@ -258,16 +257,13 @@ test('all sync - begin failure - bail', t => { true ).run(); - t.strictDeepEqual(result, { - passed: false, - reason: 'a', - result: [ - { - passed: false, - reason: 'a' - } - ] - }); + t.strictDeepEqual(result, {passed: false}); + t.strictDeepEqual(results, [ + { + passed: false, + reason: 'a' + } + ]); t.end(); }); @@ -281,20 +277,17 @@ test('all sync - mid failure - bail', t => { true ).run(); - t.strictDeepEqual(result, { - passed: false, - reason: 'b', - result: [ - { - passed: true, - result: 'a' - }, - { - passed: false, - reason: 'b' - } - ] - }); + t.strictDeepEqual(result, {passed: false}); + t.strictDeepEqual(results, [ + { + passed: true, + result: 'a' + }, + { + passed: false, + reason: 'b' + } + ]); t.end(); }); @@ -308,24 +301,21 @@ test('all sync - end failure - bail', t => { true ).run(); - t.strictDeepEqual(result, { - passed: false, - reason: 'c', - result: [ - { - passed: true, - result: 'a' - }, - { - passed: true, - result: 'b' - }, - { - passed: false, - reason: 'c' - } - ] - }); + t.strictDeepEqual(result, {passed: false}); + t.strictDeepEqual(results, [ + { + passed: true, + result: 'a' + }, + { + passed: true, + result: 'b' + }, + { + passed: false, + reason: 'c' + } + ]); t.end(); }); @@ -338,24 +328,21 @@ test('all async - no failure - no bail', t => { ], false ).run().then(result => { - t.strictDeepEqual(result, { - passed: true, - reason: null, - result: [ - { - passed: true, - result: 'a' - }, - { - passed: true, - result: 'b' - }, - { - passed: true, - result: 'c' - } - ] - }); + t.strictDeepEqual(result, {passed: true}); + t.strictDeepEqual(results, [ + { + passed: true, + result: 'a' + }, + { + passed: true, + result: 'b' + }, + { + passed: true, + result: 'c' + } + ]); }); }); @@ -368,24 +355,21 @@ test('all async - no failure - bail', t => { ], true ).run().then(result => { - t.strictDeepEqual(result, { - passed: true, - reason: null, - result: [ - { - passed: true, - result: 'a' - }, - { - passed: true, - result: 'b' - }, - { - passed: true, - result: 'c' - } - ] - }); + t.strictDeepEqual(result, {passed: true}); + t.strictDeepEqual(results, [ + { + passed: true, + result: 'a' + }, + { + passed: true, + result: 'b' + }, + { + passed: true, + result: 'c' + } + ]); }); }); @@ -398,24 +382,21 @@ test('last async - no failure - no bail', t => { ], false ).run().then(result => { - t.strictDeepEqual(result, { - passed: true, - reason: null, - result: [ - { - passed: true, - result: 'a' - }, - { - passed: true, - result: 'b' - }, - { - passed: true, - result: 'c' - } - ] - }); + t.strictDeepEqual(result, {passed: true}); + t.strictDeepEqual(results, [ + { + passed: true, + result: 'a' + }, + { + passed: true, + result: 'b' + }, + { + passed: true, + result: 'c' + } + ]); }); }); @@ -428,24 +409,21 @@ test('mid async - no failure - no bail', t => { ], false ).run().then(result => { - t.strictDeepEqual(result, { - passed: true, - reason: null, - result: [ - { - passed: true, - result: 'a' - }, - { - passed: true, - result: 'b' - }, - { - passed: true, - result: 'c' - } - ] - }); + t.strictDeepEqual(result, {passed: true}); + t.strictDeepEqual(results, [ + { + passed: true, + result: 'a' + }, + { + passed: true, + result: 'c' + }, + { + passed: true, + result: 'b' + } + ]); }); }); @@ -458,24 +436,21 @@ test('first async - no failure - no bail', t => { ], false ).run().then(result => { - t.strictDeepEqual(result, { - passed: true, - reason: null, - result: [ - { - passed: true, - result: 'a' - }, - { - passed: true, - result: 'b' - }, - { - passed: true, - result: 'c' - } - ] - }); + t.strictDeepEqual(result, {passed: true}); + t.strictDeepEqual(results, [ + { + passed: true, + result: 'b' + }, + { + passed: true, + result: 'c' + }, + { + passed: true, + result: 'a' + } + ]); }); }); @@ -488,24 +463,21 @@ test('last async - no failure - bail', t => { ], true ).run().then(result => { - t.strictDeepEqual(result, { - passed: true, - reason: null, - result: [ - { - passed: true, - result: 'a' - }, - { - passed: true, - result: 'b' - }, - { - passed: true, - result: 'c' - } - ] - }); + t.strictDeepEqual(result, {passed: true}); + t.strictDeepEqual(results, [ + { + passed: true, + result: 'a' + }, + { + passed: true, + result: 'b' + }, + { + passed: true, + result: 'c' + } + ]); }); }); @@ -518,24 +490,21 @@ test('mid async - no failure - bail', t => { ], true ).run().then(result => { - t.strictDeepEqual(result, { - passed: true, - reason: null, - result: [ - { - passed: true, - result: 'a' - }, - { - passed: true, - result: 'b' - }, - { - passed: true, - result: 'c' - } - ] - }); + t.strictDeepEqual(result, {passed: true}); + t.strictDeepEqual(results, [ + { + passed: true, + result: 'a' + }, + { + passed: true, + result: 'c' + }, + { + passed: true, + result: 'b' + } + ]); }); }); @@ -548,24 +517,21 @@ test('first async - no failure - bail', t => { ], true ).run().then(result => { - t.strictDeepEqual(result, { - passed: true, - reason: null, - result: [ - { - passed: true, - result: 'a' - }, - { - passed: true, - result: 'b' - }, - { - passed: true, - result: 'c' - } - ] - }); + t.strictDeepEqual(result, {passed: true}); + t.strictDeepEqual(results, [ + { + passed: true, + result: 'b' + }, + { + passed: true, + result: 'c' + }, + { + passed: true, + result: 'a' + } + ]); }); }); @@ -578,16 +544,21 @@ test('all async - begin failure - bail', t => { ], true ).run().then(result => { - t.strictDeepEqual(result, { - passed: false, - reason: 'a', - result: [ - { - passed: false, - reason: 'a' - } - ] - }); + t.strictDeepEqual(result, {passed: false}); + t.strictDeepEqual(results, [ + { + passed: false, + reason: 'a' + }, + { + passed: true, + result: 'b' + }, + { + passed: true, + result: 'c' + } + ]); }); }); @@ -600,20 +571,21 @@ test('all async - mid failure - bail', t => { ], true ).run().then(result => { - t.strictDeepEqual(result, { - passed: false, - reason: 'b', - result: [ - { - passed: true, - result: 'a' - }, - { - passed: false, - reason: 'b' - } - ] - }); + t.strictDeepEqual(result, {passed: false}); + t.strictDeepEqual(results, [ + { + passed: true, + result: 'a' + }, + { + passed: false, + reason: 'b' + }, + { + passed: true, + result: 'c' + } + ]); }); }); @@ -626,24 +598,21 @@ test('all async - end failure - bail', t => { ], true ).run().then(result => { - t.strictDeepEqual(result, { - passed: false, - reason: 'c', - result: [ - { - passed: true, - result: 'a' - }, - { - passed: true, - result: 'b' - }, - { - passed: false, - reason: 'c' - } - ] - }); + t.strictDeepEqual(result, {passed: false}); + t.strictDeepEqual(results, [ + { + passed: true, + result: 'a' + }, + { + passed: true, + result: 'b' + }, + { + passed: false, + reason: 'c' + } + ]); }); }); @@ -656,24 +625,21 @@ test('all async - begin failure - no bail', t => { ], false ).run().then(result => { - t.strictDeepEqual(result, { - passed: false, - reason: 'a', - result: [ - { - passed: false, - reason: 'a' - }, - { - passed: true, - result: 'b' - }, - { - passed: true, - result: 'c' - } - ] - }); + t.strictDeepEqual(result, {passed: false}); + t.strictDeepEqual(results, [ + { + passed: false, + reason: 'a' + }, + { + passed: true, + result: 'b' + }, + { + passed: true, + result: 'c' + } + ]); }); }); @@ -686,24 +652,21 @@ test('all async - mid failure - no bail', t => { ], false ).run().then(result => { - t.strictDeepEqual(result, { - passed: false, - reason: 'b', - result: [ - { - passed: true, - result: 'a' - }, - { - passed: false, - reason: 'b' - }, - { - passed: true, - result: 'c' - } - ] - }); + t.strictDeepEqual(result, {passed: false}); + t.strictDeepEqual(results, [ + { + passed: true, + result: 'a' + }, + { + passed: false, + reason: 'b' + }, + { + passed: true, + result: 'c' + } + ]); }); }); @@ -716,24 +679,21 @@ test('all async - end failure - no bail', t => { ], false ).run().then(result => { - t.strictDeepEqual(result, { - passed: false, - reason: 'c', - result: [ - { - passed: true, - result: 'a' - }, - { - passed: true, - result: 'b' - }, - { - passed: false, - reason: 'c' - } - ] - }); + t.strictDeepEqual(result, {passed: false}); + t.strictDeepEqual(results, [ + { + passed: true, + result: 'a' + }, + { + passed: true, + result: 'b' + }, + { + passed: false, + reason: 'c' + } + ]); }); }); @@ -746,24 +706,21 @@ test('all async - multiple failure - no bail', t => { ], false ).run().then(result => { - t.strictDeepEqual(result, { - passed: false, - reason: 'a', - result: [ - { - passed: false, - reason: 'a' - }, - { - passed: true, - result: 'b' - }, - { - passed: false, - reason: 'c' - } - ] - }); + t.strictDeepEqual(result, {passed: false}); + t.strictDeepEqual(results, [ + { + passed: false, + reason: 'a' + }, + { + passed: true, + result: 'b' + }, + { + passed: false, + reason: 'c' + } + ]); }); }); @@ -799,44 +756,29 @@ test('sequences of sequences', t => { new Concurrent([pass('c')]) ]).run(); - t.strictDeepEqual(result, { - passed: true, - reason: null, - result: [ - { - passed: true, - reason: null, - result: [ - { - passed: true, - result: 'a' - }, - { - passed: true, - result: 'b' - } - ] - }, - { - passed: true, - reason: null, - result: [ - { - passed: true, - result: 'c' - } - ] - } - ] - }); + t.strictDeepEqual(result, {passed: true}); + t.strictDeepEqual(results, [ + { + passed: true, + result: 'a' + }, + { + passed: true, + result: 'b' + }, + { + passed: true, + result: 'c' + } + ]); t.end(); }); -test('must be called with array of tests', t => { +test('must be called with array of runnables', t => { t.throws(() => { new Concurrent(pass('a')).run(); - }, {message: 'Expected an array of tests'}); + }, {message: 'Expected an array of runnables'}); t.end(); }); diff --git a/test/sequence.js b/test/sequence.js index fbc2ee864..d44bf4691 100644 --- a/test/sequence.js +++ b/test/sequence.js @@ -1,15 +1,32 @@ 'use strict'; -const test = require('tap').test; +const tap = require('tap'); const Promise = require('bluebird'); +const isPromise = require('is-promise'); const Sequence = require('../lib/sequence'); +let results = []; +const test = (name, fn) => { + tap.test(name, t => { + results = []; + return fn(t); + }); +}; +function collect(result) { + if (isPromise(result)) { + return result.then(collect); + } + + results.push(result); + return result; +} + function pass(val) { return { run() { - return { + return collect({ passed: true, result: val - }; + }); } }; } @@ -17,10 +34,10 @@ function pass(val) { function fail(val) { return { run() { - return { + return collect({ passed: false, reason: val - }; + }); } }; } @@ -28,10 +45,10 @@ function fail(val) { function passAsync(val) { return { run() { - return Promise.resolve({ + return collect(Promise.resolve({ passed: true, result: val - }); + })); } }; } @@ -39,10 +56,10 @@ function passAsync(val) { function failAsync(err) { return { run() { - return Promise.resolve({ + return collect(Promise.resolve({ // eslint-disable-line promise/no-promise-in-callback passed: false, reason: err - }); + })); } }; } @@ -65,24 +82,21 @@ test('all sync - no failure - no bail', t => { false ).run(); - t.strictDeepEqual(result, { - passed: true, - reason: null, - result: [ - { - passed: true, - result: 'a' - }, - { - passed: true, - result: 'b' - }, - { - passed: true, - result: 'c' - } - ] - }); + t.strictDeepEqual(result, {passed: true}); + t.strictDeepEqual(results, [ + { + passed: true, + result: 'a' + }, + { + passed: true, + result: 'b' + }, + { + passed: true, + result: 'c' + } + ]); t.end(); }); @@ -96,24 +110,21 @@ test('all sync - no failure - bail', t => { true ).run(); - t.strictDeepEqual(result, { - passed: true, - reason: null, - result: [ - { - passed: true, - result: 'a' - }, - { - passed: true, - result: 'b' - }, - { - passed: true, - result: 'c' - } - ] - }); + t.strictDeepEqual(result, {passed: true}); + t.strictDeepEqual(results, [ + { + passed: true, + result: 'a' + }, + { + passed: true, + result: 'b' + }, + { + passed: true, + result: 'c' + } + ]); t.end(); }); @@ -127,24 +138,21 @@ test('all sync - begin failure - no bail', t => { false ).run(); - t.strictDeepEqual(result, { - passed: false, - reason: 'a', - result: [ - { - passed: false, - reason: 'a' - }, - { - passed: true, - result: 'b' - }, - { - passed: true, - result: 'c' - } - ] - }); + t.strictDeepEqual(result, {passed: false}); + t.strictDeepEqual(results, [ + { + passed: false, + reason: 'a' + }, + { + passed: true, + result: 'b' + }, + { + passed: true, + result: 'c' + } + ]); t.end(); }); @@ -158,23 +166,20 @@ test('all sync - mid failure - no bail', t => { false ).run(); - t.strictDeepEqual(result, { - passed: false, - reason: 'b', - result: [ - { - passed: true, - result: 'a'}, - { - passed: false, - reason: 'b' - }, - { - passed: true, - result: 'c' - } - ] - }); + t.strictDeepEqual(result, {passed: false}); + t.strictDeepEqual(results, [ + { + passed: true, + result: 'a'}, + { + passed: false, + reason: 'b' + }, + { + passed: true, + result: 'c' + } + ]); t.end(); }); @@ -188,24 +193,21 @@ test('all sync - end failure - no bail', t => { false ).run(); - t.strictDeepEqual(result, { - passed: false, - reason: 'c', - result: [ - { - passed: true, - result: 'a' - }, - { - passed: true, - result: 'b' - }, - { - passed: false, - reason: 'c' - } - ] - }); + t.strictDeepEqual(result, {passed: false}); + t.strictDeepEqual(results, [ + { + passed: true, + result: 'a' + }, + { + passed: true, + result: 'b' + }, + { + passed: false, + reason: 'c' + } + ]); t.end(); }); @@ -219,24 +221,21 @@ test('all sync - multiple failure - no bail', t => { false ).run(); - t.strictDeepEqual(result, { - passed: false, - reason: 'a', - result: [ - { - passed: false, - reason: 'a' - }, - { - passed: true, - result: 'b' - }, - { - passed: false, - reason: 'c' - } - ] - }); + t.strictDeepEqual(result, {passed: false}); + t.strictDeepEqual(results, [ + { + passed: false, + reason: 'a' + }, + { + passed: true, + result: 'b' + }, + { + passed: false, + reason: 'c' + } + ]); t.end(); }); @@ -250,16 +249,13 @@ test('all sync - begin failure - bail', t => { true ).run(); - t.strictDeepEqual(result, { - passed: false, - reason: 'a', - result: [ - { - passed: false, - reason: 'a' - } - ] - }); + t.strictDeepEqual(result, {passed: false}); + t.strictDeepEqual(results, [ + { + passed: false, + reason: 'a' + } + ]); t.end(); }); @@ -273,20 +269,17 @@ test('all sync - mid failure - bail', t => { true ).run(); - t.strictDeepEqual(result, { - passed: false, - reason: 'b', - result: [ - { - passed: true, - result: 'a' - }, - { - passed: false, - reason: 'b' - } - ] - }); + t.strictDeepEqual(result, {passed: false}); + t.strictDeepEqual(results, [ + { + passed: true, + result: 'a' + }, + { + passed: false, + reason: 'b' + } + ]); t.end(); }); @@ -300,24 +293,21 @@ test('all sync - end failure - bail', t => { true ).run(); - t.strictDeepEqual(result, { - passed: false, - reason: 'c', - result: [ - { - passed: true, - result: 'a' - }, - { - passed: true, - result: 'b' - }, - { - passed: false, - reason: 'c' - } - ] - }); + t.strictDeepEqual(result, {passed: false}); + t.strictDeepEqual(results, [ + { + passed: true, + result: 'a' + }, + { + passed: true, + result: 'b' + }, + { + passed: false, + reason: 'c' + } + ]); t.end(); }); @@ -330,24 +320,21 @@ test('all async - no failure - no bail', t => { ], false ).run().then(result => { - t.strictDeepEqual(result, { - passed: true, - reason: null, - result: [ - { - passed: true, - result: 'a' - }, - { - passed: true, - result: 'b' - }, - { - passed: true, - result: 'c' - } - ] - }); + t.strictDeepEqual(result, {passed: true}); + t.strictDeepEqual(results, [ + { + passed: true, + result: 'a' + }, + { + passed: true, + result: 'b' + }, + { + passed: true, + result: 'c' + } + ]); t.end(); }); }); @@ -361,24 +348,21 @@ test('all async - no failure - bail', t => { ], true ).run().then(result => { - t.strictDeepEqual(result, { - passed: true, - reason: null, - result: [ - { - passed: true, - result: 'a' - }, - { - passed: true, - result: 'b' - }, - { - passed: true, - result: 'c' - } - ] - }); + t.strictDeepEqual(result, {passed: true}); + t.strictDeepEqual(results, [ + { + passed: true, + result: 'a' + }, + { + passed: true, + result: 'b' + }, + { + passed: true, + result: 'c' + } + ]); t.end(); }); }); @@ -392,24 +376,21 @@ test('last async - no failure - no bail', t => { ], false ).run().then(result => { - t.strictDeepEqual(result, { - passed: true, - reason: null, - result: [ - { - passed: true, - result: 'a' - }, - { - passed: true, - result: 'b' - }, - { - passed: true, - result: 'c' - } - ] - }); + t.strictDeepEqual(result, {passed: true}); + t.strictDeepEqual(results, [ + { + passed: true, + result: 'a' + }, + { + passed: true, + result: 'b' + }, + { + passed: true, + result: 'c' + } + ]); t.end(); }); }); @@ -423,24 +404,21 @@ test('mid async - no failure - no bail', t => { ], false ).run().then(result => { - t.strictDeepEqual(result, { - passed: true, - reason: null, - result: [ - { - passed: true, - result: 'a' - }, - { - passed: true, - result: 'b' - }, - { - passed: true, - result: 'c' - } - ] - }); + t.strictDeepEqual(result, {passed: true}); + t.strictDeepEqual(results, [ + { + passed: true, + result: 'a' + }, + { + passed: true, + result: 'b' + }, + { + passed: true, + result: 'c' + } + ]); t.end(); }); }); @@ -454,24 +432,21 @@ test('first async - no failure - no bail', t => { ], false ).run().then(result => { - t.strictDeepEqual(result, { - passed: true, - reason: null, - result: [ - { - passed: true, - result: 'a' - }, - { - passed: true, - result: 'b' - }, - { - passed: true, - result: 'c' - } - ] - }); + t.strictDeepEqual(result, {passed: true}); + t.strictDeepEqual(results, [ + { + passed: true, + result: 'a' + }, + { + passed: true, + result: 'b' + }, + { + passed: true, + result: 'c' + } + ]); t.end(); }); }); @@ -485,24 +460,21 @@ test('last async - no failure - bail', t => { ], true ).run().then(result => { - t.strictDeepEqual(result, { - passed: true, - reason: null, - result: [ - { - passed: true, - result: 'a' - }, - { - passed: true, - result: 'b' - }, - { - passed: true, - result: 'c' - } - ] - }); + t.strictDeepEqual(result, {passed: true}); + t.strictDeepEqual(results, [ + { + passed: true, + result: 'a' + }, + { + passed: true, + result: 'b' + }, + { + passed: true, + result: 'c' + } + ]); t.end(); }); }); @@ -516,24 +488,21 @@ test('mid async - no failure - bail', t => { ], true ).run().then(result => { - t.strictDeepEqual(result, { - passed: true, - reason: null, - result: [ - { - passed: true, - result: 'a' - }, - { - passed: true, - result: 'b' - }, - { - passed: true, - result: 'c' - } - ] - }); + t.strictDeepEqual(result, {passed: true}); + t.strictDeepEqual(results, [ + { + passed: true, + result: 'a' + }, + { + passed: true, + result: 'b' + }, + { + passed: true, + result: 'c' + } + ]); t.end(); }); }); @@ -547,24 +516,21 @@ test('first async - no failure - bail', t => { ], true ).run().then(result => { - t.strictDeepEqual(result, { - passed: true, - reason: null, - result: [ - { - passed: true, - result: 'a' - }, - { - passed: true, - result: 'b' - }, - { - passed: true, - result: 'c' - } - ] - }); + t.strictDeepEqual(result, {passed: true}); + t.strictDeepEqual(results, [ + { + passed: true, + result: 'a' + }, + { + passed: true, + result: 'b' + }, + { + passed: true, + result: 'c' + } + ]); t.end(); }); }); @@ -578,16 +544,13 @@ test('all async - begin failure - bail', t => { ], true ).run().then(result => { - t.strictDeepEqual(result, { - passed: false, - reason: 'a', - result: [ - { - passed: false, - reason: 'a' - } - ] - }); + t.strictDeepEqual(result, {passed: false}); + t.strictDeepEqual(results, [ + { + passed: false, + reason: 'a' + } + ]); t.end(); }); }); @@ -601,20 +564,17 @@ test('all async - mid failure - bail', t => { ], true ).run().then(result => { - t.strictDeepEqual(result, { - passed: false, - reason: 'b', - result: [ - { - passed: true, - result: 'a' - }, - { - passed: false, - reason: 'b' - } - ] - }); + t.strictDeepEqual(result, {passed: false}); + t.strictDeepEqual(results, [ + { + passed: true, + result: 'a' + }, + { + passed: false, + reason: 'b' + } + ]); t.end(); }); }); @@ -628,24 +588,21 @@ test('all async - end failure - bail', t => { ], true ).run().then(result => { - t.strictDeepEqual(result, { - passed: false, - reason: 'c', - result: [ - { - passed: true, - result: 'a' - }, - { - passed: true, - result: 'b' - }, - { - passed: false, - reason: 'c' - } - ] - }); + t.strictDeepEqual(result, {passed: false}); + t.strictDeepEqual(results, [ + { + passed: true, + result: 'a' + }, + { + passed: true, + result: 'b' + }, + { + passed: false, + reason: 'c' + } + ]); t.end(); }); }); @@ -659,24 +616,21 @@ test('all async - begin failure - no bail', t => { ], false ).run().then(result => { - t.strictDeepEqual(result, { - passed: false, - reason: 'a', - result: [ - { - passed: false, - reason: 'a' - }, - { - passed: true, - result: 'b' - }, - { - passed: true, - result: 'c' - } - ] - }); + t.strictDeepEqual(result, {passed: false}); + t.strictDeepEqual(results, [ + { + passed: false, + reason: 'a' + }, + { + passed: true, + result: 'b' + }, + { + passed: true, + result: 'c' + } + ]); t.end(); }); }); @@ -690,24 +644,21 @@ test('all async - mid failure - no bail', t => { ], false ).run().then(result => { - t.strictDeepEqual(result, { - passed: false, - reason: 'b', - result: [ - { - passed: true, - result: 'a' - }, - { - passed: false, - reason: 'b' - }, - { - passed: true, - result: 'c' - } - ] - }); + t.strictDeepEqual(result, {passed: false}); + t.strictDeepEqual(results, [ + { + passed: true, + result: 'a' + }, + { + passed: false, + reason: 'b' + }, + { + passed: true, + result: 'c' + } + ]); t.end(); }); }); @@ -721,24 +672,21 @@ test('all async - end failure - no bail', t => { ], false ).run().then(result => { - t.strictDeepEqual(result, { - passed: false, - reason: 'c', - result: [ - { - passed: true, - result: 'a' - }, - { - passed: true, - result: 'b' - }, - { - passed: false, - reason: 'c' - } - ] - }); + t.strictDeepEqual(result, {passed: false}); + t.strictDeepEqual(results, [ + { + passed: true, + result: 'a' + }, + { + passed: true, + result: 'b' + }, + { + passed: false, + reason: 'c' + } + ]); t.end(); }); }); @@ -752,24 +700,21 @@ test('all async - multiple failure - no bail', t => { ], false ).run().then(result => { - t.strictDeepEqual(result, { - passed: false, - reason: 'a', - result: [ - { - passed: false, - reason: 'a' - }, - { - passed: true, - result: 'b' - }, - { - passed: false, - reason: 'c' - } - ] - }); + t.strictDeepEqual(result, {passed: false}); + t.strictDeepEqual(results, [ + { + passed: false, + reason: 'a' + }, + { + passed: true, + result: 'b' + }, + { + passed: false, + reason: 'c' + } + ]); t.end(); }); }); @@ -802,10 +747,10 @@ test('rejections are just passed through - bail', t => { }); }); -test('needs at least one sequence item', t => { +test('needs at least one sequence runnable', t => { t.throws(() => { new Sequence().run(); - }, {message: 'Sequence items can\'t be undefined'}); + }, {message: 'Expected an array of runnables'}); t.end(); }); @@ -815,36 +760,21 @@ test('sequences of sequences', t => { new Sequence([pass('c')]) ]).run(); - t.strictDeepEqual(result, { - passed: true, - reason: null, - result: [ - { - passed: true, - reason: null, - result: [ - { - passed: true, - result: 'a' - }, - { - passed: true, - result: 'b' - } - ] - }, - { - passed: true, - reason: null, - result: [ - { - passed: true, - result: 'c' - } - ] - } - ] - }); + t.strictDeepEqual(result, {passed: true}); + t.strictDeepEqual(results, [ + { + passed: true, + result: 'a' + }, + { + passed: true, + result: 'b' + }, + { + passed: true, + result: 'c' + } + ]); t.end(); }); From 36a1eed88978356b61b85bf1202be2f9fd85b7c7 Mon Sep 17 00:00:00 2001 From: Mark Wubben Date: Thu, 16 Mar 2017 16:15:32 +0000 Subject: [PATCH 08/15] Stop returning full test results Test results are emitted directly, so Test#run() only needs to return a (promise for) a boolean, indicating whether the test passed. Make the same change in Concurrent and Sequence, and return a boolean if all runnables passed. Refactor tests to stop relying on Test instances returning their result. --- lib/concurrent.js | 19 ++- lib/sequence.js | 21 ++- lib/test-collection.js | 12 +- lib/test.js | 5 +- test/concurrent.js | 102 ++++++------- test/observable.js | 71 ++++++--- test/promise.js | 145 +++++++++++++------ test/sequence.js | 102 ++++++------- test/test-collection.js | 10 +- test/test.js | 314 +++++++++++++++++++++++++++------------- 10 files changed, 496 insertions(+), 305 deletions(-) diff --git a/lib/concurrent.js b/lib/concurrent.js index 872931025..3cdbb41c3 100644 --- a/lib/concurrent.js +++ b/lib/concurrent.js @@ -1,5 +1,4 @@ 'use strict'; -const isPromise = require('is-promise'); class Concurrent { constructor(runnables, bail) { @@ -26,8 +25,8 @@ class Concurrent { }); } - allPromises.push(promise.then(result => { - if (!result.passed) { + allPromises.push(promise.then(passed => { + if (!passed) { allPassed = false; if (this.bail) { @@ -39,26 +38,26 @@ class Concurrent { }; for (const runnable of this.runnables) { - const result = runnable.run(); + const passedOrPromise = runnable.run(); - if (isPromise(result)) { - handlePromise(result); - } else if (!result.passed) { + if (!passedOrPromise) { if (this.bail) { // Stop if the test failed and bail mode is on. - return {passed: false}; + return false; } allPassed = false; + } else if (passedOrPromise !== true) { + handlePromise(passedOrPromise); } } if (pending) { Promise.all(allPromises).then(resolvePending); - return pending.then(() => ({passed: allPassed})); + return pending.then(() => allPassed); } - return {passed: allPassed}; + return allPassed; } } diff --git a/lib/sequence.js b/lib/sequence.js index 75eca2da7..cd3e1f7d8 100644 --- a/lib/sequence.js +++ b/lib/sequence.js @@ -1,5 +1,4 @@ 'use strict'; -const isPromise = require('is-promise'); class Sequence { constructor(runnables, bail) { @@ -19,33 +18,31 @@ class Sequence { let promise; for (let next = iterator.next(); !next.done; next = iterator.next()) { - const result = next.value.run(); - if (isPromise(result)) { - promise = result; - break; - } - - if (!result.passed) { + const passedOrPromise = next.value.run(); + if (!passedOrPromise) { allPassed = false; if (this.bail) { // Stop if the test failed and bail mode is on. break; } + } else if (passedOrPromise !== true) { + promise = passedOrPromise; + break; } } if (!promise) { - return {passed: allPassed}; + return allPassed; } - return promise.then(result => { - if (!result.passed) { + return promise.then(passed => { + if (!passed) { allPassed = false; if (this.bail) { // Stop if the test failed and bail mode is on. - return {passed: false}; + return false; } } diff --git a/lib/test-collection.js b/lib/test-collection.js index c0ac1a771..87b13211e 100644 --- a/lib/test-collection.js +++ b/lib/test-collection.js @@ -88,18 +88,14 @@ class TestCollection extends EventEmitter { } } _skippedTest(test) { - const self = this; - return { - run() { - const result = { + run: () => { + this._emitTestResult({ passed: true, result: test - }; - - self._emitTestResult(result); + }); - return result; + return true; } }; } diff --git a/lib/test.js b/lib/test.js index 4ce145ccf..fb84dcb9a 100644 --- a/lib/test.js +++ b/lib/test.js @@ -181,10 +181,11 @@ class Test { this._promise.promise = new Promise((resolve, reject) => { this._promise.resolve = resolve; this._promise.reject = reject; - }).tap(result => { + }).then(result => { if (this.report) { this.report(result); } + return result.passed; }); } @@ -316,7 +317,7 @@ class Test { this.report(result); } - return result; + return result.passed; } Promise.all(this.pendingAssertions) diff --git a/test/concurrent.js b/test/concurrent.js index 5da32354c..5fe61bb50 100644 --- a/test/concurrent.js +++ b/test/concurrent.js @@ -16,7 +16,7 @@ function collect(result) { } results.push(result); - return result; + return result.passed; } function pass(val) { @@ -80,7 +80,7 @@ function reject(err) { } test('all sync - all pass - no bail', t => { - const result = new Concurrent( + const passed = new Concurrent( [ pass('a'), pass('b'), @@ -89,7 +89,7 @@ test('all sync - all pass - no bail', t => { false ).run(); - t.strictDeepEqual(result, {passed: true}); + t.equal(passed, true); t.strictDeepEqual(results, [ { passed: true, @@ -108,7 +108,7 @@ test('all sync - all pass - no bail', t => { }); test('all sync - no failure - bail', t => { - const result = new Concurrent( + const passed = new Concurrent( [ pass('a'), pass('b'), @@ -117,7 +117,7 @@ test('all sync - no failure - bail', t => { true ).run(); - t.strictDeepEqual(result, {passed: true}); + t.equal(passed, true); t.strictDeepEqual(results, [ { passed: true, @@ -136,7 +136,7 @@ test('all sync - no failure - bail', t => { }); test('all sync - begin failure - no bail', t => { - const result = new Concurrent( + const passed = new Concurrent( [ fail('a'), pass('b'), @@ -145,7 +145,7 @@ test('all sync - begin failure - no bail', t => { false ).run(); - t.strictDeepEqual(result, {passed: false}); + t.equal(passed, false); t.strictDeepEqual(results, [ { passed: false, @@ -164,7 +164,7 @@ test('all sync - begin failure - no bail', t => { }); test('all sync - mid failure - no bail', t => { - const result = new Concurrent( + const passed = new Concurrent( [ pass('a'), fail('b'), @@ -173,7 +173,7 @@ test('all sync - mid failure - no bail', t => { false ).run(); - t.strictDeepEqual(result, {passed: false}); + t.equal(passed, false); t.strictDeepEqual(results, [ { passed: true, @@ -192,7 +192,7 @@ test('all sync - mid failure - no bail', t => { }); test('all sync - end failure - no bail', t => { - const result = new Concurrent( + const passed = new Concurrent( [ pass('a'), pass('b'), @@ -201,7 +201,7 @@ test('all sync - end failure - no bail', t => { false ).run(); - t.strictDeepEqual(result, {passed: false}); + t.equal(passed, false); t.strictDeepEqual(results, [ { passed: true, @@ -220,7 +220,7 @@ test('all sync - end failure - no bail', t => { }); test('all sync - multiple failure - no bail', t => { - const result = new Concurrent( + const passed = new Concurrent( [ fail('a'), pass('b'), @@ -229,7 +229,7 @@ test('all sync - multiple failure - no bail', t => { false ).run(); - t.strictDeepEqual(result, {passed: false}); + t.equal(passed, false); t.strictDeepEqual(results, [ { passed: false, @@ -248,7 +248,7 @@ test('all sync - multiple failure - no bail', t => { }); test('all sync - begin failure - bail', t => { - const result = new Concurrent( + const passed = new Concurrent( [ fail('a'), pass('b'), @@ -257,7 +257,7 @@ test('all sync - begin failure - bail', t => { true ).run(); - t.strictDeepEqual(result, {passed: false}); + t.equal(passed, false); t.strictDeepEqual(results, [ { passed: false, @@ -268,7 +268,7 @@ test('all sync - begin failure - bail', t => { }); test('all sync - mid failure - bail', t => { - const result = new Concurrent( + const passed = new Concurrent( [ pass('a'), fail('b'), @@ -277,7 +277,7 @@ test('all sync - mid failure - bail', t => { true ).run(); - t.strictDeepEqual(result, {passed: false}); + t.equal(passed, false); t.strictDeepEqual(results, [ { passed: true, @@ -292,7 +292,7 @@ test('all sync - mid failure - bail', t => { }); test('all sync - end failure - bail', t => { - const result = new Concurrent( + const passed = new Concurrent( [ pass('a'), pass('b'), @@ -301,7 +301,7 @@ test('all sync - end failure - bail', t => { true ).run(); - t.strictDeepEqual(result, {passed: false}); + t.equal(passed, false); t.strictDeepEqual(results, [ { passed: true, @@ -327,8 +327,8 @@ test('all async - no failure - no bail', t => { passAsync('c') ], false - ).run().then(result => { - t.strictDeepEqual(result, {passed: true}); + ).run().then(passed => { + t.equal(passed, true); t.strictDeepEqual(results, [ { passed: true, @@ -354,8 +354,8 @@ test('all async - no failure - bail', t => { passAsync('c') ], true - ).run().then(result => { - t.strictDeepEqual(result, {passed: true}); + ).run().then(passed => { + t.equal(passed, true); t.strictDeepEqual(results, [ { passed: true, @@ -381,8 +381,8 @@ test('last async - no failure - no bail', t => { passAsync('c') ], false - ).run().then(result => { - t.strictDeepEqual(result, {passed: true}); + ).run().then(passed => { + t.equal(passed, true); t.strictDeepEqual(results, [ { passed: true, @@ -408,8 +408,8 @@ test('mid async - no failure - no bail', t => { pass('c') ], false - ).run().then(result => { - t.strictDeepEqual(result, {passed: true}); + ).run().then(passed => { + t.equal(passed, true); t.strictDeepEqual(results, [ { passed: true, @@ -435,8 +435,8 @@ test('first async - no failure - no bail', t => { pass('c') ], false - ).run().then(result => { - t.strictDeepEqual(result, {passed: true}); + ).run().then(passed => { + t.equal(passed, true); t.strictDeepEqual(results, [ { passed: true, @@ -462,8 +462,8 @@ test('last async - no failure - bail', t => { passAsync('c') ], true - ).run().then(result => { - t.strictDeepEqual(result, {passed: true}); + ).run().then(passed => { + t.equal(passed, true); t.strictDeepEqual(results, [ { passed: true, @@ -489,8 +489,8 @@ test('mid async - no failure - bail', t => { pass('c') ], true - ).run().then(result => { - t.strictDeepEqual(result, {passed: true}); + ).run().then(passed => { + t.equal(passed, true); t.strictDeepEqual(results, [ { passed: true, @@ -516,8 +516,8 @@ test('first async - no failure - bail', t => { pass('c') ], true - ).run().then(result => { - t.strictDeepEqual(result, {passed: true}); + ).run().then(passed => { + t.equal(passed, true); t.strictDeepEqual(results, [ { passed: true, @@ -543,8 +543,8 @@ test('all async - begin failure - bail', t => { passAsync('c') ], true - ).run().then(result => { - t.strictDeepEqual(result, {passed: false}); + ).run().then(passed => { + t.equal(passed, false); t.strictDeepEqual(results, [ { passed: false, @@ -570,8 +570,8 @@ test('all async - mid failure - bail', t => { passAsync('c') ], true - ).run().then(result => { - t.strictDeepEqual(result, {passed: false}); + ).run().then(passed => { + t.equal(passed, false); t.strictDeepEqual(results, [ { passed: true, @@ -597,8 +597,8 @@ test('all async - end failure - bail', t => { failAsync('c') ], true - ).run().then(result => { - t.strictDeepEqual(result, {passed: false}); + ).run().then(passed => { + t.equal(passed, false); t.strictDeepEqual(results, [ { passed: true, @@ -624,8 +624,8 @@ test('all async - begin failure - no bail', t => { passAsync('c') ], false - ).run().then(result => { - t.strictDeepEqual(result, {passed: false}); + ).run().then(passed => { + t.equal(passed, false); t.strictDeepEqual(results, [ { passed: false, @@ -651,8 +651,8 @@ test('all async - mid failure - no bail', t => { passAsync('c') ], false - ).run().then(result => { - t.strictDeepEqual(result, {passed: false}); + ).run().then(passed => { + t.equal(passed, false); t.strictDeepEqual(results, [ { passed: true, @@ -678,8 +678,8 @@ test('all async - end failure - no bail', t => { failAsync('c') ], false - ).run().then(result => { - t.strictDeepEqual(result, {passed: false}); + ).run().then(passed => { + t.equal(passed, false); t.strictDeepEqual(results, [ { passed: true, @@ -705,8 +705,8 @@ test('all async - multiple failure - no bail', t => { failAsync('c') ], false - ).run().then(result => { - t.strictDeepEqual(result, {passed: false}); + ).run().then(passed => { + t.equal(passed, false); t.strictDeepEqual(results, [ { passed: false, @@ -751,12 +751,12 @@ test('rejections are just passed through - bail', t => { }); test('sequences of sequences', t => { - const result = new Concurrent([ + const passed = new Concurrent([ new Concurrent([pass('a'), pass('b')]), new Concurrent([pass('c')]) ]).run(); - t.strictDeepEqual(result, {passed: true}); + t.equal(passed, true); t.strictDeepEqual(results, [ { passed: true, diff --git a/test/observable.js b/test/observable.js index 8395d1ff2..6955594ac 100644 --- a/test/observable.js +++ b/test/observable.js @@ -3,19 +3,20 @@ const test = require('tap').test; const Test = require('../lib/test'); const Observable = require('zen-observable'); // eslint-disable-line import/order -function ava(fn) { - const a = new Test(fn); +function ava(fn, onResult) { + const a = new Test(fn, null, null, onResult); a.metadata = {callback: false}; return a; } -ava.cb = function (fn) { - const a = new Test(fn); +ava.cb = function (fn, onResult) { + const a = new Test(fn, null, null, onResult); a.metadata = {callback: true}; return a; }; test('returning an observable from a legacy async fn is an error', t => { + let result; ava.cb(a => { a.plan(2); @@ -28,14 +29,17 @@ test('returning an observable from a legacy async fn is an error', t => { }, 200); return observable; - }).run().then(result => { - t.is(result.passed, false); + }, r => { + result = r; + }).run().then(passed => { + t.is(passed, false); t.match(result.reason.message, /Do not return observables/); t.end(); }); }); test('handle throws with thrown observable', t => { + let result; ava(a => { a.plan(1); @@ -44,14 +48,17 @@ test('handle throws with thrown observable', t => { }); return a.throws(observable); - }).run().then(result => { - t.is(result.passed, true); + }, r => { + result = r; + }).run().then(passed => { + t.is(passed, true); t.is(result.result.assertCount, 1); t.end(); }); }); test('handle throws with long running thrown observable', t => { + let result; ava(a => { a.plan(1); @@ -62,27 +69,33 @@ test('handle throws with long running thrown observable', t => { }); return a.throws(observable, /abc/); - }).run().then(result => { - t.is(result.passed, true); + }, r => { + result = r; + }).run().then(passed => { + t.is(passed, true); t.is(result.result.assertCount, 1); t.end(); }); }); test('handle throws with completed observable', t => { + let result; ava(a => { a.plan(1); const observable = Observable.of(); return a.throws(observable); - }).run().then(result => { - t.is(result.passed, false); + }, r => { + result = r; + }).run().then(passed => { + t.is(passed, false); t.is(result.reason.name, 'AssertionError'); t.end(); }); }); test('handle throws with regex', t => { + let result; ava(a => { a.plan(1); @@ -91,14 +104,17 @@ test('handle throws with regex', t => { }); return a.throws(observable, /abc/); - }).run().then(result => { - t.is(result.passed, true); + }, r => { + result = r; + }).run().then(passed => { + t.is(passed, true); t.is(result.result.assertCount, 1); t.end(); }); }); test('handle throws with string', t => { + let result; ava(a => { a.plan(1); @@ -107,14 +123,17 @@ test('handle throws with string', t => { }); return a.throws(observable, 'abc'); - }).run().then(result => { - t.is(result.passed, true); + }, r => { + result = r; + }).run().then(passed => { + t.is(passed, true); t.is(result.result.assertCount, 1); t.end(); }); }); test('handle throws with false-positive observable', t => { + let result; ava(a => { a.plan(1); @@ -124,27 +143,33 @@ test('handle throws with false-positive observable', t => { }); return a.throws(observable); - }).run().then(result => { - t.is(result.passed, false); + }, r => { + result = r; + }).run().then(passed => { + t.is(passed, false); t.is(result.reason.name, 'AssertionError'); t.end(); }); }); test('handle notThrows with completed observable', t => { + let result; ava(a => { a.plan(1); const observable = Observable.of(); return a.notThrows(observable); - }).run().then(result => { - t.is(result.passed, true); + }, r => { + result = r; + }).run().then(passed => { + t.is(passed, true); t.is(result.result.assertCount, 1); t.end(); }); }); test('handle notThrows with thrown observable', t => { + let result; ava(a => { a.plan(1); @@ -153,8 +178,10 @@ test('handle notThrows with thrown observable', t => { }); return a.notThrows(observable); - }).run().then(result => { - t.is(result.passed, false); + }, r => { + result = r; + }).run().then(passed => { + t.is(passed, false); t.is(result.reason.name, 'AssertionError'); t.end(); }); diff --git a/test/promise.js b/test/promise.js index f90d9f9a3..5fdbefe3a 100644 --- a/test/promise.js +++ b/test/promise.js @@ -4,14 +4,14 @@ const test = require('tap').test; const formatValue = require('../lib/format-assert-error').formatValue; const Test = require('../lib/test'); -function ava(fn) { - const a = new Test(fn); +function ava(fn, onResult) { + const a = new Test(fn, null, null, onResult); a.metadata = {callback: false}; return a; } -ava.cb = function (fn) { - const a = new Test(fn); +ava.cb = function (fn, onResult) { + const a = new Test(fn, null, null, onResult); a.metadata = {callback: true}; return a; }; @@ -31,6 +31,7 @@ function fail() { } test('returning a promise from a legacy async fn is an error', t => { + let result; ava.cb(a => { a.plan(1); @@ -38,14 +39,17 @@ test('returning a promise from a legacy async fn is an error', t => { a.pass(); a.end(); }); - }).run().then(result => { - t.is(result.passed, false); + }, r => { + result = r; + }).run().then(passed => { + t.is(passed, false); t.match(result.reason.message, /Do not return promises/); t.end(); }); }); test('assertion plan is tested after returned promise resolves', t => { + let result; const start = Date.now(); ava(a => { a.plan(2); @@ -60,8 +64,10 @@ test('assertion plan is tested after returned promise resolves', t => { a.pass(); return defer.promise; - }).run().then(result => { - t.is(result.passed, true); + }, r => { + result = r; + }).run().then(passed => { + t.is(passed, true); t.is(result.result.planCount, 2); t.is(result.result.assertCount, 2); t.true(Date.now() - start >= 500); @@ -70,6 +76,7 @@ test('assertion plan is tested after returned promise resolves', t => { }); test('missing assertion will fail the test', t => { + let result; ava(a => { a.plan(2); @@ -81,14 +88,17 @@ test('missing assertion will fail the test', t => { }, 200); return defer.promise; - }).run().then(result => { - t.is(result.passed, false); + }, r => { + result = r; + }).run().then(passed => { + t.is(passed, false); t.is(result.reason.assertion, 'plan'); t.end(); }); }); test('extra assertion will fail the test', t => { + let result; ava(a => { a.plan(2); @@ -105,21 +115,26 @@ test('extra assertion will fail the test', t => { }, 500); return defer.promise; - }).run().then(result => { - t.is(result.passed, false); + }, r => { + result = r; + }).run().then(passed => { + t.is(passed, false); t.is(result.reason.assertion, 'plan'); t.end(); }); }); test('handle throws with rejected promise', t => { + let result; ava(a => { a.plan(1); const promise = Promise.reject(new Error()); return a.throws(promise); - }).run().then(result => { - t.is(result.passed, true); + }, r => { + result = r; + }).run().then(passed => { + t.is(passed, true); t.is(result.result.assertCount, 1); t.end(); }); @@ -127,6 +142,7 @@ test('handle throws with rejected promise', t => { // TODO(team): This is a very slow test, and I can't figure out why we need it - James test('handle throws with long running rejected promise', t => { + let result; ava(a => { a.plan(1); @@ -137,161 +153,199 @@ test('handle throws with long running rejected promise', t => { }); return a.throws(promise, /abc/); - }).run().then(result => { - t.is(result.passed, true); + }, r => { + result = r; + }).run().then(passed => { + t.is(passed, true); t.is(result.result.assertCount, 1); t.end(); }); }); test('handle throws with resolved promise', t => { + let result; ava(a => { a.plan(1); const promise = Promise.resolve(); return a.throws(promise); - }).run().then(result => { - t.is(result.passed, false); + }, r => { + result = r; + }).run().then(passed => { + t.is(passed, false); t.is(result.reason.name, 'AssertionError'); t.end(); }); }); test('handle throws with regex', t => { + let result; ava(a => { a.plan(1); const promise = Promise.reject(new Error('abc')); return a.throws(promise, /abc/); - }).run().then(result => { - t.is(result.passed, true); + }, r => { + result = r; + }).run().then(passed => { + t.is(passed, true); t.is(result.result.assertCount, 1); t.end(); }); }); test('throws with regex will fail if error message does not match', t => { + let result; ava(a => { a.plan(1); const promise = Promise.reject(new Error('abc')); return a.throws(promise, /def/); - }).run().then(result => { - t.is(result.passed, false); + }, r => { + result = r; + }).run().then(passed => { + t.is(passed, false); t.is(result.reason.name, 'AssertionError'); t.end(); }); }); test('handle throws with string', t => { + let result; ava(a => { a.plan(1); const promise = Promise.reject(new Error('abc')); return a.throws(promise, 'abc'); - }).run().then(result => { - t.is(result.passed, true); + }, r => { + result = r; + }).run().then(passed => { + t.is(passed, true); t.is(result.result.assertCount, 1); t.end(); }); }); test('throws with string argument will reject if message does not match', t => { + let result; ava(a => { a.plan(1); const promise = Promise.reject(new Error('abc')); return a.throws(promise, 'def'); - }).run().then(result => { - t.is(result.passed, false); + }, r => { + result = r; + }).run().then(passed => { + t.is(passed, false); t.is(result.reason.name, 'AssertionError'); t.end(); }); }); test('does not handle throws with string reject', t => { + let result; ava(a => { a.plan(1); const promise = Promise.reject('abc'); // eslint-disable-line prefer-promise-reject-errors return a.throws(promise, 'abc'); - }).run().then(result => { - t.is(result.passed, false); + }, r => { + result = r; + }).run().then(passed => { + t.is(passed, false); t.is(result.reason.name, 'AssertionError'); t.end(); }); }); test('handle throws with false-positive promise', t => { + let result; ava(a => { a.plan(1); const promise = Promise.resolve(new Error()); return a.throws(promise); - }).run().then(result => { - t.is(result.passed, false); + }, r => { + result = r; + }).run().then(passed => { + t.is(passed, false); t.is(result.reason.name, 'AssertionError'); t.end(); }); }); test('handle notThrows with resolved promise', t => { + let result; ava(a => { a.plan(1); const promise = Promise.resolve(); return a.notThrows(promise); - }).run().then(result => { - t.is(result.passed, true); + }, r => { + result = r; + }).run().then(passed => { + t.is(passed, true); t.is(result.result.assertCount, 1); t.end(); }); }); test('handle notThrows with rejected promise', t => { + let result; ava(a => { a.plan(1); const promise = Promise.reject(new Error()); return a.notThrows(promise); - }).run().then(result => { - t.is(result.passed, false); + }, r => { + result = r; + }).run().then(passed => { + t.is(passed, false); t.is(result.reason.name, 'AssertionError'); t.end(); }); }); test('assert pass', t => { + let result; ava(a => { return pass().then(() => { a.pass(); }); - }).run().then(result => { - t.is(result.passed, true); + }, r => { + result = r; + }).run().then(passed => { + t.is(passed, true); t.is(result.result.assertCount, 1); t.end(); }); }); test('assert fail', t => { + let result; ava(a => { return pass().then(() => { a.fail(); }); - }).run().then(result => { - t.is(result.passed, false); + }, r => { + result = r; + }).run().then(passed => { + t.is(passed, false); t.is(result.reason.name, 'AssertionError'); t.end(); }); }); test('reject', t => { + let result; ava(a => { return fail().then(() => { a.pass(); }); - }).run().then(result => { - t.is(result.passed, false); + }, r => { + result = r; + }).run().then(passed => { + t.is(passed, false); t.is(result.reason.name, 'AssertionError'); t.is(result.reason.message, 'Rejected promise returned by test'); t.same(result.reason.values, [{label: 'Rejection reason:', formatted: formatValue(new Error('unicorn'))}]); @@ -300,9 +354,14 @@ test('reject', t => { }); test('reject with non-Error', t => { - // eslint-disable-next-line prefer-promise-reject-errors - ava(() => Promise.reject('failure')).run().then(result => { - t.is(result.passed, false); + let result; + ava( + () => Promise.reject('failure'), // eslint-disable-line prefer-promise-reject-errors + r => { + result = r; + } + ).run().then(passed => { + t.is(passed, false); t.is(result.reason.name, 'AssertionError'); t.is(result.reason.message, 'Rejected promise returned by test'); t.same(result.reason.values, [{label: 'Rejection reason:', formatted: formatValue('failure')}]); diff --git a/test/sequence.js b/test/sequence.js index d44bf4691..2cee0f1dc 100644 --- a/test/sequence.js +++ b/test/sequence.js @@ -17,7 +17,7 @@ function collect(result) { } results.push(result); - return result; + return result.passed; } function pass(val) { @@ -73,7 +73,7 @@ function reject(err) { } test('all sync - no failure - no bail', t => { - const result = new Sequence( + const passed = new Sequence( [ pass('a'), pass('b'), @@ -82,7 +82,7 @@ test('all sync - no failure - no bail', t => { false ).run(); - t.strictDeepEqual(result, {passed: true}); + t.equal(passed, true); t.strictDeepEqual(results, [ { passed: true, @@ -101,7 +101,7 @@ test('all sync - no failure - no bail', t => { }); test('all sync - no failure - bail', t => { - const result = new Sequence( + const passed = new Sequence( [ pass('a'), pass('b'), @@ -110,7 +110,7 @@ test('all sync - no failure - bail', t => { true ).run(); - t.strictDeepEqual(result, {passed: true}); + t.equal(passed, true); t.strictDeepEqual(results, [ { passed: true, @@ -129,7 +129,7 @@ test('all sync - no failure - bail', t => { }); test('all sync - begin failure - no bail', t => { - const result = new Sequence( + const passed = new Sequence( [ fail('a'), pass('b'), @@ -138,7 +138,7 @@ test('all sync - begin failure - no bail', t => { false ).run(); - t.strictDeepEqual(result, {passed: false}); + t.equal(passed, false); t.strictDeepEqual(results, [ { passed: false, @@ -157,7 +157,7 @@ test('all sync - begin failure - no bail', t => { }); test('all sync - mid failure - no bail', t => { - const result = new Sequence( + const passed = new Sequence( [ pass('a'), fail('b'), @@ -166,7 +166,7 @@ test('all sync - mid failure - no bail', t => { false ).run(); - t.strictDeepEqual(result, {passed: false}); + t.equal(passed, false); t.strictDeepEqual(results, [ { passed: true, @@ -184,7 +184,7 @@ test('all sync - mid failure - no bail', t => { }); test('all sync - end failure - no bail', t => { - const result = new Sequence( + const passed = new Sequence( [ pass('a'), pass('b'), @@ -193,7 +193,7 @@ test('all sync - end failure - no bail', t => { false ).run(); - t.strictDeepEqual(result, {passed: false}); + t.equal(passed, false); t.strictDeepEqual(results, [ { passed: true, @@ -212,7 +212,7 @@ test('all sync - end failure - no bail', t => { }); test('all sync - multiple failure - no bail', t => { - const result = new Sequence( + const passed = new Sequence( [ fail('a'), pass('b'), @@ -221,7 +221,7 @@ test('all sync - multiple failure - no bail', t => { false ).run(); - t.strictDeepEqual(result, {passed: false}); + t.equal(passed, false); t.strictDeepEqual(results, [ { passed: false, @@ -240,7 +240,7 @@ test('all sync - multiple failure - no bail', t => { }); test('all sync - begin failure - bail', t => { - const result = new Sequence( + const passed = new Sequence( [ fail('a'), pass('b'), @@ -249,7 +249,7 @@ test('all sync - begin failure - bail', t => { true ).run(); - t.strictDeepEqual(result, {passed: false}); + t.equal(passed, false); t.strictDeepEqual(results, [ { passed: false, @@ -260,7 +260,7 @@ test('all sync - begin failure - bail', t => { }); test('all sync - mid failure - bail', t => { - const result = new Sequence( + const passed = new Sequence( [ pass('a'), fail('b'), @@ -269,7 +269,7 @@ test('all sync - mid failure - bail', t => { true ).run(); - t.strictDeepEqual(result, {passed: false}); + t.equal(passed, false); t.strictDeepEqual(results, [ { passed: true, @@ -284,7 +284,7 @@ test('all sync - mid failure - bail', t => { }); test('all sync - end failure - bail', t => { - const result = new Sequence( + const passed = new Sequence( [ pass('a'), pass('b'), @@ -293,7 +293,7 @@ test('all sync - end failure - bail', t => { true ).run(); - t.strictDeepEqual(result, {passed: false}); + t.equal(passed, false); t.strictDeepEqual(results, [ { passed: true, @@ -319,8 +319,8 @@ test('all async - no failure - no bail', t => { passAsync('c') ], false - ).run().then(result => { - t.strictDeepEqual(result, {passed: true}); + ).run().then(passed => { + t.equal(passed, true); t.strictDeepEqual(results, [ { passed: true, @@ -347,8 +347,8 @@ test('all async - no failure - bail', t => { passAsync('c') ], true - ).run().then(result => { - t.strictDeepEqual(result, {passed: true}); + ).run().then(passed => { + t.equal(passed, true); t.strictDeepEqual(results, [ { passed: true, @@ -375,8 +375,8 @@ test('last async - no failure - no bail', t => { passAsync('c') ], false - ).run().then(result => { - t.strictDeepEqual(result, {passed: true}); + ).run().then(passed => { + t.equal(passed, true); t.strictDeepEqual(results, [ { passed: true, @@ -403,8 +403,8 @@ test('mid async - no failure - no bail', t => { pass('c') ], false - ).run().then(result => { - t.strictDeepEqual(result, {passed: true}); + ).run().then(passed => { + t.equal(passed, true); t.strictDeepEqual(results, [ { passed: true, @@ -431,8 +431,8 @@ test('first async - no failure - no bail', t => { pass('c') ], false - ).run().then(result => { - t.strictDeepEqual(result, {passed: true}); + ).run().then(passed => { + t.equal(passed, true); t.strictDeepEqual(results, [ { passed: true, @@ -459,8 +459,8 @@ test('last async - no failure - bail', t => { passAsync('c') ], true - ).run().then(result => { - t.strictDeepEqual(result, {passed: true}); + ).run().then(passed => { + t.equal(passed, true); t.strictDeepEqual(results, [ { passed: true, @@ -487,8 +487,8 @@ test('mid async - no failure - bail', t => { pass('c') ], true - ).run().then(result => { - t.strictDeepEqual(result, {passed: true}); + ).run().then(passed => { + t.equal(passed, true); t.strictDeepEqual(results, [ { passed: true, @@ -515,8 +515,8 @@ test('first async - no failure - bail', t => { pass('c') ], true - ).run().then(result => { - t.strictDeepEqual(result, {passed: true}); + ).run().then(passed => { + t.equal(passed, true); t.strictDeepEqual(results, [ { passed: true, @@ -543,8 +543,8 @@ test('all async - begin failure - bail', t => { passAsync('c') ], true - ).run().then(result => { - t.strictDeepEqual(result, {passed: false}); + ).run().then(passed => { + t.equal(passed, false); t.strictDeepEqual(results, [ { passed: false, @@ -563,8 +563,8 @@ test('all async - mid failure - bail', t => { passAsync('c') ], true - ).run().then(result => { - t.strictDeepEqual(result, {passed: false}); + ).run().then(passed => { + t.equal(passed, false); t.strictDeepEqual(results, [ { passed: true, @@ -587,8 +587,8 @@ test('all async - end failure - bail', t => { failAsync('c') ], true - ).run().then(result => { - t.strictDeepEqual(result, {passed: false}); + ).run().then(passed => { + t.equal(passed, false); t.strictDeepEqual(results, [ { passed: true, @@ -615,8 +615,8 @@ test('all async - begin failure - no bail', t => { passAsync('c') ], false - ).run().then(result => { - t.strictDeepEqual(result, {passed: false}); + ).run().then(passed => { + t.equal(passed, false); t.strictDeepEqual(results, [ { passed: false, @@ -643,8 +643,8 @@ test('all async - mid failure - no bail', t => { passAsync('c') ], false - ).run().then(result => { - t.strictDeepEqual(result, {passed: false}); + ).run().then(passed => { + t.equal(passed, false); t.strictDeepEqual(results, [ { passed: true, @@ -671,8 +671,8 @@ test('all async - end failure - no bail', t => { failAsync('c') ], false - ).run().then(result => { - t.strictDeepEqual(result, {passed: false}); + ).run().then(passed => { + t.equal(passed, false); t.strictDeepEqual(results, [ { passed: true, @@ -699,8 +699,8 @@ test('all async - multiple failure - no bail', t => { failAsync('c') ], false - ).run().then(result => { - t.strictDeepEqual(result, {passed: false}); + ).run().then(passed => { + t.equal(passed, false); t.strictDeepEqual(results, [ { passed: false, @@ -755,12 +755,12 @@ test('needs at least one sequence runnable', t => { }); test('sequences of sequences', t => { - const result = new Sequence([ + const passed = new Sequence([ new Sequence([pass('a'), pass('b')]), new Sequence([pass('c')]) ]).run(); - t.strictDeepEqual(result, {passed: true}); + t.equal(passed, true); t.strictDeepEqual(results, [ { passed: true, diff --git a/test/test-collection.js b/test/test-collection.js index ad3566727..b19a6ee75 100644 --- a/test/test-collection.js +++ b/test/test-collection.js @@ -274,9 +274,8 @@ test('foo', t => { add('after2', {type: 'after'}); add('before2', {type: 'before'}); - const result = collection.build().run(); - - t.is(result.passed, true); + const passed = collection.build().run(); + t.is(passed, true); t.strictDeepEqual(log, [ 'before1', @@ -339,9 +338,8 @@ test('foo', t => { collection.on('test', logger); - const result = collection.build().run(); - - t.is(result.passed, true); + const passed = collection.build().run(); + t.is(passed, true); t.strictDeepEqual(log, [ 'before1', diff --git a/test/test.js b/test/test.js index 93412f917..28b66b876 100644 --- a/test/test.js +++ b/test/test.js @@ -7,8 +7,8 @@ const Test = require('../lib/test'); const failingTestHint = 'Test was expected to fail, but succeeded, you should stop marking the test as failing'; -function ava(title, fn, contextRef, report) { - const t = new Test(title, fn, contextRef, report); +function ava(title, fn, contextRef, onResult) { + const t = new Test(title, fn, contextRef, onResult); t.metadata = { callback: false @@ -17,8 +17,8 @@ function ava(title, fn, contextRef, report) { return t; } -ava.failing = (title, fn, contextRef, report) => { - const t = new Test(title, fn, contextRef, report); +ava.failing = (title, fn, contextRef, onResult) => { + const t = new Test(title, fn, contextRef, onResult); t.metadata = { callback: false, @@ -28,8 +28,8 @@ ava.failing = (title, fn, contextRef, report) => { return t; }; -ava.cb = (title, fn, contextRef, report) => { - const t = new Test(title, fn, contextRef, report); +ava.cb = (title, fn, contextRef, onResult) => { + const t = new Test(title, fn, contextRef, onResult); t.metadata = { callback: true @@ -38,8 +38,8 @@ ava.cb = (title, fn, contextRef, report) => { return t; }; -ava.cb.failing = (title, fn, contextRef, report) => { - const t = new Test(title, fn, contextRef, report); +ava.cb.failing = (title, fn, contextRef, onResult) => { + const t = new Test(title, fn, contextRef, onResult); t.metadata = { callback: true, @@ -50,20 +50,23 @@ ava.cb.failing = (title, fn, contextRef, report) => { }; test('run test', t => { - const result = ava('foo', a => { + const passed = ava('foo', a => { a.fail(); }).run(); - t.is(result.passed, false); + t.is(passed, false); t.end(); }); test('title is optional', t => { - const result = ava(a => { + let result; + const passed = ava(a => { a.pass(); + }, null, null, r => { + result = r; }).run(); - t.is(result.passed, true); + t.is(passed, true); t.is(result.result.title, '[anonymous]'); t.end(); }); @@ -81,48 +84,60 @@ test('callback is required', t => { }); test('infer name from function', t => { - const result = ava(function foo(a) { // eslint-disable-line func-names, prefer-arrow-callback + let result; + const passed = ava(function foo(a) { // eslint-disable-line func-names, prefer-arrow-callback a.pass(); + }, null, null, r => { + result = r; }).run(); - t.is(result.passed, true); + t.is(passed, true); t.is(result.result.title, 'foo'); t.end(); }); test('multiple asserts', t => { - const result = ava(a => { + let result; + const passed = ava(a => { a.pass(); a.pass(); a.pass(); + }, null, null, r => { + result = r; }).run(); - t.is(result.passed, true); + t.is(passed, true); t.is(result.result.assertCount, 3); t.end(); }); test('plan assertions', t => { - const result = ava(a => { + let result; + const passed = ava(a => { a.plan(2); a.pass(); a.pass(); + }, null, null, r => { + result = r; }).run(); - t.is(result.passed, true); + t.is(passed, true); t.is(result.result.planCount, 2); t.is(result.result.assertCount, 2); t.end(); }); test('run more assertions than planned', t => { - const result = ava(a => { + let result; + const passed = ava(a => { a.plan(2); a.pass(); a.pass(); a.pass(); + }, null, null, r => { + result = r; }).run(); - t.is(result.passed, false); + t.is(passed, false); t.ok(result.reason); t.match(result.reason.message, /Planned for 2 assertions, but got 3\./); t.is(result.reason.name, 'AssertionError'); @@ -131,11 +146,14 @@ test('run more assertions than planned', t => { test('wrap non-assertion errors', t => { const err = new Error(); - const result = ava(() => { + let result; + const passed = ava(() => { throw err; + }, null, null, r => { + result = r; }).run(); - t.is(result.passed, false); + t.is(passed, false); t.is(result.reason.message, 'Error thrown in test'); t.is(result.reason.name, 'AssertionError'); t.same(result.reason.values, [{label: 'Error:', formatted: formatValue(err)}]); @@ -145,18 +163,21 @@ test('wrap non-assertion errors', t => { test('end can be used as callback without maintaining thisArg', t => { ava.cb(a => { setTimeout(a.end); - }).run().then(result => { - t.is(result.passed, true); + }).run().then(passed => { + t.is(passed, true); t.end(); }); }); test('end can be used as callback with error', t => { const err = new Error('failed'); + let result; ava.cb(a => { a.end(err); - }).run().then(result => { - t.is(result.passed, false); + }, null, null, r => { + result = r; + }).run().then(passed => { + t.is(passed, false); t.is(result.reason.message, 'Callback called with an error'); t.is(result.reason.name, 'AssertionError'); t.same(result.reason.values, [{label: 'Error:', formatted: formatValue(err)}]); @@ -166,10 +187,13 @@ test('end can be used as callback with error', t => { test('end can be used as callback with a non-error as its error argument', t => { const nonError = {foo: 'bar'}; + let result; ava.cb(a => { a.end(nonError); - }).run().then(result => { - t.is(result.passed, false); + }, null, null, r => { + result = r; + }).run().then(passed => { + t.is(passed, false); t.ok(result.reason); t.is(result.reason.message, 'Callback called with an error'); t.is(result.reason.name, 'AssertionError'); @@ -180,39 +204,49 @@ test('end can be used as callback with a non-error as its error argument', t => test('handle non-assertion errors even when planned', t => { const err = new Error('bar'); - const result = ava(a => { + let result; + const passed = ava(a => { a.plan(1); throw err; + }, null, null, r => { + result = r; }).run(); - t.is(result.passed, false); + t.is(passed, false); t.is(result.reason.name, 'AssertionError'); t.is(result.reason.message, 'Error thrown in test'); t.end(); }); test('handle testing of arrays', t => { - const result = ava(a => { + let result; + const passed = ava(a => { a.deepEqual(['foo', 'bar'], ['foo', 'bar']); + }, null, null, r => { + result = r; }).run(); - t.is(result.passed, true); + t.is(passed, true); t.is(result.result.assertCount, 1); t.end(); }); test('handle falsy testing of arrays', t => { - const result = ava(a => { + let result; + const passed = ava(a => { a.notDeepEqual(['foo', 'bar'], ['foo', 'bar', 'cat']); + }, null, null, r => { + result = r; }).run(); - t.is(result.passed, true); + t.is(passed, true); t.is(result.result.assertCount, 1); t.end(); }); test('handle testing of objects', t => { - const result = ava(a => { + let result; + const passed = ava(a => { a.deepEqual({ foo: 'foo', bar: 'bar' @@ -220,15 +254,18 @@ test('handle testing of objects', t => { foo: 'foo', bar: 'bar' }); + }, null, null, r => { + result = r; }).run(); - t.is(result.passed, true); + t.is(passed, true); t.is(result.result.assertCount, 1); t.end(); }); test('handle falsy testing of objects', t => { - const result = ava(a => { + let result; + const passed = ava(a => { a.notDeepEqual({ foo: 'foo', bar: 'bar' @@ -237,9 +274,11 @@ test('handle falsy testing of objects', t => { bar: 'bar', cat: 'cake' }); + }, null, null, r => { + result = r; }).run(); - t.is(result.passed, true); + t.is(passed, true); t.is(result.result.assertCount, 1); t.end(); }); @@ -247,14 +286,14 @@ test('handle falsy testing of objects', t => { test('run functions after last planned assertion', t => { let i = 0; - const result = ava(a => { + const passed = ava(a => { a.plan(1); a.pass(); i++; }).run(); t.is(i, 1); - t.is(result.passed, true); + t.is(passed, true); t.end(); }); @@ -266,14 +305,15 @@ test('run async functions after last planned assertion', t => { a.pass(); a.end(); i++; - }).run().then(result => { + }).run().then(passed => { t.is(i, 1); - t.is(result.passed, true); + t.is(passed, true); t.end(); }); }); test('planned async assertion', t => { + let result; ava.cb(a => { a.plan(1); @@ -281,39 +321,48 @@ test('planned async assertion', t => { a.pass(); a.end(); }, 100); - }).run().then(result => { - t.is(result.passed, true); + }, null, null, r => { + result = r; + }).run().then(passed => { + t.is(passed, true); t.is(result.result.assertCount, 1); t.end(); }); }); test('async assertion with `.end()`', t => { + let result; ava.cb(a => { setTimeout(() => { a.pass(); a.end(); }, 100); - }).run().then(result => { - t.is(result.passed, true); + }, null, null, r => { + result = r; + }).run().then(passed => { + t.is(passed, true); t.is(result.result.assertCount, 1); t.end(); }); }); test('more assertions than planned should emit an assertion error', t => { - const result = ava(a => { + let result; + const passed = ava(a => { a.plan(1); a.pass(); a.pass(); + }, null, null, r => { + result = r; }).run(); - t.is(result.passed, false); + t.is(passed, false); t.is(result.reason.name, 'AssertionError'); t.end(); }); test('record test duration', t => { + let result; ava.cb(a => { a.plan(1); @@ -321,8 +370,10 @@ test('record test duration', t => { a.true(true); a.end(); }, 1234); - }).run().then(result => { - t.is(result.passed, true); + }, null, null, r => { + result = r; + }).run().then(passed => { + t.is(passed, true); t.true(result.result.duration >= 1000); t.end(); }); @@ -331,12 +382,15 @@ test('record test duration', t => { test('wait for test to end', t => { let avaTest; + let result; ava.cb(a => { a.plan(1); avaTest = a; - }).run().then(result => { - t.is(result.passed, true); + }, null, null, r => { + result = r; + }).run().then(passed => { + t.is(passed, true); t.is(result.result.planCount, 1); t.is(result.result.assertCount, 1); t.true(result.result.duration >= 1000); @@ -350,13 +404,16 @@ test('wait for test to end', t => { }); test('fails with the first assertError', t => { - const result = ava(a => { + let result; + const passed = ava(a => { a.plan(2); a.is(1, 2); a.is(3, 4); + }, null, null, r => { + result = r; }).run(); - t.is(result.passed, false); + t.is(passed, false); t.is(result.reason.name, 'AssertionError'); t.same(result.reason.values, [ {label: 'Actual:', formatted: formatValue(1)}, @@ -366,11 +423,14 @@ test('fails with the first assertError', t => { }); test('fails with thrown falsy value', t => { - const result = ava(() => { + let result; + const passed = ava(() => { throw 0; // eslint-disable-line no-throw-literal + }, null, null, r => { + result = r; }).run(); - t.is(result.passed, false); + t.is(passed, false); t.is(result.reason.message, 'Error thrown in test'); t.is(result.reason.name, 'AssertionError'); t.same(result.reason.values, [{label: 'Error:', formatted: formatValue(0)}]); @@ -379,11 +439,14 @@ test('fails with thrown falsy value', t => { test('fails with thrown non-error object', t => { const obj = {foo: 'bar'}; - const result = ava(() => { + let result; + const passed = ava(() => { throw obj; + }, null, null, r => { + result = r; }).run(); - t.is(result.passed, false); + t.is(passed, false); t.is(result.reason.message, 'Error thrown in test'); t.is(result.reason.name, 'AssertionError'); t.same(result.reason.values, [{label: 'Error:', formatted: formatValue(obj)}]); @@ -391,13 +454,16 @@ test('fails with thrown non-error object', t => { }); test('skipped assertions count towards the plan', t => { - const result = ava(a => { + let result; + const passed = ava(a => { a.plan(2); a.pass(); a.skip.fail(); + }, null, null, r => { + result = r; }).run(); - t.is(result.passed, true); + t.is(passed, true); t.is(result.result.planCount, 2); t.is(result.result.assertCount, 2); t.end(); @@ -405,14 +471,17 @@ test('skipped assertions count towards the plan', t => { test('throws and notThrows work with promises', t => { let asyncCalled = false; + let result; ava(a => { a.plan(2); a.throws(delay.reject(10, new Error('foo')), 'foo'); a.notThrows(delay(20).then(() => { asyncCalled = true; })); - }).run().then(result => { - t.is(result.passed, true); + }, null, null, r => { + result = r; + }).run().then(passed => { + t.is(passed, true); t.is(result.result.planCount, 2); t.is(result.result.assertCount, 2); t.is(asyncCalled, true); @@ -421,35 +490,44 @@ test('throws and notThrows work with promises', t => { }); test('end should not be called multiple times', t => { + let result; ava.cb(a => { a.end(); a.end(); - }).run().then(result => { - t.is(result.passed, false); + }, null, null, r => { + result = r; + }).run().then(passed => { + t.is(passed, false); t.is(result.reason.message, '`t.end()` called more than once'); t.end(); }); }); test('cb test that throws sync', t => { + let result; const err = new Error('foo'); - const result = ava.cb(() => { + const passed = ava.cb(() => { throw err; + }, null, null, r => { + result = r; }).run(); - t.is(result.passed, false); + t.is(passed, false); t.is(result.reason.message, 'Error thrown in test'); t.is(result.reason.name, 'AssertionError'); t.end(); }); test('waits for t.throws to resolve after t.end is called', t => { + let result; ava.cb(a => { a.plan(1); a.notThrows(delay(10), 'foo'); a.end(); - }).run().then(result => { - t.is(result.passed, true); + }, null, null, r => { + result = r; + }).run().then(passed => { + t.is(passed, true); t.is(result.result.planCount, 1); t.is(result.result.assertCount, 1); t.end(); @@ -457,12 +535,15 @@ test('waits for t.throws to resolve after t.end is called', t => { }); test('waits for t.throws to reject after t.end is called', t => { + let result; ava.cb(a => { a.plan(1); a.throws(delay.reject(10, new Error('foo')), 'foo'); a.end(); - }).run().then(result => { - t.is(result.passed, true); + }, null, null, r => { + result = r; + }).run().then(passed => { + t.is(passed, true); t.is(result.result.planCount, 1); t.is(result.result.assertCount, 1); t.end(); @@ -470,12 +551,15 @@ test('waits for t.throws to reject after t.end is called', t => { }); test('waits for t.throws to resolve after the promise returned from the test resolves', t => { + let result; ava(a => { a.plan(1); a.notThrows(delay(10), 'foo'); return Promise.resolve(); - }).run().then(result => { - t.is(result.passed, true); + }, null, null, r => { + result = r; + }).run().then(passed => { + t.is(passed, true); t.is(result.result.planCount, 1); t.is(result.result.assertCount, 1); t.end(); @@ -483,12 +567,15 @@ test('waits for t.throws to resolve after the promise returned from the test res }); test('waits for t.throws to reject after the promise returned from the test resolves', t => { + let result; ava(a => { a.plan(1); a.throws(delay.reject(10, new Error('foo')), 'foo'); return Promise.resolve(); - }).run().then(result => { - t.is(result.passed, true); + }, null, null, r => { + result = r; + }).run().then(passed => { + t.is(passed, true); t.is(result.result.planCount, 1); t.is(result.result.assertCount, 1); t.end(); @@ -496,14 +583,17 @@ test('waits for t.throws to reject after the promise returned from the test reso }); test('multiple resolving and rejecting promises passed to t.throws/t.notThrows', t => { + let result; ava(a => { a.plan(6); for (let i = 0; i < 3; i++) { a.throws(delay.reject(10, new Error('foo')), 'foo'); a.notThrows(delay(10), 'foo'); } - }).run().then(result => { - t.is(result.passed, true); + }, null, null, r => { + result = r; + }).run().then(passed => { + t.is(passed, true); t.is(result.result.planCount, 6); t.is(result.result.assertCount, 6); t.end(); @@ -511,6 +601,7 @@ test('multiple resolving and rejecting promises passed to t.throws/t.notThrows', }); test('number of assertions matches t.plan when the test exits, but before all promises resolve another is added', t => { + let result; ava(a => { a.plan(2); a.throws(delay.reject(10, new Error('foo')), 'foo'); @@ -518,8 +609,10 @@ test('number of assertions matches t.plan when the test exits, but before all pr setTimeout(() => { a.throws(Promise.reject(new Error('foo')), 'foo'); }, 5); - }).run().then(result => { - t.is(result.passed, false); + }, null, null, r => { + result = r; + }).run().then(passed => { + t.is(passed, false); t.is(result.reason.assertion, 'plan'); t.is(result.reason.operator, '==='); t.end(); @@ -527,6 +620,7 @@ test('number of assertions matches t.plan when the test exits, but before all pr }); test('number of assertions doesn\'t match plan when the test exits, but before all promises resolve another is added', t => { + let result; ava(a => { a.plan(3); a.throws(delay.reject(10, new Error('foo')), 'foo'); @@ -534,8 +628,10 @@ test('number of assertions doesn\'t match plan when the test exits, but before a setTimeout(() => { a.throws(Promise.reject(new Error('foo')), 'foo'); }, 5); - }).run().then(result => { - t.is(result.passed, false); + }, null, null, r => { + result = r; + }).run().then(passed => { + t.is(passed, false); t.is(result.reason.assertion, 'plan'); t.is(result.reason.operator, '==='); t.end(); @@ -547,8 +643,8 @@ test('assertions return promises', t => { a.plan(2); t.ok(isPromise(a.throws(Promise.reject(new Error('foo'))))); t.ok(isPromise(a.notThrows(Promise.resolve(true)))); - }).run().then(result => { - t.is(result.passed, true); + }).run().then(passed => { + t.is(passed, true); t.end(); }); }); @@ -564,62 +660,77 @@ test('contextRef', t => { }); test('it is an error to set context in a hook', t => { + let result; const avaTest = ava(a => { a.context = 'foo'; + }, null, null, r => { + result = r; }); avaTest.metadata.type = 'foo'; - const result = avaTest.run(); - t.is(result.passed, false); + const passed = avaTest.run(); + t.is(passed, false); t.match(result.reason.message, /`t\.context` is not available in foo tests/); t.end(); }); test('failing tests should fail', t => { - const result = ava.failing('foo', a => { + const passed = ava.failing('foo', a => { a.fail(); }).run(); - t.is(result.passed, true); + t.is(passed, true); t.end(); }); test('failing callback tests should end without error', t => { const err = new Error('failed'); + let result; ava.cb.failing(a => { a.end(err); - }).run().then(result => { - t.is(result.passed, true); + }, null, null, r => { + result = r; + }).run().then(passed => { + t.is(passed, true); t.is(result.reason, undefined); t.end(); }); }); test('failing tests must not pass', t => { - const result = ava.failing('foo', a => { + let result; + const passed = ava.failing('foo', a => { a.pass(); + }, null, r => { + result = r; }).run(); - t.is(result.passed, false); + t.is(passed, false); t.is(result.reason.message, failingTestHint); t.end(); }); test('failing callback tests must not pass', t => { + let result; ava.cb.failing(a => { a.end(); - }).run().then(result => { - t.is(result.passed, false); + }, null, null, r => { + result = r; + }).run().then(passed => { + t.is(passed, false); t.is(result.reason.message, failingTestHint); t.end(); }); }); test('failing tests must not return a fulfilled promise', t => { + let result; ava.failing(() => { return Promise.resolve(); - }).run().then(result => { - t.is(result.passed, false); + }, null, null, r => { + result = r; + }).run().then(passed => { + t.is(passed, false); t.is(result.reason.message, failingTestHint); t.end(); }); @@ -630,8 +741,8 @@ test('failing tests pass when returning a rejected promise', t => { a.plan(1); a.notThrows(delay(10), 'foo'); return Promise.reject(); - }).run().then(result => { - t.is(result.passed, true); + }).run().then(passed => { + t.is(passed, true); t.end(); }); }); @@ -639,17 +750,20 @@ test('failing tests pass when returning a rejected promise', t => { test('failing tests pass with `t.throws(nonThrowingPromise)`', t => { ava.failing(a => { a.throws(Promise.resolve(10)); - }).run().then(result => { - t.is(result.passed, true); + }).run().then(passed => { + t.is(passed, true); t.end(); }); }); test('failing tests fail with `t.notThrows(throws)`', t => { + let result; ava.failing(a => { a.notThrows(Promise.resolve('foo')); - }).run().then(result => { - t.is(result.passed, false); + }, null, null, r => { + result = r; + }).run().then(passed => { + t.is(passed, false); t.is(result.reason.message, failingTestHint); t.end(); }); From 455bfa870c93af5fc47640694e33a32486c949bd Mon Sep 17 00:00:00 2001 From: Mark Wubben Date: Thu, 16 Mar 2017 18:16:03 +0000 Subject: [PATCH 09/15] Refactor Test implementation * Do away with underscore prefixes. They were used inconsistently, and are generally not worth it * Keep `_test` prefixed in ExecutionContext, since that is exposed to calling code * Reorder methods * Assume all arguments are passed and are correct. They're already validated in `test-collection.js` *Pass metadata when instantiating Tests * Rewrite test finish logic. There's still a fair bit of interleaving due to the `test.cb()` API, but it's now a lot easier to understand. Due to the last change, tests with `t.end()` and only synchronous assertions end immediately. Previously they would end asynchronously due to a promise being in the completion chain. Similarly, returning a promise or observable for a `test.cb()` test immediately fails. --- lib/test-collection.js | 12 +- lib/test.js | 369 +++++++++++++++++++---------------------- test/observable.js | 24 ++- test/promise.js | 20 +-- test/test.js | 273 ++++++++++-------------------- 5 files changed, 282 insertions(+), 416 deletions(-) diff --git a/lib/test-collection.js b/lib/test-collection.js index 87b13211e..0e50be8c1 100644 --- a/lib/test-collection.js +++ b/lib/test-collection.js @@ -124,22 +124,14 @@ class TestCollection extends EventEmitter { context = null; } - const test = new Test(title, hook.fn, context, this._emitTestResult); - test.metadata = hook.metadata; - - return test; + return new Test(hook.metadata, title, hook.fn, context, this._emitTestResult); } _buildTest(test, context) { if (!context) { context = null; } - const metadata = test.metadata; - - test = new Test(test.title, test.fn, context, this._emitTestResult); - test.metadata = metadata; - - return test; + return new Test(test.metadata, test.title, test.fn, context, this._emitTestResult); } _buildTestWithHooks(test) { if (test.metadata.skipped || test.metadata.todo) { diff --git a/lib/test.js b/lib/test.js index fb84dcb9a..e92a21fdb 100644 --- a/lib/test.js +++ b/lib/test.js @@ -1,8 +1,6 @@ 'use strict'; const isGeneratorFn = require('is-generator-fn'); const maxTimeout = require('max-timeout'); -const Promise = require('bluebird'); -const fnName = require('fn-name'); const co = require('co-with-promise'); const observableToPromise = require('observable-to-promise'); const isPromise = require('is-promise'); @@ -33,26 +31,31 @@ class ExecutionContext { this._test = test; this.skip = new SkipApi(test); } + plan(ct) { this._test.plan(ct, captureStack(this.plan)); } + get end() { - const end = this._test.end; + const end = this._test.bindEndCallback(); const endFn = err => end(err, captureStack(endFn)); return endFn; } + get title() { return this._test.title; } + get context() { const contextRef = this._test.contextRef; return contextRef && contextRef.context; } + set context(context) { const contextRef = this._test.contextRef; if (!contextRef) { - this._test._setAssertError(new Error(`\`t.context\` is not available in ${this._test.metadata.type} tests`)); + this._test.saveFirstError(new Error(`\`t.context\` is not available in ${this._test.metadata.type} tests`)); return; } @@ -64,21 +67,21 @@ Object.defineProperty(ExecutionContext.prototype, 'context', {enumerable: true}) { const assertions = assert.wrapAssertions({ pass(executionContext) { - executionContext._test._assertionPassed(); + executionContext._test.countPassedAssertion(); }, pending(executionContext, promise) { - executionContext._test._assertionPending(promise); + executionContext._test.addPendingAssertion(promise); }, fail(executionContext, error) { - executionContext._test._assertionFailed(error); + executionContext._test.addFailedAssertion(error); } }); Object.assign(ExecutionContext.prototype, assertions); function skipFn() { - this._test._assertionPassed(); + this._test.countPassedAssertion(); } Object.keys(assertions).forEach(el => { SkipApi.prototype[el] = skipFn; @@ -86,61 +89,78 @@ Object.defineProperty(ExecutionContext.prototype, 'context', {enumerable: true}) } class Test { - constructor(title, fn, contextRef, report) { - if (typeof title === 'function') { - contextRef = fn; - fn = title; - title = null; - } - - if (typeof fn !== 'function') { - throw new TypeError('You must provide a callback'); - } - - this.title = title || fnName(fn) || '[anonymous]'; + constructor(metadata, title, fn, contextRef, onResult) { // eslint-disable-line max-params + this.metadata = metadata; + this.title = title; this.fn = isGeneratorFn(fn) ? co.wrap(fn) : fn; - this.pendingAssertions = []; + this.contextRef = contextRef; + this.onResult = onResult; + this.assertCount = 0; - this.planCount = null; - this.duration = null; this.assertError = undefined; - this.sync = true; - this.contextRef = contextRef; - this.report = report; - this.threwSync = false; + this.calledEnd = false; + this.duration = null; + this.endCallbackFinisher = null; + this.pendingAssertions = []; + this.planCount = null; + this.startedAt = 0; + } - // TODO(jamestalmage): Make this an optional constructor arg instead of having Runner set it after the fact. - // metadata should just always exist, otherwise it requires a bunch of ugly checks all over the place. - this.metadata = {}; + bindEndCallback() { + if (this.metadata.callback) { + return (err, stack) => { + this.endCallback(err, stack); + }; + } - // Store the time point before test execution - // to calculate the total time spent in test - this._timeStart = null; + throw new Error('`t.end()`` is not supported in this context. To use `t.end()` as a callback, you must use "callback mode" via `test.cb(testName, fn)`'); + } + + endCallback(err, stack) { + if (this.calledEnd) { + this.saveFirstError(new Error('`t.end()` called more than once')); + return; + } + this.calledEnd = true; + + if (err) { + this.saveFirstError(new assert.AssertionError({ + actual: err, + message: 'Callback called with an error', + stack, + values: [formatAssertError.formatWithLabel('Error:', err)] + })); + } - // Workaround for Babel giving anonymous functions a name - if (this.title === 'callee$0$0') { - this.title = '[anonymous]'; + if (this.endCallbackFinisher) { + this.endCallbackFinisher(); } } - _assertionPassed() { + + createExecutionContext() { + return new ExecutionContext(this); + } + + countPassedAssertion() { this.assertCount++; } - _assertionPending(promise) { - this.sync = false; + + addPendingAssertion(promise) { this.assertCount++; this.pendingAssertions.push(promise); } - _assertionFailed(error) { - this._setAssertError(error); + + addFailedAssertion(error) { this.assertCount++; + this.saveFirstError(error); } - _setAssertError(err) { - if (this.assertError !== undefined) { - return; - } - this.assertError = err; + saveFirstError(err) { + if (!this.assertError) { + this.assertError = err; + } } + plan(count, planStack) { if (typeof count !== 'number') { throw new TypeError('Expected a number'); @@ -148,103 +168,141 @@ class Test { this.planCount = count; - // In case the `planCount` doesn't match `assertCount, - // we need the stack of this function to throw with a useful stack + // In case the `planCount` doesn't match `assertCount, we need the stack of + // this function to throw with a useful stack. this.planStack = planStack; } - _run() { - let ret; + verifyPlan() { + if (!this.assertError && this.planCount !== null && this.planCount !== this.assertCount) { + this.saveFirstError(new assert.AssertionError({ + assertion: 'plan', + message: `Planned for ${this.planCount} ${plur('assertion', this.planCount)}, but got ${this.assertCount}.`, + operator: '===', + stack: this.planStack + })); + } + } + + callFn() { try { - ret = this.fn(this._createExecutionContext()); + return { + ok: true, + retval: this.fn(this.createExecutionContext()) + }; } catch (err) { - this.threwSync = true; - throwsHelper(err); - - let error = err; - if (!(err instanceof assert.AssertionError)) { - error = new assert.AssertionError({ - message: 'Error thrown in test', - stack: err instanceof Error && err.stack, - values: [formatAssertError.formatWithLabel('Error:', err)] - }); - } - this._setAssertError(error); + return { + ok: false, + error: err + }; } - - return ret; } - promise() { - if (!this._promise) { - this._promise = {}; - - this._promise.promise = new Promise((resolve, reject) => { - this._promise.resolve = resolve; - this._promise.reject = reject; - }).then(result => { - if (this.report) { - this.report(result); - } - return result.passed; - }); - } - return this._promise; - } run() { - if (this.metadata.callback) { - this.sync = false; - } - - this._timeStart = globals.now(); + this.startedAt = globals.now(); // Wait until all assertions are complete - this._timeout = globals.setTimeout(() => {}, maxTimeout); + this.timeoutHandle = globals.setTimeout(() => {}, maxTimeout); - let ret = this._run(); - let asyncType = 'promises'; + const result = this.callFn(); + if (!result.ok) { + throwsHelper(result.error); - if (isObservable(ret)) { - asyncType = 'observables'; - ret = observableToPromise(ret); + this.saveFirstError(new assert.AssertionError({ + message: 'Error thrown in test', + stack: result.error instanceof Error && result.error.stack, + values: [formatAssertError.formatWithLabel('Error:', result.error)] + })); + return this.finish(); } - if (isPromise(ret)) { - this.sync = false; + const returnedObservable = isObservable(result.retval); + const returnedPromise = isPromise(result.retval); - if (this.metadata.callback) { - this._setAssertError(new Error(`Do not return ${asyncType} from tests declared via \`test.cb(...)\`, if you want to return a promise simply declare the test via \`test(...)\``)); + let promise; + if (returnedObservable) { + promise = observableToPromise(result.retval); + } else if (returnedPromise) { + // `retval` can be any thenable, so convert to a proper promise. + promise = Promise.resolve(result.retval); + } + + if (this.metadata.callback) { + if (returnedObservable || returnedPromise) { + const asyncType = returnedObservable ? 'observables' : 'promises'; + this.saveFirstError(new Error(`Do not return ${asyncType} from tests declared via \`test.cb(...)\`, if you want to return a promise simply declare the test via \`test(...)\``)); + return this.finish(); + } + + if (this.calledEnd) { + return this.finish(); } - // Convert to a Bluebird promise - return Promise.resolve(ret).then( - () => this.exit(), + return new Promise(resolve => { + this.endCallbackFinisher = () => { + resolve(this.finishPromised()); + }; + }); + } + + if (promise) { + return promise.then( + () => this.finish(), err => { throwsHelper(err); - if (!(err instanceof assert.AssertionError)) { - err = new assert.AssertionError({ - message: 'Rejected promise returned by test', - stack: err instanceof Error && err.stack, - values: [formatAssertError.formatWithLabel('Rejection reason:', err)] - }); - } - - this._setAssertError(err); - return this.exit(); + this.saveFirstError(new assert.AssertionError({ + message: 'Rejected promise returned by test', + stack: err instanceof Error && err.stack, + values: [formatAssertError.formatWithLabel('Rejection reason:', err)] + })); + return this.finish(); } ); } - if (this.metadata.callback && !this.threwSync) { - return this.promise().promise; + return this.finish(); + } + + finish() { + if (this.pendingAssertions.length === 0) { + return this.finishImmediately(); } - return this.exit(); + this.verifyPlan(); + + // Consume errors, ensuring there are no unhandled rejections. + const consumedErrors = Promise.all(this.pendingAssertions) + .catch(err => this.saveFirstError(err)); + + // Don't wait if there is an error. + if (this.assertError) { + return this.completeFinish(); + } + + // Finish after potential errors from pending assertions have been consumed. + // Note that the plan must be verified again in case a new assertion was + // added. + return consumedErrors.then(() => this.finishImmediately()); } - _result() { + + finishPromised() { + return new Promise(resolve => { + resolve(this.finish()); + }); + } + + finishImmediately() { + this.verifyPlan(); + return this.completeFinish(); + } + + completeFinish() { + this.duration = globals.now() - this.startedAt; + globals.clearTimeout(this.timeoutHandle); + let reason = this.assertError; - let passed = reason === undefined; + let passed = !reason; if (this.metadata.failing) { passed = !passed; @@ -256,90 +314,13 @@ class Test { } } - return { + this.onResult({ passed, result: this, reason - }; - } - get end() { - if (this.metadata.callback) { - return this._end.bind(this); - } - - throw new Error('`t.end()`` is not supported in this context. To use `t.end()` as a callback, you must use "callback mode" via `test.cb(testName, fn)`'); - } - _end(err, stack) { - if (err) { - if (!(err instanceof assert.AssertionError)) { - err = new assert.AssertionError({ - actual: err, - message: 'Callback called with an error', - stack, - values: [formatAssertError.formatWithLabel('Error:', err)] - }); - } + }); - this._setAssertError(err); - this.exit(); - - return; - } - - if (this.endCalled) { - this._setAssertError(new Error('`t.end()` called more than once')); - return; - } - - this.endCalled = true; - this.exit(); - } - _checkPlanCount() { - if (this.assertError === undefined && this.planCount !== null && this.planCount !== this.assertCount) { - this._setAssertError(new assert.AssertionError({ - assertion: 'plan', - message: `Planned for ${this.planCount} ${plur('assertion', this.planCount)}, but got ${this.assertCount}.`, - operator: '===', - stack: this.planStack - })); - } - } - exit() { - this._checkPlanCount(); - - if (this.sync || this.threwSync) { - this.duration = globals.now() - this._timeStart; - globals.clearTimeout(this._timeout); - - const result = this._result(); - - if (this.report) { - this.report(result); - } - - return result.passed; - } - - Promise.all(this.pendingAssertions) - .catch(err => { - this._setAssertError(err); - }) - .finally(() => { - // Calculate total time spent in test - this.duration = globals.now() - this._timeStart; - - // Stop infinite timer - globals.clearTimeout(this._timeout); - - this._checkPlanCount(); - - this.promise().resolve(this._result()); - }); - - return this.promise().promise; - } - _createExecutionContext() { - return new ExecutionContext(this); + return passed; } } diff --git a/test/observable.js b/test/observable.js index 6955594ac..22a752930 100644 --- a/test/observable.js +++ b/test/observable.js @@ -4,38 +4,34 @@ const Test = require('../lib/test'); const Observable = require('zen-observable'); // eslint-disable-line import/order function ava(fn, onResult) { - const a = new Test(fn, null, null, onResult); - a.metadata = {callback: false}; - return a; + return new Test({callback: false}, '[anonymous]', fn, null, onResult); } ava.cb = function (fn, onResult) { - const a = new Test(fn, null, null, onResult); - a.metadata = {callback: true}; - return a; + return new Test({callback: true}, '[anonymous]', fn, null, onResult); }; test('returning an observable from a legacy async fn is an error', t => { let result; - ava.cb(a => { + const passed = ava.cb(a => { a.plan(2); const observable = Observable.of(); - setTimeout(() => { + setImmediate(() => { a.pass(); a.pass(); a.end(); - }, 200); + }); return observable; }, r => { result = r; - }).run().then(passed => { - t.is(passed, false); - t.match(result.reason.message, /Do not return observables/); - t.end(); - }); + }).run(); + + t.is(passed, false); + t.match(result.reason.message, /Do not return observables/); + t.end(); }); test('handle throws with thrown observable', t => { diff --git a/test/promise.js b/test/promise.js index 5fdbefe3a..d7d1dbee2 100644 --- a/test/promise.js +++ b/test/promise.js @@ -5,15 +5,11 @@ const formatValue = require('../lib/format-assert-error').formatValue; const Test = require('../lib/test'); function ava(fn, onResult) { - const a = new Test(fn, null, null, onResult); - a.metadata = {callback: false}; - return a; + return new Test({callback: false}, '[anonymous]', fn, null, onResult); } ava.cb = function (fn, onResult) { - const a = new Test(fn, null, null, onResult); - a.metadata = {callback: true}; - return a; + return new Test({callback: true}, '[anonymous]', fn, null, onResult); }; function pass() { @@ -32,7 +28,7 @@ function fail() { test('returning a promise from a legacy async fn is an error', t => { let result; - ava.cb(a => { + const passed = ava.cb(a => { a.plan(1); return Promise.resolve(true).then(() => { @@ -41,11 +37,11 @@ test('returning a promise from a legacy async fn is an error', t => { }); }, r => { result = r; - }).run().then(passed => { - t.is(passed, false); - t.match(result.reason.message, /Do not return promises/); - t.end(); - }); + }).run(); + + t.is(passed, false); + t.match(result.reason.message, /Do not return promises/); + t.end(); }); test('assertion plan is tested after returned promise resolves', t => { diff --git a/test/test.js b/test/test.js index 28b66b876..7437b09ef 100644 --- a/test/test.js +++ b/test/test.js @@ -6,51 +6,26 @@ const formatValue = require('../lib/format-assert-error').formatValue; const Test = require('../lib/test'); const failingTestHint = 'Test was expected to fail, but succeeded, you should stop marking the test as failing'; +const noop = () => {}; -function ava(title, fn, contextRef, onResult) { - const t = new Test(title, fn, contextRef, onResult); - - t.metadata = { - callback: false - }; - - return t; +function ava(fn, contextRef, onResult) { + return new Test({callback: false}, '[anonymous]', fn, contextRef, onResult || noop); } -ava.failing = (title, fn, contextRef, onResult) => { - const t = new Test(title, fn, contextRef, onResult); - - t.metadata = { - callback: false, - failing: true - }; - - return t; +ava.failing = (fn, contextRef, onResult) => { + return new Test({callback: false, failing: true}, '[anonymous]', fn, contextRef, onResult || noop); }; -ava.cb = (title, fn, contextRef, onResult) => { - const t = new Test(title, fn, contextRef, onResult); - - t.metadata = { - callback: true - }; - - return t; +ava.cb = (fn, contextRef, onResult) => { + return new Test({callback: true}, '[anonymous]', fn, contextRef, onResult || noop); }; -ava.cb.failing = (title, fn, contextRef, onResult) => { - const t = new Test(title, fn, contextRef, onResult); - - t.metadata = { - callback: true, - failing: true - }; - - return t; +ava.cb.failing = (fn, contextRef, onResult) => { + return new Test({callback: true, failing: true}, '[anonymous]', fn, contextRef, onResult || noop); }; test('run test', t => { - const passed = ava('foo', a => { + const passed = ava(a => { a.fail(); }).run(); @@ -58,51 +33,13 @@ test('run test', t => { t.end(); }); -test('title is optional', t => { - let result; - const passed = ava(a => { - a.pass(); - }, null, null, r => { - result = r; - }).run(); - - t.is(passed, true); - t.is(result.result.title, '[anonymous]'); - t.end(); -}); - -test('callback is required', t => { - t.throws(() => { - ava(); - }, /You must provide a callback/); - - t.throws(() => { - ava('title'); - }, /You must provide a callback/); - - t.end(); -}); - -test('infer name from function', t => { - let result; - const passed = ava(function foo(a) { // eslint-disable-line func-names, prefer-arrow-callback - a.pass(); - }, null, null, r => { - result = r; - }).run(); - - t.is(passed, true); - t.is(result.result.title, 'foo'); - t.end(); -}); - test('multiple asserts', t => { let result; const passed = ava(a => { a.pass(); a.pass(); a.pass(); - }, null, null, r => { + }, null, r => { result = r; }).run(); @@ -117,7 +54,7 @@ test('plan assertions', t => { a.plan(2); a.pass(); a.pass(); - }, null, null, r => { + }, null, r => { result = r; }).run(); t.is(passed, true); @@ -133,7 +70,7 @@ test('run more assertions than planned', t => { a.pass(); a.pass(); a.pass(); - }, null, null, r => { + }, null, r => { result = r; }).run(); @@ -149,7 +86,7 @@ test('wrap non-assertion errors', t => { let result; const passed = ava(() => { throw err; - }, null, null, r => { + }, null, r => { result = r; }).run(); @@ -172,34 +109,34 @@ test('end can be used as callback without maintaining thisArg', t => { test('end can be used as callback with error', t => { const err = new Error('failed'); let result; - ava.cb(a => { + const passed = ava.cb(a => { a.end(err); - }, null, null, r => { + }, null, r => { result = r; - }).run().then(passed => { - t.is(passed, false); - t.is(result.reason.message, 'Callback called with an error'); - t.is(result.reason.name, 'AssertionError'); - t.same(result.reason.values, [{label: 'Error:', formatted: formatValue(err)}]); - t.end(); - }); + }).run(); + + t.is(passed, false); + t.is(result.reason.message, 'Callback called with an error'); + t.is(result.reason.name, 'AssertionError'); + t.same(result.reason.values, [{label: 'Error:', formatted: formatValue(err)}]); + t.end(); }); test('end can be used as callback with a non-error as its error argument', t => { const nonError = {foo: 'bar'}; let result; - ava.cb(a => { + const passed = ava.cb(a => { a.end(nonError); - }, null, null, r => { + }, null, r => { result = r; - }).run().then(passed => { - t.is(passed, false); - t.ok(result.reason); - t.is(result.reason.message, 'Callback called with an error'); - t.is(result.reason.name, 'AssertionError'); - t.same(result.reason.values, [{label: 'Error:', formatted: formatValue(nonError)}]); - t.end(); - }); + }).run(); + + t.is(passed, false); + t.ok(result.reason); + t.is(result.reason.message, 'Callback called with an error'); + t.is(result.reason.name, 'AssertionError'); + t.same(result.reason.values, [{label: 'Error:', formatted: formatValue(nonError)}]); + t.end(); }); test('handle non-assertion errors even when planned', t => { @@ -208,7 +145,7 @@ test('handle non-assertion errors even when planned', t => { const passed = ava(a => { a.plan(1); throw err; - }, null, null, r => { + }, null, r => { result = r; }).run(); @@ -222,7 +159,7 @@ test('handle testing of arrays', t => { let result; const passed = ava(a => { a.deepEqual(['foo', 'bar'], ['foo', 'bar']); - }, null, null, r => { + }, null, r => { result = r; }).run(); @@ -235,7 +172,7 @@ test('handle falsy testing of arrays', t => { let result; const passed = ava(a => { a.notDeepEqual(['foo', 'bar'], ['foo', 'bar', 'cat']); - }, null, null, r => { + }, null, r => { result = r; }).run(); @@ -254,7 +191,7 @@ test('handle testing of objects', t => { foo: 'foo', bar: 'bar' }); - }, null, null, r => { + }, null, r => { result = r; }).run(); @@ -274,7 +211,7 @@ test('handle falsy testing of objects', t => { bar: 'bar', cat: 'cake' }); - }, null, null, r => { + }, null, r => { result = r; }).run(); @@ -283,35 +220,6 @@ test('handle falsy testing of objects', t => { t.end(); }); -test('run functions after last planned assertion', t => { - let i = 0; - - const passed = ava(a => { - a.plan(1); - a.pass(); - i++; - }).run(); - - t.is(i, 1); - t.is(passed, true); - t.end(); -}); - -test('run async functions after last planned assertion', t => { - let i = 0; - - ava.cb(a => { - a.plan(1); - a.pass(); - a.end(); - i++; - }).run().then(passed => { - t.is(i, 1); - t.is(passed, true); - t.end(); - }); -}); - test('planned async assertion', t => { let result; ava.cb(a => { @@ -321,7 +229,7 @@ test('planned async assertion', t => { a.pass(); a.end(); }, 100); - }, null, null, r => { + }, null, r => { result = r; }).run().then(passed => { t.is(passed, true); @@ -337,7 +245,7 @@ test('async assertion with `.end()`', t => { a.pass(); a.end(); }, 100); - }, null, null, r => { + }, null, r => { result = r; }).run().then(passed => { t.is(passed, true); @@ -352,7 +260,7 @@ test('more assertions than planned should emit an assertion error', t => { a.plan(1); a.pass(); a.pass(); - }, null, null, r => { + }, null, r => { result = r; }).run(); @@ -370,7 +278,7 @@ test('record test duration', t => { a.true(true); a.end(); }, 1234); - }, null, null, r => { + }, null, r => { result = r; }).run().then(passed => { t.is(passed, true); @@ -387,7 +295,7 @@ test('wait for test to end', t => { a.plan(1); avaTest = a; - }, null, null, r => { + }, null, r => { result = r; }).run().then(passed => { t.is(passed, true); @@ -409,7 +317,7 @@ test('fails with the first assertError', t => { a.plan(2); a.is(1, 2); a.is(3, 4); - }, null, null, r => { + }, null, r => { result = r; }).run(); @@ -426,7 +334,7 @@ test('fails with thrown falsy value', t => { let result; const passed = ava(() => { throw 0; // eslint-disable-line no-throw-literal - }, null, null, r => { + }, null, r => { result = r; }).run(); @@ -442,7 +350,7 @@ test('fails with thrown non-error object', t => { let result; const passed = ava(() => { throw obj; - }, null, null, r => { + }, null, r => { result = r; }).run(); @@ -459,7 +367,7 @@ test('skipped assertions count towards the plan', t => { a.plan(2); a.pass(); a.skip.fail(); - }, null, null, r => { + }, null, r => { result = r; }).run(); @@ -478,7 +386,7 @@ test('throws and notThrows work with promises', t => { a.notThrows(delay(20).then(() => { asyncCalled = true; })); - }, null, null, r => { + }, null, r => { result = r; }).run().then(passed => { t.is(passed, true); @@ -491,16 +399,16 @@ test('throws and notThrows work with promises', t => { test('end should not be called multiple times', t => { let result; - ava.cb(a => { + const passed = ava.cb(a => { a.end(); a.end(); - }, null, null, r => { + }, null, r => { result = r; - }).run().then(passed => { - t.is(passed, false); - t.is(result.reason.message, '`t.end()` called more than once'); - t.end(); - }); + }).run(); + + t.is(passed, false); + t.is(result.reason.message, '`t.end()` called more than once'); + t.end(); }); test('cb test that throws sync', t => { @@ -508,7 +416,7 @@ test('cb test that throws sync', t => { const err = new Error('foo'); const passed = ava.cb(() => { throw err; - }, null, null, r => { + }, null, r => { result = r; }).run(); @@ -524,7 +432,7 @@ test('waits for t.throws to resolve after t.end is called', t => { a.plan(1); a.notThrows(delay(10), 'foo'); a.end(); - }, null, null, r => { + }, null, r => { result = r; }).run().then(passed => { t.is(passed, true); @@ -540,7 +448,7 @@ test('waits for t.throws to reject after t.end is called', t => { a.plan(1); a.throws(delay.reject(10, new Error('foo')), 'foo'); a.end(); - }, null, null, r => { + }, null, r => { result = r; }).run().then(passed => { t.is(passed, true); @@ -556,7 +464,7 @@ test('waits for t.throws to resolve after the promise returned from the test res a.plan(1); a.notThrows(delay(10), 'foo'); return Promise.resolve(); - }, null, null, r => { + }, null, r => { result = r; }).run().then(passed => { t.is(passed, true); @@ -572,7 +480,7 @@ test('waits for t.throws to reject after the promise returned from the test reso a.plan(1); a.throws(delay.reject(10, new Error('foo')), 'foo'); return Promise.resolve(); - }, null, null, r => { + }, null, r => { result = r; }).run().then(passed => { t.is(passed, true); @@ -590,7 +498,7 @@ test('multiple resolving and rejecting promises passed to t.throws/t.notThrows', a.throws(delay.reject(10, new Error('foo')), 'foo'); a.notThrows(delay(10), 'foo'); } - }, null, null, r => { + }, null, r => { result = r; }).run().then(passed => { t.is(passed, true); @@ -609,7 +517,7 @@ test('number of assertions matches t.plan when the test exits, but before all pr setTimeout(() => { a.throws(Promise.reject(new Error('foo')), 'foo'); }, 5); - }, null, null, r => { + }, null, r => { result = r; }).run().then(passed => { t.is(passed, false); @@ -621,21 +529,21 @@ test('number of assertions matches t.plan when the test exits, but before all pr test('number of assertions doesn\'t match plan when the test exits, but before all promises resolve another is added', t => { let result; - ava(a => { + const passed = 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, null, r => { + }, null, r => { result = r; - }).run().then(passed => { - t.is(passed, false); - t.is(result.reason.assertion, 'plan'); - t.is(result.reason.operator, '==='); - t.end(); - }); + }).run(); + + t.is(passed, false); + t.is(result.reason.assertion, 'plan'); + t.is(result.reason.operator, '==='); + t.end(); }); test('assertions return promises', t => { @@ -650,12 +558,13 @@ test('assertions return promises', t => { }); test('contextRef', t => { - new Test('foo', + new Test({}, 'foo', a => { t.strictDeepEqual(a.context, {foo: 'bar'}); t.end(); }, - {context: {foo: 'bar'}} + {context: {foo: 'bar'}}, + () => {} ).run(); }); @@ -663,7 +572,7 @@ test('it is an error to set context in a hook', t => { let result; const avaTest = ava(a => { a.context = 'foo'; - }, null, null, r => { + }, null, r => { result = r; }); avaTest.metadata.type = 'foo'; @@ -685,21 +594,17 @@ test('failing tests should fail', t => { test('failing callback tests should end without error', t => { const err = new Error('failed'); - let result; - ava.cb.failing(a => { + const passed = ava.cb.failing(a => { a.end(err); - }, null, null, r => { - result = r; - }).run().then(passed => { - t.is(passed, true); - t.is(result.reason, undefined); - t.end(); - }); + }).run(); + + t.is(passed, true); + t.end(); }); test('failing tests must not pass', t => { let result; - const passed = ava.failing('foo', a => { + const passed = ava.failing(a => { a.pass(); }, null, r => { result = r; @@ -711,23 +616,19 @@ test('failing tests must not pass', t => { }); test('failing callback tests must not pass', t => { - let result; - ava.cb.failing(a => { + const passed = ava.cb.failing(a => { a.end(); - }, null, null, r => { - result = r; - }).run().then(passed => { - t.is(passed, false); - t.is(result.reason.message, failingTestHint); - t.end(); - }); + }).run(); + + t.is(passed, false); + t.end(); }); test('failing tests must not return a fulfilled promise', t => { let result; ava.failing(() => { return Promise.resolve(); - }, null, null, r => { + }, null, r => { result = r; }).run().then(passed => { t.is(passed, false); @@ -760,7 +661,7 @@ test('failing tests fail with `t.notThrows(throws)`', t => { let result; ava.failing(a => { a.notThrows(Promise.resolve('foo')); - }, null, null, r => { + }, null, r => { result = r; }).run().then(passed => { t.is(passed, false); From a710eab19e4742784801131e04c382c7bd0a8483 Mon Sep 17 00:00:00 2001 From: Mark Wubben Date: Fri, 17 Mar 2017 15:09:46 +0000 Subject: [PATCH 10/15] Try to fail test if assertions are added after test finishes Though currently this error is likely to get lost unless there is a pending assertion or `test.cb()` is used. --- lib/test.js | 29 ++++++++++++++++++----------- test/test.js | 27 +++++++++++++++++++++++---- 2 files changed, 41 insertions(+), 15 deletions(-) diff --git a/lib/test.js b/lib/test.js index e92a21fdb..d4908e64f 100644 --- a/lib/test.js +++ b/lib/test.js @@ -101,6 +101,7 @@ class Test { this.calledEnd = false; this.duration = null; this.endCallbackFinisher = null; + this.finishing = false; this.pendingAssertions = []; this.planCount = null; this.startedAt = 0; @@ -142,15 +143,27 @@ class Test { } countPassedAssertion() { + if (this.finishing) { + this.saveFirstError(new Error('Assertion passed, but test has already ended')); + } + this.assertCount++; } addPendingAssertion(promise) { + if (this.finishing) { + this.saveFirstError(new Error('Assertion passed, but test has already ended')); + } + this.assertCount++; this.pendingAssertions.push(promise); } addFailedAssertion(error) { + if (this.finishing) { + this.saveFirstError(new Error('Assertion failed, but test has already ended')); + } + this.assertCount++; this.saveFirstError(error); } @@ -265,12 +278,13 @@ class Test { } finish() { + this.finishing = true; + this.verifyPlan(); + if (this.pendingAssertions.length === 0) { - return this.finishImmediately(); + return this.completeFinish(); } - this.verifyPlan(); - // Consume errors, ensuring there are no unhandled rejections. const consumedErrors = Promise.all(this.pendingAssertions) .catch(err => this.saveFirstError(err)); @@ -281,9 +295,7 @@ class Test { } // Finish after potential errors from pending assertions have been consumed. - // Note that the plan must be verified again in case a new assertion was - // added. - return consumedErrors.then(() => this.finishImmediately()); + return consumedErrors.then(() => this.completeFinish()); } finishPromised() { @@ -292,11 +304,6 @@ class Test { }); } - finishImmediately() { - this.verifyPlan(); - return this.completeFinish(); - } - completeFinish() { this.duration = globals.now() - this.startedAt; globals.clearTimeout(this.timeoutHandle); diff --git a/test/test.js b/test/test.js index 7437b09ef..cd93d1f3f 100644 --- a/test/test.js +++ b/test/test.js @@ -508,21 +508,40 @@ test('multiple resolving and rejecting promises passed to t.throws/t.notThrows', }); }); -test('number of assertions matches t.plan when the test exits, but before all promises resolve another is added', t => { +test('number of assertions matches t.plan when the test exits, but before all pending assertions resolve another is added', t => { let result; ava(a => { a.plan(2); a.throws(delay.reject(10, new Error('foo')), 'foo'); a.notThrows(delay(10), 'foo'); setTimeout(() => { - a.throws(Promise.reject(new Error('foo')), 'foo'); + a.pass(); + }, 5); + }, null, r => { + 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.end(); + }); +}); + +test('number of assertions matches t.plan when the test exits, but before all pending assertions resolve, a failing assertion is added', t => { + let result; + ava(a => { + a.plan(2); + a.throws(delay.reject(10, new Error('foo')), 'foo'); + a.notThrows(delay(10), 'foo'); + setTimeout(() => { + a.fail(); }, 5); }, null, r => { result = r; }).run().then(passed => { t.is(passed, false); - t.is(result.reason.assertion, 'plan'); - t.is(result.reason.operator, '==='); + t.match(result.reason.message, /Assertion failed, but test has already ended/); + t.is(result.reason.name, 'Error'); t.end(); }); }); From 120d2b241e6d3f97f5f73fd32ab16722dbe408ee Mon Sep 17 00:00:00 2001 From: Mark Wubben Date: Fri, 17 Mar 2017 10:31:29 +0000 Subject: [PATCH 11/15] Fail tests that finish without running assertions --- lib/cli.js | 1 + lib/main.js | 1 + lib/runner.js | 3 +- lib/test-collection.js | 5 +- lib/test.js | 10 ++- readme.md | 5 ++ test/api.js | 10 +-- test/cli.js | 7 ++ test/fixture/_generate_source-map-initial.js | 6 +- test/fixture/hooks-passing.js | 4 +- test/fixture/long-running.js | 2 +- .../fail-without-assertions/package.json | 5 ++ .../pkg-conf/fail-without-assertions/test.js | 3 + test/fixture/source-map-file-browser-env.js | 3 +- test/fixture/source-map-file.js | 3 +- test/fixture/source-map-initial.js | 20 +++--- test/fixture/source-map-initial.js.map | 2 +- test/fixture/throw-anonymous-function.js | 3 +- test/fixture/throw-named-function.js | 3 +- test/fixture/uncaught-exception.js | 3 +- test/hooks.js | 39 ++++++---- test/observable.js | 4 +- test/promise.js | 4 +- test/runner.js | 71 +++++++++++++------ test/test-collection.js | 3 +- test/test.js | 47 ++++++++++-- 26 files changed, 195 insertions(+), 72 deletions(-) create mode 100644 test/fixture/pkg-conf/fail-without-assertions/package.json create mode 100644 test/fixture/pkg-conf/fail-without-assertions/test.js diff --git a/lib/cli.js b/lib/cli.js index 85c5a8c06..57408bf60 100644 --- a/lib/cli.js +++ b/lib/cli.js @@ -126,6 +126,7 @@ exports.run = () => { const api = new Api({ failFast: conf.failFast, + failWithoutAssertions: conf.failWithoutAssertions !== false, serial: conf.serial, require: arrify(conf.require), cacheEnabled: conf.cache !== false, diff --git a/lib/main.js b/lib/main.js index 9a5da627e..d21cd3256 100644 --- a/lib/main.js +++ b/lib/main.js @@ -7,6 +7,7 @@ const Runner = require('./runner'); const opts = globals.options; const runner = new Runner({ + failWithoutAssertions: opts.failWithoutAssertions, serial: opts.serial, bail: opts.failFast, match: opts.match diff --git a/lib/runner.js b/lib/runner.js index 839100813..9df270a5c 100644 --- a/lib/runner.js +++ b/lib/runner.js @@ -47,7 +47,8 @@ class Runner extends EventEmitter { this.results = []; this.tests = new TestCollection({ - bail: options.bail + bail: options.bail, + failWithoutAssertions: options.failWithoutAssertions }); this.hasStarted = false; this._serial = options.serial; diff --git a/lib/test-collection.js b/lib/test-collection.js index 0e50be8c1..3af9c1009 100644 --- a/lib/test-collection.js +++ b/lib/test-collection.js @@ -10,6 +10,7 @@ class TestCollection extends EventEmitter { super(); this.bail = options.bail; + this.failWithoutAssertions = options.failWithoutAssertions; this.hasExclusive = false; this.testCount = 0; @@ -124,14 +125,14 @@ class TestCollection extends EventEmitter { context = null; } - return new Test(hook.metadata, title, hook.fn, context, this._emitTestResult); + return new Test(hook.metadata, title, hook.fn, false, context, this._emitTestResult); } _buildTest(test, context) { if (!context) { context = null; } - return new Test(test.metadata, test.title, test.fn, context, this._emitTestResult); + return new Test(test.metadata, test.title, test.fn, this.failWithoutAssertions, context, this._emitTestResult); } _buildTestWithHooks(test) { if (test.metadata.skipped || test.metadata.todo) { diff --git a/lib/test.js b/lib/test.js index d4908e64f..185804e66 100644 --- a/lib/test.js +++ b/lib/test.js @@ -89,10 +89,11 @@ Object.defineProperty(ExecutionContext.prototype, 'context', {enumerable: true}) } class Test { - constructor(metadata, title, fn, contextRef, onResult) { // eslint-disable-line max-params + constructor(metadata, title, fn, failWithoutAssertions, contextRef, onResult) { // eslint-disable-line max-params this.metadata = metadata; this.title = title; this.fn = isGeneratorFn(fn) ? co.wrap(fn) : fn; + this.failWithoutAssertions = failWithoutAssertions; this.contextRef = contextRef; this.onResult = onResult; @@ -197,6 +198,12 @@ class Test { } } + verifyAssertions() { + if (this.failWithoutAssertions && !this.assertError && !this.calledEnd && this.planCount === null && this.assertCount === 0) { + this.saveFirstError(new Error('Test finished without running any assertions')); + } + } + callFn() { try { return { @@ -280,6 +287,7 @@ class Test { finish() { this.finishing = true; this.verifyPlan(); + this.verifyAssertions(); if (this.pendingAssertions.length === 0) { return this.completeFinish(); diff --git a/readme.md b/readme.md index cc1c39aad..d6132eedb 100644 --- a/readme.md +++ b/readme.md @@ -262,6 +262,7 @@ All of the CLI options can be configured in the `ava` section of your `package.j ], "concurrency": 5, "failFast": true, + "failWithoutAssertions": false, "tap": true, "powerAssert": false, "require": [ @@ -326,6 +327,8 @@ test(function name(t) { Assertion plans ensure tests only pass when a specific number of assertions have been executed. They'll help you catch cases where tests exit too early. They'll also cause tests to fail if too many assertions are executed, which can be useful if you have assertions inside callbacks or loops. +If you do not specify an assertion plan, your test will still fail if no assertions are executed. Set the `failWithoutAssertions` option to `false` in AVA's [`package.json` configuration](#configuration) to disable this behavior. + Note that, unlike [`tap`](https://www.npmjs.com/package/tap) and [`tape`](https://www.npmjs.com/package/tape), AVA does *not* automatically end a test when the planned assertion count is reached. These examples will result in a passed test: @@ -673,6 +676,8 @@ You can use any assertion library instead of or in addition to the built-in one, This won't give you as nice an experience as you'd get with the [built-in assertions](#assertions) though, and you won't be able to use the [assertion planning](#assertion-planning) ([see #25](https://github.com/avajs/ava/issues/25)). +You'll have to configure AVA to not fail tests if no assertions are executed, because AVA can't tell if custom assertions pass. Set the `failWithoutAssertions` option to `false` in AVA's [`package.json` configuration](#configuration). + ```js import assert from 'assert'; diff --git a/test/api.js b/test/api.js index 6f459068e..e8143b948 100644 --- a/test/api.js +++ b/test/api.js @@ -373,7 +373,7 @@ function generateTests(prefix, apiCreator) { runStatus.on('error', data => { t.match(data.message, /Thrown by source-map-fixtures/); t.match(data.stack, /^.*?Object\.t.*?as run\b.*source-map-fixtures.src.throws.js:1.*$/m); - t.match(data.stack, /^.*?Immediate\b.*source-map-file.js:11.*$/m); + t.match(data.stack, /^.*?Immediate\b.*source-map-file.js:12.*$/m); }); }); @@ -394,7 +394,7 @@ function generateTests(prefix, apiCreator) { runStatus.on('error', data => { t.match(data.message, /Thrown by source-map-fixtures/); t.match(data.stack, /^.*?Object\.t.*?as run\b.*source-map-fixtures.src.throws.js:1.*$/m); - t.match(data.stack, /^.*?Immediate\b.*source-map-file-browser-env.js:14.*$/m); + t.match(data.stack, /^.*?Immediate\b.*source-map-file-browser-env.js:15.*$/m); }); }); @@ -415,7 +415,7 @@ function generateTests(prefix, apiCreator) { runStatus.on('error', data => { t.match(data.message, /Thrown by source-map-fixtures/); t.match(data.stack, /^.*?Object\.t.*?as run\b.*source-map-fixtures.src.throws.js:1.*$/m); - t.match(data.stack, /^.*?Immediate\b.*source-map-file.js:11.*$/m); + t.match(data.stack, /^.*?Immediate\b.*source-map-file.js:12.*$/m); }); }); @@ -436,7 +436,7 @@ function generateTests(prefix, apiCreator) { runStatus.on('error', data => { t.match(data.message, /Thrown by source-map-fixtures/); t.match(data.stack, /^.*?Object\.t.*?as run\b.*source-map-fixtures.src.throws.js:1.*$/m); - t.match(data.stack, /^.*?Immediate\b.*source-map-initial-input.js:7.*$/m); + t.match(data.stack, /^.*?Immediate\b.*source-map-initial-input.js:14.*$/m); }); }); @@ -457,7 +457,7 @@ function generateTests(prefix, apiCreator) { runStatus.on('error', data => { t.match(data.message, /Thrown by source-map-fixtures/); t.match(data.stack, /^.*?Object\.t.*?as run\b.*source-map-fixtures.src.throws.js:1.*$/m); - t.match(data.stack, /^.*?Immediate\b.*source-map-initial-input.js:7.*$/m); + t.match(data.stack, /^.*?Immediate\b.*source-map-initial-input.js:14.*$/m); }); }); diff --git a/test/cli.js b/test/cli.js index 021757221..2dd5b30e7 100644 --- a/test/cli.js +++ b/test/cli.js @@ -418,3 +418,10 @@ test('uncaught exceptions are raised for worker errors even if the error cannot t.end(); }); }); + +test('tests without assertions do not fail if failWithoutAssertions option is set to false', t => { + execCli([], {dirname: 'fixture/pkg-conf/fail-without-assertions'}, err => { + t.ifError(err); + t.end(); + }); +}); diff --git a/test/fixture/_generate_source-map-initial.js b/test/fixture/_generate_source-map-initial.js index 261caabbc..5a446d18d 100644 --- a/test/fixture/_generate_source-map-initial.js +++ b/test/fixture/_generate_source-map-initial.js @@ -15,11 +15,13 @@ const fixture = mapFile('throws').require(); // string. test('throw an uncaught exception', t => { setImmediate(run); + t.pass(); }) const run = () => fixture.run(); -`, { +`.trim(), { filename: 'source-map-initial-input.js', - sourceMaps: true + sourceMaps: true, + presets: ['@ava/stage-4'] }); fs.writeFileSync( diff --git a/test/fixture/hooks-passing.js b/test/fixture/hooks-passing.js index afdcb27fd..98f585012 100644 --- a/test/fixture/hooks-passing.js +++ b/test/fixture/hooks-passing.js @@ -6,4 +6,6 @@ test.after(pass); test.afterEach(pass); test(pass); -function pass() {} +function pass(t) { + t.pass(); +} diff --git a/test/fixture/long-running.js b/test/fixture/long-running.js index f10ae198c..1d0eb8312 100644 --- a/test/fixture/long-running.js +++ b/test/fixture/long-running.js @@ -4,4 +4,4 @@ test.cb('slow', t => { setTimeout(t.end, 5000); }); -test('fast', () => {}); +test('fast', t => t.pass()); diff --git a/test/fixture/pkg-conf/fail-without-assertions/package.json b/test/fixture/pkg-conf/fail-without-assertions/package.json new file mode 100644 index 000000000..4e0a9cf33 --- /dev/null +++ b/test/fixture/pkg-conf/fail-without-assertions/package.json @@ -0,0 +1,5 @@ +{ + "ava": { + "failWithoutAssertions": false + } +} diff --git a/test/fixture/pkg-conf/fail-without-assertions/test.js b/test/fixture/pkg-conf/fail-without-assertions/test.js new file mode 100644 index 000000000..32902f80b --- /dev/null +++ b/test/fixture/pkg-conf/fail-without-assertions/test.js @@ -0,0 +1,3 @@ +import test from '../../../..' + +test(() => {}) diff --git a/test/fixture/source-map-file-browser-env.js b/test/fixture/source-map-file-browser-env.js index 0306677fb..709ea4aec 100644 --- a/test/fixture/source-map-file-browser-env.js +++ b/test/fixture/source-map-file-browser-env.js @@ -7,8 +7,9 @@ global.XMLHttpRequest = function () {}; // The uncaught exception is passed to the corresponding cli test. The line // numbers from the 'throws' fixture (which uses a map file), as well as the // line of the fixture.run() call, should match the source lines. -test('throw an uncaught exception', () => { +test('throw an uncaught exception', t => { setImmediate(run); + t.pass(); }); const run = () => fixture.run(); diff --git a/test/fixture/source-map-file.js b/test/fixture/source-map-file.js index eded736c9..f39b507c0 100644 --- a/test/fixture/source-map-file.js +++ b/test/fixture/source-map-file.js @@ -4,8 +4,9 @@ const test = require('../../'); // The uncaught exception is passed to the corresponding cli test. The line // numbers from the 'throws' fixture (which uses a map file), as well as the // line of the fixture.run() call, should match the source lines. -test('throw an uncaught exception', () => { +test('throw an uncaught exception', t => { setImmediate(run); + t.pass(); }); const run = () => fixture.run(); diff --git a/test/fixture/source-map-initial.js b/test/fixture/source-map-initial.js index 3b0129d75..782605200 100644 --- a/test/fixture/source-map-initial.js +++ b/test/fixture/source-map-initial.js @@ -1,19 +1,23 @@ 'use strict'; -function _interopRequireDefault(obj) { return obj && obj.__esModule ? obj : { 'default': obj }; } - var _sourceMapFixtures = require('source-map-fixtures'); var _ = require('../../'); var _2 = _interopRequireDefault(_); -var fixture = (0, _sourceMapFixtures.mapFile)('throws').require(); -(0, _2['default'])('throw an uncaught exception', function (t) { - setImmediate(run); +function _interopRequireDefault(obj) { return obj && obj.__esModule ? obj : { default: obj }; } + +const fixture = (0, _sourceMapFixtures.mapFile)('throws').require(); + +// The uncaught exception is passed to the corresponding cli test. The line +// numbers from the 'throws' fixture (which uses a map file), as well as the +// line of the fixture.run() call, should match the source lines from this +// string. +(0, _2.default)('throw an uncaught exception', t => { + setImmediate(run); + t.pass(); }); -var run = function run() { - return fixture.run(); -}; +const run = () => fixture.run(); //# sourceMappingURL=./source-map-initial.js.map // Generated using node test/fixtures/_generate-source-map-initial.js diff --git a/test/fixture/source-map-initial.js.map b/test/fixture/source-map-initial.js.map index 66a7d7665..9f092b861 100644 --- a/test/fixture/source-map-initial.js.map +++ b/test/fixture/source-map-initial.js.map @@ -1 +1 @@ -{"version":3,"sources":["source-map-initial-input.js"],"names":[],"mappings":";;;;iCAAwB,qBAAqB;;gBAC5B,QAAQ;;;;AACzB,IAAM,OAAO,GAAG,gCAAQ,QAAQ,CAAC,CAAC,OAAO,EAAE,CAAA;AAC3C,mBAAK,6BAA6B,EAAE,UAAA,CAAC,EAAI;AACvC,cAAY,CAAC,GAAG,CAAC,CAAA;CAClB,CAAC,CAAA;AACF,IAAM,GAAG,GAAG,SAAN,GAAG;SAAS,OAAO,CAAC,GAAG,EAAE;CAAA,CAAA","file":"source-map-initial-input.js","sourcesContent":["import { mapFile } from 'source-map-fixtures'\nimport test from '../../'\nconst fixture = mapFile('throws').require()\ntest('throw an uncaught exception', t => {\n setImmediate(run)\n})\nconst run = () => fixture.run()"]} \ No newline at end of file +{"version":3,"sources":["source-map-initial-input.js"],"names":["fixture","require","t","setImmediate","run","pass"],"mappings":";;AAAA;;AACA;;;;;;AAEA,MAAMA,UAAU,gCAAQ,QAAR,EAAkBC,OAAlB,EAAhB;;AAEA;AACA;AACA;AACA;AACA,gBAAK,6BAAL,EAAoCC,KAAK;AACxCC,cAAaC,GAAb;AACAF,GAAEG,IAAF;AACA,CAHD;AAIA,MAAMD,MAAM,MAAMJ,QAAQI,GAAR,EAAlB","file":"source-map-initial-input.js","sourcesContent":["import {mapFile} from 'source-map-fixtures';\nimport test from '../../';\n\nconst fixture = mapFile('throws').require();\n\n// The uncaught exception is passed to the corresponding cli test. The line\n// numbers from the 'throws' fixture (which uses a map file), as well as the\n// line of the fixture.run() call, should match the source lines from this\n// string.\ntest('throw an uncaught exception', t => {\n\tsetImmediate(run);\n\tt.pass();\n})\nconst run = () => fixture.run();"]} \ No newline at end of file diff --git a/test/fixture/throw-anonymous-function.js b/test/fixture/throw-anonymous-function.js index a553e5ae3..6aa231cad 100644 --- a/test/fixture/throw-anonymous-function.js +++ b/test/fixture/throw-anonymous-function.js @@ -1,7 +1,8 @@ import test from '../../'; -test('throw an uncaught exception', () => { +test('throw an uncaught exception', t => { setImmediate(() => { throw () => {}; }); + t.pass(); }); diff --git a/test/fixture/throw-named-function.js b/test/fixture/throw-named-function.js index 4963e5583..4e4b79b16 100644 --- a/test/fixture/throw-named-function.js +++ b/test/fixture/throw-named-function.js @@ -2,8 +2,9 @@ import test from '../../'; function fooFn() {} -test('throw an uncaught exception', () => { +test('throw an uncaught exception', t => { setImmediate(() => { throw fooFn; }); + t.pass(); }); diff --git a/test/fixture/uncaught-exception.js b/test/fixture/uncaught-exception.js index c17afe6fc..2c110d45f 100644 --- a/test/fixture/uncaught-exception.js +++ b/test/fixture/uncaught-exception.js @@ -1,7 +1,8 @@ import test from '../../'; -test('throw an uncaught exception', () => { +test('throw an uncaught exception', t => { setImmediate(() => { throw new Error(`Can't catch me!`); }); + t.pass(); }); diff --git a/test/hooks.js b/test/hooks.js index 0981396a5..e45c9e3bd 100644 --- a/test/hooks.js +++ b/test/hooks.js @@ -38,7 +38,8 @@ test('before', t => { arr.push('a'); }); - runner.test(() => { + runner.test(a => { + a.pass(); arr.push('b'); }); @@ -57,7 +58,8 @@ test('after', t => { arr.push('b'); }); - runner.test(() => { + runner.test(a => { + a.pass(); arr.push('a'); }); @@ -146,6 +148,7 @@ test('stop if before hooks failed', t => { }); runner.test(a => { + a.pass(); arr.push('b'); a.end(); }); @@ -172,11 +175,13 @@ test('before each with concurrent tests', t => { arr[k++].push('b'); }); - runner.test(() => { + runner.test(a => { + a.pass(); arr[0].push('c'); }); - runner.test(() => { + runner.test(a => { + a.pass(); arr[1].push('d'); }); @@ -200,11 +205,13 @@ test('before each with serial tests', t => { arr.push('b'); }); - runner.serial(() => { + runner.serial(a => { + a.pass(); arr.push('c'); }); - runner.serial(() => { + runner.serial(a => { + a.pass(); arr.push('d'); }); @@ -253,11 +260,13 @@ test('after each with concurrent tests', t => { arr[k++].push('b'); }); - runner.test(() => { + runner.test(a => { + a.pass(); arr[0].push('c'); }); - runner.test(() => { + runner.test(a => { + a.pass(); arr[1].push('d'); }); @@ -281,11 +290,13 @@ test('after each with serial tests', t => { arr.push('b'); }); - runner.serial(() => { + runner.serial(a => { + a.pass(); arr.push('c'); }); - runner.serial(() => { + runner.serial(a => { + a.pass(); arr.push('d'); }); @@ -385,7 +396,8 @@ test('afterEach.always run even if beforeEach failed', t => { throw new Error('something went wrong'); }); - runner.test(() => { + runner.test(a => { + a.pass(); arr.push('a'); }); @@ -421,7 +433,8 @@ test('ensure hooks run only around tests', t => { arr.push('after'); }); - runner.test(() => { + runner.test(a => { + a.pass(); arr.push('test'); }); @@ -449,6 +462,7 @@ test('shared context', t => { }); runner.test(a => { + a.pass(); a.context.arr.push('b'); a.deepEqual(a.context.arr, ['a', 'b']); }); @@ -474,6 +488,7 @@ test('shared context of any type', t => { }); runner.test(a => { + a.pass(); a.is(a.context, 'foo'); }); diff --git a/test/observable.js b/test/observable.js index 22a752930..65338c2de 100644 --- a/test/observable.js +++ b/test/observable.js @@ -4,11 +4,11 @@ const Test = require('../lib/test'); const Observable = require('zen-observable'); // eslint-disable-line import/order function ava(fn, onResult) { - return new Test({callback: false}, '[anonymous]', fn, null, onResult); + return new Test({type: 'test', callback: false}, '[anonymous]', fn, true, null, onResult); } ava.cb = function (fn, onResult) { - return new Test({callback: true}, '[anonymous]', fn, null, onResult); + return new Test({type: 'test', callback: true}, '[anonymous]', fn, true, null, onResult); }; test('returning an observable from a legacy async fn is an error', t => { diff --git a/test/promise.js b/test/promise.js index d7d1dbee2..3a750f48d 100644 --- a/test/promise.js +++ b/test/promise.js @@ -5,11 +5,11 @@ const formatValue = require('../lib/format-assert-error').formatValue; const Test = require('../lib/test'); function ava(fn, onResult) { - return new Test({callback: false}, '[anonymous]', fn, null, onResult); + return new Test({type: 'test', callback: false}, '[anonymous]', fn, true, null, onResult); } ava.cb = function (fn, onResult) { - return new Test({callback: true}, '[anonymous]', fn, null, onResult); + return new Test({type: 'test', callback: true}, '[anonymous]', fn, true, null, onResult); }; function pass() { diff --git a/test/runner.js b/test/runner.js index f3e7c63e9..9742d3df1 100644 --- a/test/runner.js +++ b/test/runner.js @@ -10,10 +10,11 @@ test('nested tests and hooks aren\'t allowed', t => { const runner = new Runner(); - runner.test(() => { + runner.test(a => { t.throws(() => { runner.test(noop); }, {message: 'All tests and hooks must be declared synchronously in your test file, and cannot be nested within other tests or hooks.'}); + a.pass(); }); runner.run({}).then(() => { @@ -26,7 +27,10 @@ test('tests must be declared synchronously', t => { const runner = new Runner(); - runner.test(() => Promise.resolve()); + runner.test(a => { + a.pass(); + return Promise.resolve(); + }); runner.run({}); @@ -84,8 +88,9 @@ test('anything can be skipped', t => { const arr = []; function pusher(title) { - return () => { + return a => { arr.push(title); + a.pass(); }; } @@ -132,7 +137,7 @@ test('include skipped tests in results', t => { runner.beforeEach('beforeEach', noop); runner.beforeEach.skip('beforeEach.skip', noop); - runner.test.serial('test', noop); + runner.test.serial('test', a => a.pass()); runner.test.serial.skip('test.skip', noop); runner.after('after', noop); @@ -224,8 +229,9 @@ test('skip test', t => { const runner = new Runner(); const arr = []; - runner.test(() => { + runner.test(a => { arr.push('a'); + a.pass(); }); runner.skip(() => { @@ -261,8 +267,9 @@ test('todo test', t => { const runner = new Runner(); const arr = []; - runner.test(() => { + runner.test(a => { arr.push('a'); + a.pass(); }); runner.todo('todo'); @@ -290,12 +297,14 @@ test('only test', t => { const runner = new Runner(); const arr = []; - runner.test(() => { + runner.test(a => { arr.push('a'); + a.pass(); }); - runner.only(() => { + runner.only(a => { arr.push('b'); + a.pass(); }); runner.run({}).then(stats => { @@ -503,6 +512,7 @@ test('options.serial forces all tests to be serial', t => { arr.push(1); a.end(); }, 200); + a.pass(); }); runner.cb(a => { @@ -510,9 +520,11 @@ test('options.serial forces all tests to be serial', t => { arr.push(2); a.end(); }, 100); + a.pass(); }); - runner.test(() => { + runner.test(a => { + a.pass(); t.strictDeepEqual(arr, [1, 2]); t.end(); }); @@ -551,6 +563,7 @@ test('options.bail will bail out (async)', t => { a.fail(); a.end(); }, 100); + a.pass(); }); runner.cb(a => { @@ -558,6 +571,7 @@ test('options.bail will bail out (async)', t => { tests.push(2); a.end(); }, 300); + a.pass(); }); runner.run({}).then(() => { @@ -611,20 +625,24 @@ test('options.match will not run tests with non-matching titles', t => { match: ['*oo', '!foo'] }); - runner.test('mhm. grass tasty. moo', () => { + runner.test('mhm. grass tasty. moo', a => { t.pass(); + a.pass(); }); - runner.test('juggaloo', () => { + runner.test('juggaloo', a => { t.pass(); + a.pass(); }); - runner.test('foo', () => { + runner.test('foo', a => { t.fail(); + a.pass(); }); - runner.test(() => { + runner.test(a => { t.fail(); + a.pass(); }); runner.run({}).then(stats => { @@ -648,8 +666,9 @@ test('options.match hold no effect on hooks with titles', t => { actual = 'foo'; }); - runner.test('after', () => { + runner.test('after', a => { t.is(actual, 'foo'); + a.pass(); }); runner.run({}).then(stats => { @@ -667,12 +686,14 @@ test('options.match overrides .only', t => { match: ['*oo'] }); - runner.test('moo', () => { + runner.test('moo', a => { t.pass(); + a.pass(); }); - runner.test.only('boo', () => { + runner.test.only('boo', a => { t.pass(); + a.pass(); }); runner.run({}).then(stats => { @@ -688,8 +709,9 @@ test('macros: Additional args will be spread as additional args on implementatio const runner = new Runner(); - runner.test('test1', function () { + runner.test('test1', function (a) { t.deepEqual(slice.call(arguments, 1), ['foo', 'bar']); + a.pass(); }, 'foo', 'bar'); runner.run({}).then(stats => { @@ -717,6 +739,7 @@ test('macros: Customize test names attaching a `title` function', t => { function macroFn(avaT) { t.is(avaT.title, expectedTitles.shift()); t.deepEqual(slice.call(arguments, 1), expectedArgs.shift()); + avaT.pass(); } macroFn.title = (title, firstArg) => (title || 'default') + firstArg; @@ -739,6 +762,7 @@ test('match applies to macros', t => { function macroFn(avaT) { t.is(avaT.title, 'foobar'); + avaT.pass(); } macroFn.title = (title, firstArg) => `${firstArg}bar`; @@ -770,12 +794,14 @@ test('arrays of macros', t => { ['D'] ]; - function macroFnA() { + function macroFnA(a) { t.deepEqual(slice.call(arguments, 1), expectedArgsA.shift()); + a.pass(); } - function macroFnB() { + function macroFnB(a) { t.deepEqual(slice.call(arguments, 1), expectedArgsB.shift()); + a.pass(); } const runner = new Runner(); @@ -798,18 +824,21 @@ test('match applies to arrays of macros', t => { t.plan(3); // Foo - function fooMacro() { + function fooMacro(a) { t.fail(); + a.pass(); } fooMacro.title = (title, firstArg) => `${firstArg}foo`; function barMacro(avaT) { t.is(avaT.title, 'foobar'); + avaT.pass(); } barMacro.title = (title, firstArg) => `${firstArg}bar`; - function bazMacro() { + function bazMacro(a) { t.fail(); + a.pass(); } bazMacro.title = firstArg => `${firstArg}baz`; diff --git a/test/test-collection.js b/test/test-collection.js index b19a6ee75..69384b37a 100644 --- a/test/test-collection.js +++ b/test/test-collection.js @@ -245,6 +245,7 @@ test('foo', t => { function logger(a) { log.push(a.title); + a.pass(); } function add(title, opts) { @@ -313,7 +314,7 @@ test('foo', t => { collection.add({ title, metadata: metadata(opts), - fn: () => {} + fn: a => a.pass() }); } diff --git a/test/test.js b/test/test.js index cd93d1f3f..8920ad0af 100644 --- a/test/test.js +++ b/test/test.js @@ -9,19 +9,19 @@ const failingTestHint = 'Test was expected to fail, but succeeded, you should st const noop = () => {}; function ava(fn, contextRef, onResult) { - return new Test({callback: false}, '[anonymous]', fn, contextRef, onResult || noop); + return new Test({type: 'test', callback: false}, '[anonymous]', fn, true, contextRef, onResult || noop); } ava.failing = (fn, contextRef, onResult) => { - return new Test({callback: false, failing: true}, '[anonymous]', fn, contextRef, onResult || noop); + return new Test({type: 'test', callback: false, failing: true}, '[anonymous]', fn, true, contextRef, onResult || noop); }; ava.cb = (fn, contextRef, onResult) => { - return new Test({callback: true}, '[anonymous]', fn, contextRef, onResult || noop); + return new Test({type: 'test', callback: true}, '[anonymous]', fn, true, contextRef, onResult || noop); }; ava.cb.failing = (fn, contextRef, onResult) => { - return new Test({callback: true, failing: true}, '[anonymous]', fn, contextRef, onResult || noop); + return new Test({type: 'test', callback: true, failing: true}, '[anonymous]', fn, true, contextRef, onResult || noop); }; test('run test', t => { @@ -81,6 +81,31 @@ test('run more assertions than planned', t => { t.end(); }); +test('fails if no assertions are run', t => { + let result; + const passed = ava(() => {}, null, r => { + result = r; + }).run(); + + t.is(passed, false); + t.ok(result.reason); + t.is(result.reason.name, 'Error'); + t.match(result.reason.message, /Test finished without running any assertions/); + t.end(); +}); + +test('fails if no assertions are run, unless so planned', t => { + const passed = ava(a => a.plan(0)).run(); + t.is(passed, true); + t.end(); +}); + +test('fails if no assertions are run, unless an ended callback test', t => { + const passed = ava.cb(a => a.end()).run(); + t.is(passed, true); + t.end(); +}); + test('wrap non-assertion errors', t => { const err = new Error(); let result; @@ -99,6 +124,7 @@ test('wrap non-assertion errors', t => { test('end can be used as callback without maintaining thisArg', t => { ava.cb(a => { + a.pass(); setTimeout(a.end); }).run().then(passed => { t.is(passed, true); @@ -400,6 +426,7 @@ test('throws and notThrows work with promises', t => { test('end should not be called multiple times', t => { let result; const passed = ava.cb(a => { + a.pass(); a.end(); a.end(); }, null, r => { @@ -577,11 +604,13 @@ test('assertions return promises', t => { }); test('contextRef', t => { - new Test({}, 'foo', + new Test({type: 'test'}, 'foo', a => { + a.pass(); t.strictDeepEqual(a.context, {foo: 'bar'}); t.end(); }, + true, {context: {foo: 'bar'}}, () => {} ).run(); @@ -636,6 +665,7 @@ test('failing tests must not pass', t => { test('failing callback tests must not pass', t => { const passed = ava.cb.failing(a => { + a.pass(); a.end(); }).run(); @@ -645,8 +675,11 @@ test('failing callback tests must not pass', t => { test('failing tests must not return a fulfilled promise', t => { let result; - ava.failing(() => { - return Promise.resolve(); + ava.failing(a => { + return Promise.resolve() + .then(() => { + a.pass(); + }); }, null, r => { result = r; }).run().then(passed => { From 387461e4a171ed22d4a133e1374488ef27095035 Mon Sep 17 00:00:00 2001 From: Mark Wubben Date: Fri, 17 Mar 2017 17:35:14 +0000 Subject: [PATCH 12/15] Fail async tests if not ended before event loop empties Rather than keeping an infinite timer open, waiting for `t.end()` to be called, fail the callback test if it is not ended when the event loop empties. Similarly, fail promise/observable returning tests if the promise hasn't fulfilled or the observable hasn't completed when the event loop empties. Note that user code can keep the event loop busy, e.g. if it's listening on a socket or starts long timers. Even after this change, async tests may hang if the event loop is kept busy. --- lib/main.js | 9 ++++- lib/process-adapter.js | 4 +++ lib/sequence.js | 20 +++++++++-- lib/test-worker.js | 5 +++ lib/test.js | 45 ++++++++++++++---------- package.json | 1 - profile.js | 1 + test/cli.js | 21 +++++++++++ test/fixture/stalled-tests/callback.js | 5 +++ test/fixture/stalled-tests/observable.js | 8 +++++ test/fixture/stalled-tests/promise.js | 7 ++++ 11 files changed, 103 insertions(+), 23 deletions(-) create mode 100644 test/fixture/stalled-tests/callback.js create mode 100644 test/fixture/stalled-tests/observable.js create mode 100644 test/fixture/stalled-tests/promise.js diff --git a/lib/main.js b/lib/main.js index d21cd3256..69e6f25bc 100644 --- a/lib/main.js +++ b/lib/main.js @@ -49,8 +49,10 @@ function test(props) { } function exit() { - const stats = runner._buildStats(); + // Reference the IPC channel now that tests have finished running. + adapter.ipcChannel.ref(); + const stats = runner._buildStats(); adapter.send('results', {stats}); } @@ -71,6 +73,11 @@ globals.setImmediate(() => { runner.on('test', test); process.on('ava-run', options => { + // Unreference the IPC channel. This stops it from keeping the event loop + // busy, which means the `beforeExit` event can be used to detect when tests + // stall. + adapter.ipcChannel.unref(); + runner.run(options) .then(exit) .catch(err => { diff --git a/lib/process-adapter.js b/lib/process-adapter.js index 2959c3c36..b50f37398 100644 --- a/lib/process-adapter.js +++ b/lib/process-adapter.js @@ -22,6 +22,10 @@ exports.send = (name, data) => { }); }; +// `process.channel` was added in Node.js 7.1.0, but the channel was available +// through an undocumented API as `process._channel`. +exports.ipcChannel = process.channel || process._channel; + const opts = JSON.parse(process.argv[2]); exports.opts = opts; diff --git a/lib/sequence.js b/lib/sequence.js index cd3e1f7d8..237449a63 100644 --- a/lib/sequence.js +++ b/lib/sequence.js @@ -13,12 +13,26 @@ class Sequence { run() { const iterator = this.runnables[Symbol.iterator](); + let activeRunnable; + const onBeforeExit = () => { + if (activeRunnable.finishDueToInactivity) { + activeRunnable.finishDueToInactivity(); + } + }; + process.on('beforeExit', onBeforeExit); + let allPassed = true; + const finish = () => { + process.removeListener('beforeExit', onBeforeExit); + return allPassed; + }; + const runNext = () => { let promise; for (let next = iterator.next(); !next.done; next = iterator.next()) { - const passedOrPromise = next.value.run(); + activeRunnable = next.value; + const passedOrPromise = activeRunnable.run(); if (!passedOrPromise) { allPassed = false; @@ -33,7 +47,7 @@ class Sequence { } if (!promise) { - return allPassed; + return finish(); } return promise.then(passed => { @@ -42,7 +56,7 @@ class Sequence { if (this.bail) { // Stop if the test failed and bail mode is on. - return false; + return finish(); } } diff --git a/lib/test-worker.js b/lib/test-worker.js index 823dfbd2f..3f48c4bad 100644 --- a/lib/test-worker.js +++ b/lib/test-worker.js @@ -69,6 +69,11 @@ process.on('uncaughtException', exception => { stack: err.stack }; } + + // Ensure the IPC channel is refereced. The uncaught exception will kick off + // the teardown sequence, for which the messages must be received. + adapter.ipcChannel.ref(); + adapter.send('uncaughtException', {exception: serialized}); }); diff --git a/lib/test.js b/lib/test.js index 185804e66..bf7a1effb 100644 --- a/lib/test.js +++ b/lib/test.js @@ -1,6 +1,5 @@ 'use strict'; const isGeneratorFn = require('is-generator-fn'); -const maxTimeout = require('max-timeout'); const co = require('co-with-promise'); const observableToPromise = require('observable-to-promise'); const isPromise = require('is-promise'); @@ -102,6 +101,7 @@ class Test { this.calledEnd = false; this.duration = null; this.endCallbackFinisher = null; + this.finishDueToInactivity = null; this.finishing = false; this.pendingAssertions = []; this.planCount = null; @@ -221,9 +221,6 @@ class Test { run() { this.startedAt = globals.now(); - // Wait until all assertions are complete - this.timeoutHandle = globals.setTimeout(() => {}, maxTimeout); - const result = this.callFn(); if (!result.ok) { throwsHelper(result.error); @@ -262,23 +259,36 @@ class Test { this.endCallbackFinisher = () => { resolve(this.finishPromised()); }; + + this.finishDueToInactivity = () => { + this.saveFirstError(new Error('`t.end()` was never called')); + resolve(this.finishPromised()); + }; }); } if (promise) { - return promise.then( - () => this.finish(), - err => { - throwsHelper(err); - - this.saveFirstError(new assert.AssertionError({ - message: 'Rejected promise returned by test', - stack: err instanceof Error && err.stack, - values: [formatAssertError.formatWithLabel('Rejection reason:', err)] - })); - return this.finish(); - } - ); + return new Promise(resolve => { + this.finishDueToInactivity = () => { + const err = returnedObservable ? + new Error('Observable returned by test never completed') : + new Error('Promise returned by test never resolved'); + this.saveFirstError(err); + resolve(this.finishPromised()); + }; + + promise + .catch(err => { + throwsHelper(err); + + this.saveFirstError(new assert.AssertionError({ + message: 'Rejected promise returned by test', + stack: err instanceof Error && err.stack, + values: [formatAssertError.formatWithLabel('Rejection reason:', err)] + })); + }) + .then(() => resolve(this.finishPromised())); + }); } return this.finish(); @@ -314,7 +324,6 @@ class Test { completeFinish() { this.duration = globals.now() - this.startedAt; - globals.clearTimeout(this.timeoutHandle); let reason = this.assertError; let passed = !reason; diff --git a/package.json b/package.json index eb49978c2..fbc82fb91 100644 --- a/package.json +++ b/package.json @@ -142,7 +142,6 @@ "lodash.isequal": "^4.5.0", "loud-rejection": "^1.2.0", "matcher": "^0.1.1", - "max-timeout": "^1.0.0", "md5-hex": "^2.0.0", "meow": "^3.7.0", "mkdirp": "^0.5.1", diff --git a/profile.js b/profile.js index 2a65587a9..530543fcb 100644 --- a/profile.js +++ b/profile.js @@ -101,6 +101,7 @@ babelConfigHelper.build(process.cwd(), cacheDir, conf.babel, true) let uncaughtExceptionCount = 0; // Mock the behavior of a parent process + process.channel = {ref() {}, unref() {}}; process.send = data => { if (data && data.ava) { const name = data.name.replace(/^ava-/, ''); diff --git a/test/cli.js b/test/cli.js index 2dd5b30e7..fefd2e3f7 100644 --- a/test/cli.js +++ b/test/cli.js @@ -425,3 +425,24 @@ test('tests without assertions do not fail if failWithoutAssertions option is se t.end(); }); }); + +test('callback tests fail if event loop empties before they\'re ended', t => { + execCli('callback.js', {dirname: 'fixture/stalled-tests'}, (_, __, stderr) => { + t.match(stderr, /`t\.end\(\)` was never called/); + t.end(); + }); +}); + +test('observable tests fail if event loop empties before they\'re resolved', t => { + execCli('observable.js', {dirname: 'fixture/stalled-tests'}, (_, __, stderr) => { + t.match(stderr, /Observable returned by test never completed/); + t.end(); + }); +}); + +test('promise tests fail if event loop empties before they\'re resolved', t => { + execCli('promise.js', {dirname: 'fixture/stalled-tests'}, (_, __, stderr) => { + t.match(stderr, /Promise returned by test never resolved/); + t.end(); + }); +}); diff --git a/test/fixture/stalled-tests/callback.js b/test/fixture/stalled-tests/callback.js new file mode 100644 index 000000000..022a64557 --- /dev/null +++ b/test/fixture/stalled-tests/callback.js @@ -0,0 +1,5 @@ +import test from '../../..' + +test.cb(t => { + t.pass(); +}); diff --git a/test/fixture/stalled-tests/observable.js b/test/fixture/stalled-tests/observable.js new file mode 100644 index 000000000..b87d59f58 --- /dev/null +++ b/test/fixture/stalled-tests/observable.js @@ -0,0 +1,8 @@ +import test from '../../..' +import Observable from 'zen-observable' + +test(t => { + return new Observable(() => { + t.pass(); + }); +}); diff --git a/test/fixture/stalled-tests/promise.js b/test/fixture/stalled-tests/promise.js new file mode 100644 index 000000000..2f9179b1a --- /dev/null +++ b/test/fixture/stalled-tests/promise.js @@ -0,0 +1,7 @@ +import test from '../../..' + +test(t => { + return new Promise(() => { + t.pass(); + }); +}); From 5e0d220b84ee63d178818d5e48febadbc3ff0e95 Mon Sep 17 00:00:00 2001 From: Mark Wubben Date: Tue, 21 Mar 2017 14:20:06 +0000 Subject: [PATCH 13/15] Record stack for async throws assertions If the assertion fails, the AssertionError no longer has access to the stack of when the assertion was called. Record it before entering the promise chain. --- lib/assert.js | 20 ++++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/lib/assert.js b/lib/assert.js index 640e6c4d7..17291f4db 100644 --- a/lib/assert.js +++ b/lib/assert.js @@ -29,6 +29,12 @@ class AssertionError extends Error { } exports.AssertionError = AssertionError; +function getStack() { + const obj = {}; + Error.captureStackTrace(obj, getStack); + return obj.stack; +} + function wrapAssertions(callbacks) { const pass = callbacks.pass; const pending = callbacks.pending; @@ -138,7 +144,7 @@ function wrapAssertions(callbacks) { coreAssertThrowsErrorArg = err; } - const test = fn => { + const test = (fn, stack) => { let actual; let threw = false; try { @@ -160,13 +166,16 @@ function wrapAssertions(callbacks) { throw new AssertionError({ assertion: 'throws', message, + stack, values }); } }; if (promise) { - const intermediate = promise.then(makeNoop, makeRethrow).then(test); + // Record stack before it gets lost in the promise chain. + const stack = getStack(); + const intermediate = promise.then(makeNoop, makeRethrow).then(fn => test(fn, stack)); pending(this, intermediate); // Don't reject the returned promise, even if the assertion fails. return intermediate.catch(noop); @@ -196,20 +205,23 @@ function wrapAssertions(callbacks) { return; } - const test = fn => { + const test = (fn, stack) => { try { coreAssert.doesNotThrow(fn); } catch (err) { throw new AssertionError({ assertion: 'notThrows', message, + stack, values: [formatAssertError.formatWithLabel('Threw:', err.actual)] }); } }; if (promise) { - const intermediate = promise.then(noop, reason => test(makeRethrow(reason))); + // Record stack before it gets lost in the promise chain. + const stack = getStack(); + const intermediate = promise.then(noop, reason => test(makeRethrow(reason), stack)); pending(this, intermediate); // Don't reject the returned promise, even if the assertion fails. return intermediate.catch(noop); From 1fee62e7a2119abc75e55eb9c28feaa5409f2b7a Mon Sep 17 00:00:00 2001 From: Mark Wubben Date: Tue, 21 Mar 2017 15:06:38 +0000 Subject: [PATCH 14/15] Pass options object to Test constructor There are too many parameters. --- lib/test-collection.js | 30 ++++++++++++++++++------- lib/test.js | 14 ++++++------ test/observable.js | 18 +++++++++++++-- test/promise.js | 18 +++++++++++++-- test/test.js | 50 +++++++++++++++++++++++++++++++++--------- 5 files changed, 101 insertions(+), 29 deletions(-) diff --git a/lib/test-collection.js b/lib/test-collection.js index 3af9c1009..6d1d9a6ce 100644 --- a/lib/test-collection.js +++ b/lib/test-collection.js @@ -114,25 +114,39 @@ class TestCollection extends EventEmitter { return test; }); } - _buildHook(hook, testTitle, context) { + _buildHook(hook, testTitle, contextRef) { let title = hook.title; if (testTitle) { title += ` for ${testTitle}`; } - if (!context) { - context = null; + if (!contextRef) { + contextRef = null; } - return new Test(hook.metadata, title, hook.fn, false, context, this._emitTestResult); + return new Test({ + contextRef, + failWithoutAssertions: false, + fn: hook.fn, + metadata: hook.metadata, + onResult: this._emitTestResult, + title + }); } - _buildTest(test, context) { - if (!context) { - context = null; + _buildTest(test, contextRef) { + if (!contextRef) { + contextRef = null; } - return new Test(test.metadata, test.title, test.fn, this.failWithoutAssertions, context, this._emitTestResult); + return new Test({ + contextRef, + failWithoutAssertions: this.failWithoutAssertions, + fn: test.fn, + metadata: test.metadata, + onResult: this._emitTestResult, + title: test.title + }); } _buildTestWithHooks(test) { if (test.metadata.skipped || test.metadata.todo) { diff --git a/lib/test.js b/lib/test.js index bf7a1effb..d9b4e65b8 100644 --- a/lib/test.js +++ b/lib/test.js @@ -88,13 +88,13 @@ Object.defineProperty(ExecutionContext.prototype, 'context', {enumerable: true}) } class Test { - constructor(metadata, title, fn, failWithoutAssertions, contextRef, onResult) { // eslint-disable-line max-params - this.metadata = metadata; - this.title = title; - this.fn = isGeneratorFn(fn) ? co.wrap(fn) : fn; - this.failWithoutAssertions = failWithoutAssertions; - this.contextRef = contextRef; - this.onResult = onResult; + constructor(options) { + this.contextRef = options.contextRef; + this.failWithoutAssertions = options.failWithoutAssertions; + this.fn = isGeneratorFn(options.fn) ? co.wrap(options.fn) : options.fn; + this.metadata = options.metadata; + this.onResult = options.onResult; + this.title = options.title; this.assertCount = 0; this.assertError = undefined; diff --git a/test/observable.js b/test/observable.js index 65338c2de..4943de9ee 100644 --- a/test/observable.js +++ b/test/observable.js @@ -4,11 +4,25 @@ const Test = require('../lib/test'); const Observable = require('zen-observable'); // eslint-disable-line import/order function ava(fn, onResult) { - return new Test({type: 'test', callback: false}, '[anonymous]', fn, true, null, onResult); + return new Test({ + contextRef: null, + failWithoutAssertions: true, + fn, + metadata: {type: 'test', callback: false}, + onResult, + title: '[anonymous]' + }); } ava.cb = function (fn, onResult) { - return new Test({type: 'test', callback: true}, '[anonymous]', fn, true, null, onResult); + return new Test({ + contextRef: null, + failWithoutAssertions: true, + fn, + metadata: {type: 'test', callback: true}, + onResult, + title: '[anonymous]' + }); }; test('returning an observable from a legacy async fn is an error', t => { diff --git a/test/promise.js b/test/promise.js index 3a750f48d..ac96ebe05 100644 --- a/test/promise.js +++ b/test/promise.js @@ -5,11 +5,25 @@ const formatValue = require('../lib/format-assert-error').formatValue; const Test = require('../lib/test'); function ava(fn, onResult) { - return new Test({type: 'test', callback: false}, '[anonymous]', fn, true, null, onResult); + return new Test({ + contextRef: null, + failWithoutAssertions: true, + fn, + metadata: {type: 'test', callback: false}, + onResult, + title: '[anonymous]' + }); } ava.cb = function (fn, onResult) { - return new Test({type: 'test', callback: true}, '[anonymous]', fn, true, null, onResult); + return new Test({ + contextRef: null, + failWithoutAssertions: true, + fn, + metadata: {type: 'test', callback: true}, + onResult, + title: '[anonymous]' + }); }; function pass() { diff --git a/test/test.js b/test/test.js index 8920ad0af..6aab79146 100644 --- a/test/test.js +++ b/test/test.js @@ -9,19 +9,47 @@ const failingTestHint = 'Test was expected to fail, but succeeded, you should st const noop = () => {}; function ava(fn, contextRef, onResult) { - return new Test({type: 'test', callback: false}, '[anonymous]', fn, true, contextRef, onResult || noop); + return new Test({ + contextRef, + failWithoutAssertions: true, + fn, + metadata: {type: 'test', callback: false}, + onResult: onResult || noop, + title: '[anonymous]' + }); } ava.failing = (fn, contextRef, onResult) => { - return new Test({type: 'test', callback: false, failing: true}, '[anonymous]', fn, true, contextRef, onResult || noop); + return new Test({ + contextRef, + failWithoutAssertions: true, + fn, + metadata: {type: 'test', callback: false, failing: true}, + onResult: onResult || noop, + title: '[anonymous]' + }); }; ava.cb = (fn, contextRef, onResult) => { - return new Test({type: 'test', callback: true}, '[anonymous]', fn, true, contextRef, onResult || noop); + return new Test({ + contextRef, + failWithoutAssertions: true, + fn, + metadata: {type: 'test', callback: true}, + onResult: onResult || noop, + title: '[anonymous]' + }); }; ava.cb.failing = (fn, contextRef, onResult) => { - return new Test({type: 'test', callback: true, failing: true}, '[anonymous]', fn, true, contextRef, onResult || noop); + return new Test({ + contextRef, + failWithoutAssertions: true, + fn, + metadata: {type: 'test', callback: true, failing: true}, + onResult: onResult || noop, + title: '[anonymous]' + }); }; test('run test', t => { @@ -604,16 +632,18 @@ test('assertions return promises', t => { }); test('contextRef', t => { - new Test({type: 'test'}, 'foo', - a => { + new Test({ + contextRef: {context: {foo: 'bar'}}, + failWithoutAssertions: true, + fn(a) { a.pass(); t.strictDeepEqual(a.context, {foo: 'bar'}); t.end(); }, - true, - {context: {foo: 'bar'}}, - () => {} - ).run(); + metadata: {type: 'test'}, + onResult() {}, + title: 'foo' + }).run(); }); test('it is an error to set context in a hook', t => { From 950e71d1b6819193f3d35aa8015be714c3fcbe81 Mon Sep 17 00:00:00 2001 From: Mark Wubben Date: Wed, 22 Mar 2017 12:39:35 +0000 Subject: [PATCH 15/15] Fail test due to pending assertion, not a returned promise rejection If a pending assertion fails before the test implementation returns a rejected promise, the assertion failure should cause the test to fail, not the promise rejection. --- lib/test.js | 15 +++------------ test/test.js | 15 +++++++++++++++ 2 files changed, 18 insertions(+), 12 deletions(-) diff --git a/lib/test.js b/lib/test.js index d9b4e65b8..241b5f1ac 100644 --- a/lib/test.js +++ b/lib/test.js @@ -157,7 +157,7 @@ class Test { } this.assertCount++; - this.pendingAssertions.push(promise); + this.pendingAssertions.push(promise.catch(err => this.saveFirstError(err))); } addFailedAssertion(error) { @@ -299,21 +299,12 @@ class Test { this.verifyPlan(); this.verifyAssertions(); - if (this.pendingAssertions.length === 0) { - return this.completeFinish(); - } - - // Consume errors, ensuring there are no unhandled rejections. - const consumedErrors = Promise.all(this.pendingAssertions) - .catch(err => this.saveFirstError(err)); - - // Don't wait if there is an error. - if (this.assertError) { + if (this.assertError || this.pendingAssertions.length === 0) { return this.completeFinish(); } // Finish after potential errors from pending assertions have been consumed. - return consumedErrors.then(() => this.completeFinish()); + return Promise.all(this.pendingAssertions).then(() => this.completeFinish()); } finishPromised() { diff --git a/test/test.js b/test/test.js index 6aab79146..e8111d6b6 100644 --- a/test/test.js +++ b/test/test.js @@ -384,6 +384,21 @@ test('fails with the first assertError', t => { t.end(); }); +test('failing pending assertion causes test to fail, not promise rejection', t => { + let result; + return ava(a => { + return a.throws(Promise.resolve()) + .then(() => { + throw new Error('Should be ignored'); + }); + }, null, r => { + result = r; + }).run().then(passed => { + t.is(passed, false); + t.notMatch(result.reason.message, /Rejected promise returned by test/); + }); +}); + test('fails with thrown falsy value', t => { let result; const passed = ava(() => {