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.
avajs · Mar 23, 2017 · df54324 · df54324
1 parent f25935e
commit df54324
Show file tree

Hide file tree

Showing 4 changed files with 74 additions and 94 deletions.
diff --git 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,19 +50,19 @@ function test(props) {
 function exit() {
 	const stats = runner._buildStats();
 
-	send('results', {stats});
+	adapter.send('results', {stats});
 }
 
 globals.setImmediate(() => {
 	const hasExclusive = runner.tests.hasExclusive;
 	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
@@ -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
@@ -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
@@ -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');
 };