Dont run before and after hooks when all tests are skipped (#1283)

lib/test-collection.js

@@ -178,19 +178,32 @@ class TestCollection extends EventEmitter {
 	_buildTests(tests) {
 		return tests.map(test => this._buildTestWithHooks(test));
 	}
+	_hasUnskippedTests() {
+		return this.tests.serial.concat(this.tests.concurrent)
+			.some(test => {
+				return !(test.metadata && test.metadata.skipped === true);
+			});
+	}
 	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), 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);
+		let finalTests;
+		// Only run before and after hooks when there are unskipped tests
+		if (this._hasUnskippedTests()) {
+			const beforeHooks = new Sequence(this._buildHooks(this.hooks.before));
+			const afterHooks = new Sequence(this._buildHooks(this.hooks.after));
+			finalTests = new Sequence([beforeHooks, allTests, afterHooks], true);
+		} else {
+			finalTests = new Sequence([allTests], true);
+		}
+
 		if (this.hooks.afterAlways.length > 0) {
 			const afterAlwaysHooks = new Sequence(this._buildHooks(this.hooks.afterAlways));
 			finalTests = new Sequence([finalTests, afterAlwaysHooks], false);
 		}
+
 		return finalTests;
 	}
 	attributeLeakedError(err) {

readme.md

@@ -526,10 +526,12 @@ test.failing('demonstrate some bug', t => {
 
 AVA lets you register hooks that are run before and after your tests. This allows you to run setup and/or teardown code.
 
-`test.before()` registers a hook to be run before the first test in your test file. Similarly `test.after()` registers a hook to be run after the last test. Use `test.after.always()` to register a hook that will **always** run once your tests and other hooks complete. `.always()` hooks run regardless of whether there were earlier failures, so they are ideal for cleanup tasks. There are two exceptions to this however. If you use `--fail-fast` AVA will stop testing as soon as a failure occurs, and it won't run any hooks including the `.always()` hooks. Uncaught exceptions will crash your tests, possibly preventing `.always()` hooks from running.
+`test.before()` registers a hook to be run before the first test in your test file. Similarly `test.after()` registers a hook to be run after the last test. Use `test.after.always()` to register a hook that will **always** run once your tests and other hooks complete. `.always()` hooks run regardless of whether there were earlier failures or if all tests were skipped, so they are ideal for cleanup tasks. There are two exceptions to this however. If you use `--fail-fast` AVA will stop testing as soon as a failure occurs, and it won't run any hooks including the `.always()` hooks. Uncaught exceptions will crash your tests, possibly preventing `.always()` hooks from running.
 
 `test.beforeEach()` registers a hook to be run before each test in your test file. Similarly `test.afterEach()` a hook to be run after each test. Use `test.afterEach.always()` to register an after hook that is called even if other test hooks, or the test itself, fail. `.always()` hooks are ideal for cleanup tasks.
 
+If a test is skipped with the `.skip` modifier, the respective `.beforeEach()` and `.afterEach()` hooks are not run. Likewise, if all tests in a test file are skipped `.before()` and `.after()` hooks for the file are not run. Hooks modified with `.always()` will always run, even if all tests are skipped.
+
 **Note**: If the `--fail-fast` flag is specified, AVA will stop after the first test failure and the `.always` hook will **not** run.
 
 Like `test()` these methods take an optional title and a callback function. The title is shown if your hook fails to execute. The callback is called with an [execution object](#t).

test/api.js

@@ -714,6 +714,18 @@ function generateTests(prefix, apiCreator) {
 			});
 	});
 
+	test(`${prefix} test file with only skipped tests does not run hooks`, t => {
+		const api = apiCreator();
+
+		return api.run([path.join(__dirname, 'fixture/hooks-skipped.js')])
+			.then(result => {
+				t.is(result.tests.length, 1);
+				t.is(result.skipCount, 1);
+				t.is(result.passCount, 0);
+				t.is(result.failCount, 0);
+			});
+	});
+
 	test(`${prefix} resets state before running`, t => {
 		const api = apiCreator();
 

test/fixture/hooks-skipped.js

@@ -0,0 +1,21 @@
+import test from '../..';
+
+test.before(() => {
+	throw new Error('should not run');
+});
+
+test.after(() => {
+	throw new Error('should not run');
+});
+
+test.beforeEach(() => {
+	throw new Error('should not run');
+});
+
+test.afterEach(() => {
+	throw new Error('should not run');
+});
+
+test.skip('some skipped test', t => {
+	t.fail();
+});

test/test-collection.js

@@ -239,6 +239,122 @@ test('adding a bunch of different types', t => {
 	t.end();
 });
 
+test('skips before and after hooks when all tests are skipped', t => {
+	t.plan(5);
+
+	const collection = new TestCollection({});
+	collection.add({
+		metadata: metadata({type: 'before'}),
+		fn: a => a.fail()
+	});
+	collection.add({
+		metadata: metadata({type: 'after'}),
+		fn: a => a.fail()
+	});
+	collection.add({
+		title: 'some serial test',
+		metadata: metadata({skipped: true, serial: true}),
+		fn: a => a.fail()
+	});
+	collection.add({
+		title: 'some concurrent test',
+		metadata: metadata({skipped: true}),
+		fn: a => a.fail()
+	});
+
+	const log = [];
+	collection.on('test', result => {
+		t.is(result.result.metadata.skipped, true);
+		t.is(result.result.metadata.type, 'test');
+		log.push(result.result.title);
+	});
+
+	collection.build().run();
+
+	t.strictDeepEqual(log, [
+		'some serial test',
+		'some concurrent test'
+	]);
+
+	t.end();
+});
+
+test('runs after.always hook, even if all tests are skipped', t => {
+	t.plan(6);
+
+	const collection = new TestCollection({});
+	collection.add({
+		title: 'some serial test',
+		metadata: metadata({skipped: true, serial: true}),
+		fn: a => a.fail()
+	});
+	collection.add({
+		title: 'some concurrent test',
+		metadata: metadata({skipped: true}),
+		fn: a => a.fail()
+	});
+	collection.add({
+		title: 'after always',
+		metadata: metadata({type: 'after', always: true}),
+		fn: a => a.pass()
+	});
+
+	const log = [];
+	collection.on('test', result => {
+		if (result.result.metadata.type === 'after') {
+			t.is(result.result.metadata.skipped, false);
+		} else {
+			t.is(result.result.metadata.skipped, true);
+			t.is(result.result.metadata.type, 'test');
+		}
+		log.push(result.result.title);
+	});
+
+	collection.build().run();
+
+	t.strictDeepEqual(log, [
+		'some serial test',
+		'some concurrent test',
+		'after always'
+	]);
+
+	t.end();
+});
+
+test('skips beforeEach and afterEach hooks when test is skipped', t => {
+	t.plan(3);
+
+	const collection = new TestCollection({});
+	collection.add({
+		metadata: metadata({type: 'beforeEach'}),
+		fn: a => a.fail()
+	});
+	collection.add({
+		metadata: metadata({type: 'afterEach'}),
+		fn: a => a.fail()
+	});
+	collection.add({
+		title: 'some test',
+		metadata: metadata({skipped: true}),
+		fn: a => a.fail()
+	});
+
+	const log = [];
+	collection.on('test', result => {
+		t.is(result.result.metadata.skipped, true);
+		t.is(result.result.metadata.type, 'test');
+		log.push(result.result.title);
+	});
+
+	collection.build().run();
+
+	t.strictDeepEqual(log, [
+		'some test'
+	]);
+
+	t.end();
+});
+
 test('foo', t => {
 	const collection = new TestCollection({});
 	const log = [];