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.

Author: novemberborn

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 => {

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;
 

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();
 					}
 				}
 

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});
 });
 

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;

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",

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-/, '');

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();
+	});
+});

test/fixture/stalled-tests/callback.js

@@ -0,0 +1,6 @@
+'use strict';
+
+const test = require('../../..');
+test.cb(t => {
+	t.pass();
+});

test/fixture/stalled-tests/observable.js

@@ -0,0 +1,9 @@
+'use strict';
+const Observable = require('zen-observable')
+
+const test = require('../../..');
+test(t => {
+	return new Observable(() => {
+		t.pass();
+	});
+});

test/fixture/stalled-tests/promise.js

@@ -0,0 +1,8 @@
+'use strict';
+
+const test = require('../../..');
+test(t => {
+	return new Promise(() => {
+		t.pass();
+	});
+});