Fail tests that finish without running assertions

novemberborn · novemberborn · commit 3a4553cf2860 · 2017-03-23T15:32:04.000Z
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
diff --git 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
@@ -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
@@ -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
diff --git a/test/observable.js b/test/observable.js
diff --git a/test/promise.js b/test/promise.js
diff --git a/test/runner.js b/test/runner.js
diff --git a/test/test-collection.js b/test/test-collection.js
diff --git a/test/test.js b/test/test.js

Original file line number	Diff line number	Diff line change
`@@ -10,6 +10,7 @@ class TestCollection extends EventEmitter {`
`10`	`10`	`super();`
`11`	`11`
`12`	`12`	`this.bail = options.bail;`
	`13`	`+ this.failWithoutAssertions = options.failWithoutAssertions;`
`13`	`14`	`this.hasExclusive = false;`
`14`	`15`	`this.testCount = 0;`
`15`	`16`
`@@ -124,14 +125,14 @@ class TestCollection extends EventEmitter {`
`124`	`125`	`context = null;`
`125`	`126`	`}`
`126`	`127`
`127`		`- return new Test(hook.metadata, title, hook.fn, context, this._emitTestResult);`
	`128`	`+ return new Test(hook.metadata, title, hook.fn, false, context, this._emitTestResult);`
`128`	`129`	`}`
`129`	`130`	`_buildTest(test, context) {`
`130`	`131`	`if (!context) {`
`131`	`132`	`context = null;`
`132`	`133`	`}`
`133`	`134`
`134`		`- return new Test(test.metadata, test.title, test.fn, context, this._emitTestResult);`
	`135`	`+ return new Test(test.metadata, test.title, test.fn, this.failWithoutAssertions, context, this._emitTestResult);`
`135`	`136`	`}`
`136`	`137`	`_buildTestWithHooks(test) {`
`137`	`138`	`if (test.metadata.skipped \|\| test.metadata.todo) {`