Skip to content

Commit a631a73

Browse files
committed
Clean up collecting and running tests
Test runners will now run tests as soon as (in the next tick) they're declared. The main process no longer instructs the workers to start running tests. Various IPC commands had to be changed to ensure we can still detect whether "ava" is loaded, or whether a test file contains no actual tests. Fixes #1674.
1 parent f4971a7 commit a631a73

File tree

9 files changed

+581
-756
lines changed

9 files changed

+581
-756
lines changed

api.js

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -157,7 +157,10 @@ class Api extends EventEmitter {
157157
let forked;
158158
return Bluebird.resolve(
159159
this._computeForkExecArgv().then(execArgv => {
160-
const options = Object.assign({}, apiOptions);
160+
const options = Object.assign({}, apiOptions, {
161+
// If we're looking for matches, run every single test process in exclusive-only mode
162+
runOnlyExclusive: apiOptions.match.length > 0 || runtimeOptions.runOnlyExclusive === true
163+
});
161164
if (precompilation) {
162165
options.cacheDir = precompilation.cacheDir;
163166
options.precompiled = precompilation.map;
@@ -173,12 +176,6 @@ class Api extends EventEmitter {
173176
pendingForks.add(forked);
174177
runStatus.observeFork(forked);
175178
restartTimer();
176-
177-
forked.run({
178-
// If we're looking for matches, run every single test process in exclusive-only mode
179-
runOnlyExclusive: apiOptions.match.length > 0 || runtimeOptions.runOnlyExclusive === true
180-
});
181-
182179
return forked;
183180
}).catch(err => {
184181
// Prevent new test files from running.

lib/fork.js

Lines changed: 7 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,7 @@ module.exports = (file, opts, execArgv) => {
5959
}
6060
};
6161

62+
let loadedFile = false;
6263
const testResults = [];
6364
let results;
6465

@@ -100,21 +101,20 @@ module.exports = (file, opts, execArgv) => {
100101

101102
if (results) {
102103
resolve(results);
104+
} else if (loadedFile) {
105+
reject(new AvaError(`No tests found in ${relFile}`));
103106
} else {
104107
reject(new AvaError(`Test results were not received from ${relFile}`));
105108
}
106109
});
107110

108-
ps.on('no-tests', data => {
109-
send('teardown');
110-
111-
let message = `No tests found in ${relFile}`;
111+
ps.on('loaded-file', data => {
112+
loadedFile = true;
112113

113114
if (!data.avaRequired) {
114-
message += ', make sure to import "ava" at the top of your test file';
115+
send('teardown');
116+
reject(new AvaError(`No tests found in ${relFile}, make sure to import "ava" at the top of your test file`));
115117
}
116-
117-
reject(new AvaError(message));
118118
});
119119
});
120120

@@ -152,25 +152,5 @@ module.exports = (file, opts, execArgv) => {
152152
return promise;
153153
};
154154

155-
// Send 'run' event only when fork is listening for it
156-
let isReady = false;
157-
158-
ps.on('stats', () => {
159-
isReady = true;
160-
});
161-
162-
promise.run = options => {
163-
if (isReady) {
164-
send('run', options);
165-
return promise;
166-
}
167-
168-
ps.on('stats', () => {
169-
send('run', options);
170-
});
171-
172-
return promise;
173-
};
174-
175155
return promise;
176156
};

lib/main.js

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,8 @@ const runner = new Runner({
1111
projectDir: opts.projectDir,
1212
serial: opts.serial,
1313
updateSnapshots: opts.updateSnapshots,
14-
snapshotDir: opts.snapshotDir
14+
snapshotDir: opts.snapshotDir,
15+
runOnlyExclusive: opts.runOnlyExclusive
1516
});
1617

1718
worker.setRunner(runner);

lib/runner.js

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -127,6 +127,7 @@ class Runner extends EventEmitter {
127127
this.serial = options.serial;
128128
this.updateSnapshots = options.updateSnapshots;
129129
this.snapshotDir = options.snapshotDir;
130+
this.runOnlyExclusive = options.runOnlyExclusive;
130131

131132
this.hasStarted = false;
132133
this.results = [];
@@ -208,6 +209,13 @@ class Runner extends EventEmitter {
208209
fn,
209210
title
210211
});
212+
213+
if (!this.scheduledStart) {
214+
this.scheduledStart = true;
215+
process.nextTick(() => {
216+
this.emit('start', this._run());
217+
});
218+
}
211219
}
212220

213221
addTestResult(result) {
@@ -294,12 +302,13 @@ class Runner extends EventEmitter {
294302
}
295303
}
296304

297-
run(options) {
298-
if (options.runOnlyExclusive && !this.tests.hasExclusive) {
305+
_run() {
306+
this.hasStarted = true;
307+
308+
if (this.runOnlyExclusive && !this.tests.hasExclusive) {
299309
return Promise.resolve(null);
300310
}
301311

302-
this.hasStarted = true;
303312
this.tests.on('test', result => {
304313
this.addTestResult(result);
305314
});

lib/test-worker.js

Lines changed: 21 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,6 @@
1919
const currentlyUnhandled = require('currently-unhandled')();
2020
const isObj = require('is-obj');
2121

22-
const nowAndTimers = require('./now-and-timers');
2322
const adapter = require('./process-adapter');
2423
const serializeError = require('./serialize-error');
2524
const opts = require('./worker-options').get();
@@ -58,11 +57,20 @@ exports.setRunner = newRunner => {
5857
touchedFiles.add(file);
5958
}
6059
});
61-
runner.on('test', props => {
62-
if (exiting) {
63-
return;
64-
}
60+
runner.on('start', promise => {
61+
adapter.send('stats', {
62+
testCount: runner.tests.testCount,
63+
hasExclusive: runner.tests.hasExclusive
64+
});
6565

66+
promise.then(() => {
67+
runner.saveSnapshotState();
68+
return exit();
69+
}).catch(err => {
70+
handleUncaughtException(err);
71+
});
72+
});
73+
runner.on('test', props => {
6674
const hasError = typeof props.error !== 'undefined';
6775
// Don't send anything if it's a passed hook
6876
if (!hasError && props.type !== 'test') {
@@ -190,44 +198,14 @@ try {
190198
} catch (err) {
191199
handleUncaughtException(err);
192200
} finally {
193-
if (runner) {
194-
nowAndTimers.setImmediate(() => {
195-
const hasExclusive = runner.tests.hasExclusive;
196-
const numberOfTests = runner.tests.testCount;
197-
198-
if (numberOfTests === 0) {
199-
adapter.send('no-tests', {avaRequired: true});
200-
return;
201-
}
201+
adapter.send('loaded-file', {avaRequired: Boolean(runner)});
202202

203-
adapter.send('stats', {
204-
testCount: numberOfTests,
205-
hasExclusive
206-
});
207-
208-
process.on('ava-run', options => {
209-
// Unreference the IPC channel. This stops it from keeping the event loop
210-
// busy, which means the `beforeExit` event can be used to detect when tests
211-
// stall.
212-
adapter.ipcChannel.unref();
213-
214-
runner.run(options)
215-
.then(() => {
216-
runner.saveSnapshotState();
217-
218-
return exit();
219-
})
220-
.catch(err => {
221-
handleUncaughtException(err);
222-
});
223-
});
224-
225-
process.on('ava-init-exit', () => {
226-
exit();
227-
});
228-
});
229-
} else {
230-
// If AVA was not required, show an error
231-
adapter.send('no-tests', {avaRequired: false});
203+
if (runner) {
204+
// Unreference the IPC channel if the test file required AVA. This stops it
205+
// from keeping the event loop busy, which means the `beforeExit` event can be
206+
// used to detect when tests stall.
207+
// If AVA was not required then the parent process will initiated a teardown
208+
// sequence, for which this process ought to stay active.
209+
adapter.ipcChannel.unref();
232210
}
233211
}

profile.js

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,8 @@ babelConfigHelper.build(process.cwd(), cacheDir, babelConfigHelper.validate(conf
9797
};
9898

9999
const events = new EventEmitter();
100+
events.on('loaded-file', () => {});
101+
100102
let uncaughtExceptionCount = 0;
101103

102104
// Mock the behavior of a parent process

test/fork.js

Lines changed: 3 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,6 @@ test('emits test event', t => {
3535
t.plan(1);
3636

3737
fork(fixture('generators.js'))
38-
.run({})
3938
.on('test', tt => {
4039
t.is(tt.title, 'generator function');
4140
t.end();
@@ -48,7 +47,6 @@ test('resolves promise with tests info', t => {
4847
const file = fixture('generators.js');
4948

5049
return fork(file)
51-
.run({})
5250
.then(info => {
5351
t.is(info.stats.passCount, 1);
5452
t.is(info.tests.length, 1);
@@ -64,7 +62,6 @@ test('exit after tests are finished', t => {
6462
let cleanupCompleted = false;
6563

6664
fork(fixture('slow-exit.js'))
67-
.run({})
6865
.on('exit', () => {
6966
t.true(Date.now() - start < 10000, 'test waited for a pending setTimeout');
7067
t.true(cleanupCompleted, 'cleanup did not complete');
@@ -104,7 +101,6 @@ test('rejects promise if the process is killed', t => {
104101

105102
test('fake timers do not break duration', t => {
106103
return fork(fixture('fake-timers.js'))
107-
.run({})
108104
.then(info => {
109105
const duration = info.tests[0].duration;
110106
t.true(duration < 1000, `${duration} < 1000`);
@@ -128,7 +124,6 @@ test('destructuring of `t` is allowed', t => {
128124

129125
test('babelrc is ignored', t => {
130126
return fork(fixture('babelrc/test.js'))
131-
.run({})
132127
.then(info => {
133128
t.is(info.stats.passCount, 1);
134129
t.end();
@@ -139,7 +134,6 @@ test('@std/esm support', t => {
139134
return fork(fixture('std-esm/test.js'), {
140135
require: [require.resolve('@std/esm')]
141136
})
142-
.run({})
143137
.then(info => {
144138
t.is(info.stats.passCount, 1);
145139
t.end();
@@ -151,9 +145,9 @@ test('color support is initialized correctly', t => {
151145
t.plan(1);
152146

153147
return Promise.all([
154-
fork(fixture('chalk-enabled.js'), {color: true}).run({}),
155-
fork(fixture('chalk-disabled.js'), {color: false}).run({}),
156-
fork(fixture('chalk-disabled.js'), {}).run({})
148+
fork(fixture('chalk-enabled.js'), {color: true}),
149+
fork(fixture('chalk-disabled.js'), {color: false}),
150+
fork(fixture('chalk-disabled.js'), {})
157151
]).then(infos => {
158152
for (const info of infos) {
159153
if (info.stats.failCount > 0) {

0 commit comments

Comments
 (0)