Improve metadata checks (#980)

lib/runner.js

@@ -4,6 +4,7 @@ const Promise = require('bluebird');
 const optionChain = require('option-chain');
 const matcher = require('matcher');
 const TestCollection = require('./test-collection');
+const validateTest = require('./validate-test');
 
 function noop() {}
 
@@ -68,18 +69,13 @@ class Runner extends EventEmitter {
 			opts.exclusive = title !== null && matcher([title], this._match).length === 1;
 		}
 
-		if (opts.todo) {
-			if (typeof fn === 'function') {
-				throw new TypeError('`todo` tests are not allowed to have an implementation. Use `test.skip()` for tests with an implementation.');
-			}
+		const validationError = validateTest(title, fn, opts);
+		if (validationError !== null) {
+			throw new TypeError(validationError);
+		}
 
+		if (opts.todo) {
 			fn = noop;
-
-			if (typeof title !== 'string') {
-				throw new TypeError('`todo` tests require a title');
-			}
-		} else if (typeof fn !== 'function') {
-			throw new TypeError('Expected an implementation. Use `test.todo()` for tests without an implementation.');
 		}
 
 		this.tests.add({

lib/validate-test.js

@@ -0,0 +1,48 @@
+'use strict';
+
+function validate(title, fn, metadata) {
+	if (metadata.type !== 'test') {
+		if (metadata.exclusive) {
+			return '`only` is only for tests and cannot be used with hooks';
+		}
+
+		if (metadata.failing) {
+			return '`failing` is only for tests and cannot be used with hooks';
+		}
+
+		if (metadata.todo) {
+			return '`todo` is only for documentation of future tests and cannot be used with hooks';
+		}
+	}
+
+	if (metadata.todo) {
+		if (typeof fn === 'function') {
+			return '`todo` tests are not allowed to have an implementation. Use ' +
+			'`test.skip()` for tests with an implementation.';
+		}
+
+		if (typeof title !== 'string') {
+			return '`todo` tests require a title';
+		}
+
+		if (metadata.skipped || metadata.failing || metadata.exclusive) {
+			return '`todo` tests are just for documentation and cannot be used with `skip`, `only`, or `failing`';
+		}
+	} else if (typeof fn !== 'function') {
+		return 'Expected an implementation. Use `test.todo()` for tests without an implementation.';
+	}
+
+	if (metadata.always) {
+		if (!(metadata.type === 'after' || metadata.type === 'afterEach')) {
+			return '`always` can only be used with `after` and `afterEach`';
+		}
+	}
+
+	if (metadata.skipped && metadata.exclusive) {
+		return '`only` tests cannot be skipped';
+	}
+
+	return null;
+}
+
+module.exports = validate;

test/runner.js

@@ -234,7 +234,7 @@ test('skip test', t => {
 
 	t.throws(() => {
 		runner.skip('should be a todo');
-	}, {message: 'Expected an implementation. Use `test.todo()` for tests without an implementation.'});
+	}, new TypeError('Expected an implementation. Use `test.todo()` for tests without an implementation.'));
 
 	runner.run({}).then(stats => {
 		t.is(stats.testCount, 2);
@@ -252,7 +252,7 @@ test('test throws when given no function', t => {
 
 	t.throws(() => {
 		runner.test();
-	}, {message: 'Expected an implementation. Use `test.todo()` for tests without an implementation.'});
+	}, new TypeError('Expected an implementation. Use `test.todo()` for tests without an implementation.'));
 });
 
 test('todo test', t => {
@@ -269,11 +269,11 @@ test('todo test', t => {
 
 	t.throws(() => {
 		runner.todo('todo', () => {});
-	}, {message: '`todo` tests are not allowed to have an implementation. Use `test.skip()` for tests with an implementation.'});
+	}, new TypeError('`todo` tests are not allowed to have an implementation. Use `test.skip()` for tests with an implementation.'));
 
 	t.throws(() => {
 		runner.todo();
-	}, {message: '`todo` tests require a title'});
+	}, new TypeError('`todo` tests require a title'));
 
 	runner.run({}).then(stats => {
 		t.is(stats.testCount, 2);
@@ -306,6 +306,175 @@ test('only test', t => {
 	});
 });
 
+test('throws if you try to set a hook as exclusive', t => {
+	const runner = new Runner();
+
+	t.throws(() => {
+		runner.beforeEach.only('', noop);
+	}, new TypeError('`only` is only for tests and cannot be used with hooks'));
+
+	t.end();
+});
+
+test('throws if you try to set a before hook as always', t => {
+	const runner = new Runner();
+
+	t.throws(() => {
+		runner.before.always('', noop);
+	}, new TypeError('`always` can only be used with `after` and `afterEach`'));
+
+	t.end();
+});
+
+test('throws if you try to set a test as always', t => {
+	const runner = new Runner();
+
+	t.throws(() => {
+		runner.test.always('', noop);
+	}, new TypeError('`always` can only be used with `after` and `afterEach`'));
+
+	t.end();
+});
+
+test('throws if you give a function to todo', t => {
+	const runner = new Runner();
+
+	t.throws(() => {
+		runner.test.todo('todo with function', noop);
+	}, new TypeError('`todo` tests are not allowed to have an implementation. Use ' +
+	'`test.skip()` for tests with an implementation.'));
+
+	t.end();
+});
+
+test('throws if todo has no title', t => {
+	const runner = new Runner();
+
+	t.throws(() => {
+		runner.test.todo();
+	}, new TypeError('`todo` tests require a title'));
+
+	t.end();
+});
+
+test('throws if todo has failing, skip, or only', t => {
+	const runner = new Runner();
+
+	const errorMessage = '`todo` tests are just for documentation and cannot be' +
+		' used with `skip`, `only`, or `failing`';
+
+	t.throws(() => {
+		runner.test.failing.todo('test');
+	}, new TypeError(errorMessage));
+
+	t.throws(() => {
+		runner.test.skip.todo('test');
+	}, new TypeError(errorMessage));
+
+	t.throws(() => {
+		runner.test.only.todo('test');
+	}, new TypeError(errorMessage));
+
+	t.end();
+});
+
+test('throws if todo isn\'t a test', t => {
+	const runner = new Runner();
+
+	const errorMessage = '`todo` is only for documentation of future tests and' +
+		' cannot be used with hooks';
+
+	t.throws(() => {
+		runner.before.todo('test');
+	}, new TypeError(errorMessage));
+
+	t.throws(() => {
+		runner.beforeEach.todo('test');
+	}, new TypeError(errorMessage));
+
+	t.throws(() => {
+		runner.after.todo('test');
+	}, new TypeError(errorMessage));
+
+	t.throws(() => {
+		runner.afterEach.todo('test');
+	}, new TypeError(errorMessage));
+
+	t.end();
+});
+
+test('throws if test has skip and only', t => {
+	const runner = new Runner();
+
+	t.throws(() => {
+		runner.test.only.skip('test', noop);
+	}, new TypeError('`only` tests cannot be skipped'));
+
+	t.end();
+});
+
+test('throws if failing is used on non-tests', t => {
+	const runner = new Runner();
+
+	const errorMessage = '`failing` is only for tests and cannot be used with hooks';
+
+	t.throws(() => {
+		runner.beforeEach.failing('', noop);
+	}, new TypeError(errorMessage));
+
+	t.throws(() => {
+		runner.before.failing('', noop);
+	}, new TypeError(errorMessage));
+
+	t.throws(() => {
+		runner.afterEach.failing('', noop);
+	}, new TypeError(errorMessage));
+
+	t.throws(() => {
+		runner.after.failing('', noop);
+	}, new TypeError(errorMessage));
+
+	t.end();
+});
+
+test('throws if only is used on non-tests', t => {
+	const runner = new Runner();
+
+	const errorMessage = '`only` is only for tests and cannot be used with hooks';
+
+	t.throws(() => {
+		runner.beforeEach.only(noop);
+	}, new TypeError(errorMessage));
+
+	t.throws(() => {
+		runner.before.only(noop);
+	}, new TypeError(errorMessage));
+
+	t.throws(() => {
+		runner.afterEach.only(noop);
+	}, new TypeError(errorMessage));
+
+	t.throws(() => {
+		runner.after.only(noop);
+	}, new TypeError(errorMessage));
+
+	t.end();
+});
+
+test('validate accepts skipping failing tests', t => {
+	t.plan(2);
+
+	const runner = new Runner();
+
+	runner.test.skip.failing('skip failing', noop);
+
+	runner.run({}).then(function (stats) {
+		t.is(stats.testCount, 1);
+		t.is(stats.skipCount, 1);
+		t.end();
+	});
+});
+
 test('runOnlyExclusive option test', t => {
 	t.plan(1);