Skip to content

Commit d9bcc66

Browse files
committed
WIP
Minor: * browsers: rename Headless Firefox => Firefox Headless. * browsers: fix safari() failing to run multiple test files. * client: window.qunit_config_reporters_html = false * client: debugMode `<pre>` * server: change clientId from "client_1" to "client_S1_C1" to make verbose logs more useful by making it obvious which clients are for the same testFile (and thus ControlServer). * reporter: flesh out more of the default reporter, and refine the events we emit to ease the work of reporters. * qtap: De-duplicate browsers and test files. Major: * qtap,server: Streamline error handling and signal handling to avoid errors going unhandled, bypassing teardown, or causing the process to be stuck waiting for nothing. In particular, "Could not open <file>" is now propagated to a top-level error (e.g. run throws/rejects) rather than a test-level "bail". This means we emit "error" instead of "result" or "finish", and other tests are cancelled as well. With this in place, other uncaught errors are now handled better as well, such as errors from reporters. * qtap: handle uncaught errors from reporters, which otherwise cause emit() to throw. Provide a dedicated EventEmitter proxy to improve attribution in the error message to a given reporter. This is limited to "on" both because its simpler that way, and because it protects our EventEmitter object from reporters tampering with "emit" or "on".
1 parent a44acc4 commit d9bcc66

File tree

12 files changed

+711
-314
lines changed

12 files changed

+711
-314
lines changed

API.md

Lines changed: 52 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -155,70 +155,86 @@ async function mybrowser (url, signals) {
155155
}
156156
```
157157

158-
## QTap basic events
158+
## QTap summary events
159159

160-
### Event: `'error'`
160+
These are emitted at most once for the run overall.
161161

162-
* `error <Error|string>`
162+
### Event: `'clients'`
163163

164-
### Event: `'finish'`
164+
The `clients` event conveys which browsers are being started, and which tests will be run. It is emitted as soon as QTap has validated the parameters. Each client is a browser process that runs one test suite. For example, if you run 2 test suites in 3 different browsers, there will be 6 clients.
165165

166-
Summary event that is ideal for when you run one test suite in one browser, or if you otherwise don't need a break down of results by client.
166+
* `event.clients {Object<string,Object>}` Keyed by clientId
167+
* `clientId {string}` An identifier unique within the current qtap process (e.g. `client_123`).
168+
* `testFile {string}` Relative file path or URL (e.g. `test/index.html` or `http://localhost/test/`).
169+
* `browserName {string}` Browser name, as specified in config or CLI (e.g. `firefox`).
170+
* `displayName {string}` Browser pretty name (e.g. "Headless Firefox").
171+
172+
### Event: `'error'`
167173

168-
* `event.ok <boolean>` Aggregate status of each client's results. If any failed, this is false.
169-
* `event.exitCode <number>` Suggested exit code, 0 for success, 1 for failed.
170-
* `event.total <number>` Aggregated from `result` events.
171-
* `event.passed <number>` Aggregated from `result` events.
172-
* `event.failed <number>` Aggregated from `result` events.
173-
* `event.skips <array>` Carried from the first client that failed, or empty.
174-
* `event.todos <array>` Carried from the first client that failed, or empty.
175-
* `event.failures <array>` Carried from the first client that failed, or empty.
176-
* `event.bailout <false|string>` Carried from the first client that failed, or false.
174+
* `error {Error|string}`
177175

178-
## QTap reporter events
176+
### Event: `'finish'`
179177

180-
A client will emit each of these events only once, except `consoleerror` which may be emitted any number of times.
178+
Summary event that is ideal for when you run one test suite in one browser, or if you don't need a break down of results by client.
181179

182-
### Event: `'client'`
180+
* `event.ok {boolean}` Aggregate status of each client's results. If any failed, this is false.
181+
* `event.exitCode {number}` Suggested exit code, 0 for success, 1 for failed.
182+
* `event.total {number}` Aggregated from `result` events.
183+
* `event.passed {number}` Aggregated from `result` events.
184+
* `event.failed {number}` Aggregated from `result` events.
185+
* `event.skips {array}` Carried from the first client that failed, or empty.
186+
* `event.todos {array}` Carried from the first client that failed, or empty.
187+
* `event.failures {array}` Carried from the first client that failed, or empty.
188+
* `event.bailout {false|string}` Carried from the first client that failed, or false.
183189

184-
The `client` event is emitted when a client is created. A client is a dedicated browser instance that runs one test suite. For example, if you run 2 test suites in 3 different browsers, there will be 6 clients.
190+
## QTap client events
185191

186-
* `event.clientId <string>` An identifier unique within the current qtap process (e.g. `client_123`).
187-
* `event.testFile <string>` Relative file path or URL (e.g. `test/index.html` or `http://localhost/test/`).
188-
* `event.browserName <string>` Browser name, as specified in config or CLI (e.g. `firefox`).
189-
* `event.displayName <string>` Browser pretty name, (e.g. "Headless Firefox").
192+
These are emitted once per client, except `consoleerror` and `assert` which may be emitted many times by a client during a test run.
190193

191194
### Event: `'online'`
192195

193196
The `online` event is emitted when a browser has successfully started and opened the test file. If a browser fails to connect, a `bail` event is emitted instead.
194197

195-
* `event.clientId <string>`
198+
* `event.clientId {string}`
199+
* `event.testFile {string}`
200+
* `event.browserName {string}`
201+
* `event.displayName {string}`
196202

197203
### Event: `'result'`
198204

199205
The `result` event is emitted when a browser has completed a test run. This is mutually exclusive with the `bail` event.
200206

201-
* `event.clientId <string>`
202-
* `event.ok <boolean>`
203-
* `event.total <number>`
204-
* `event.passed <number>`
205-
* `event.failed <number>`
206-
* `event.skips <array>` Details about skipped tests (count as passed).
207-
* `event.todos <array>` Details about todo tests (count as passed).
208-
* `event.failures <array>` Details about failed tests.
207+
* `event.clientId {string}`
208+
* `event.ok {boolean}`
209+
* `event.total {number}`
210+
* `event.passed {number}`
211+
* `event.failed {number}`
212+
* `event.skips {array}` Details about skipped tests (count as passed).
213+
* `event.todos {array}` Details about todo tests (count as passed).
214+
* `event.failures {array}` Details about failed tests.
209215

210216
### Event: `'bail'`
211217

212-
The `bail` event is emitted when a browser was unable to start or complete a test run.
218+
The `bail` event is emitted when a browser was unable to start or unable to complete a test run.
213219

214-
* `event.clientId <string>`
215-
* `event.reason <string>`
220+
* `event.clientId {string}`
221+
* `event.reason {string}`
216222

217223
### Event: `'consoleerror'`
218224

219225
The `consoleerror` event relays any warning or error messages from the browser console. These are for debug purposes only, and do not indicate that a test has failed. A complete and successful test run, may nonetheless print warnings or errors to the console.
220226

221227
It is recommended that reporters only display console errors if a test run failed (i.e. there was a failed test result, or the cilent bailed).
222228

223-
* `event.clientId <string>`
224-
* `event.message <string>`
229+
* `event.clientId {string}`
230+
* `event.message {string}`
231+
232+
### Event: `'assert'`
233+
234+
The `assert` event describes a single test result (whether passing or failing). This can be used by reporters to indicate activity, display the name of a test in real-time, or to convey failures early.
235+
236+
* `event.clientId {string}`
237+
* `event.result {Object}`
238+
* `ok {boolean}`
239+
* `fullname {string}`
240+
* `diag {undefined|Object}`

ARCHITECTURE.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -436,7 +436,7 @@ Safari has long resisted the temptation to offer a reasonable command-line inter
436436
{"value":null}
437437
```
438438

439-
This addresses all previous concerns, and seems to be the best as of 2025. The only downside is that it requires a bit more code to setup (find available port, and perform various HTTP requests).
439+
This addresses all previous concerns, and seems to work best as of 2025. The only downside is that it requires more code to set up (find available port, and perform various HTTP requests).
440440

441441
- https://webkit.org/blog/6900/webdriver-support-in-safari-10/
442442
- https://developer.apple.com/documentation/webkit/macos-webdriver-commands-for-safari-12-and-later

eslint.config.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ export default [
2525
}
2626
},
2727
{
28-
files: ['test/*.js'],
28+
files: ['test/**/*.js'],
2929
languageOptions: {
3030
globals: {
3131
QUnit: 'readonly'

src/browsers.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -118,7 +118,7 @@ async function firefox (url, signals, logger, debugMode) {
118118
const profileDir = LocalBrowser.makeTempDir(signals, logger);
119119
const args = [url, '-profile', profileDir, '-no-remote', '-wait-for-browser'];
120120
if (!debugMode) {
121-
firefox.displayName = 'Headless Firefox';
121+
firefox.displayName = 'Firefox Headless';
122122
args.push('-headless');
123123
}
124124

@@ -164,7 +164,7 @@ firefox.displayName = 'Firefox';
164164
function makeChromium (displayName, getPaths) {
165165
/** @type {Browser} - https://github.com/microsoft/TypeScript/issues/22063 */
166166
const chromium = async function (url, signals, logger, debugMode) {
167-
chromium.displayName = debugMode ? displayName : `Headless ${displayName}`;
167+
chromium.displayName = debugMode ? displayName : `${displayName} Headless`;
168168
// https://github.com/GoogleChrome/chrome-launcher/blob/main/docs/chrome-flags-for-tools.md
169169
const dataDir = LocalBrowser.makeTempDir(signals, logger);
170170
const args = [

src/client.cjs

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,10 +4,12 @@
44
function qtapClientHead () {
55
// Support QUnit 2.24+: Enable TAP reporter, declaratively.
66
window.qunit_config_reporters_tap = true;
7+
window.qunit_config_reporters_html = false;
78

89
// Cache references to original methods, to avoid getting trapped by mocks (e.g. Sinon)
910
var setTimeout = window.setTimeout;
1011
var XMLHttpRequest = window.XMLHttpRequest;
12+
var createTextNode = document.createTextNode && document.createTextNode.bind && document.createTextNode.bind(document);
1113

1214
// Support IE 9: console.log.apply is undefined.
1315
// Don't bother with Function.apply.call. Skip super call instead.
@@ -67,6 +69,7 @@ function qtapClientHead () {
6769
function createBufferedWrite (url) {
6870
var buffer = '';
6971
var isSending = false;
72+
var debugElement = false;
7073
function send () {
7174
var body = buffer;
7275
buffer = '';
@@ -81,8 +84,16 @@ function qtapClientHead () {
8184
};
8285
xhr.open('POST', url, true);
8386
xhr.send(body);
87+
88+
// Optimization: Only check this once, during the first send
89+
if (debugElement === false) {
90+
debugElement = document.getElementById('__qtap_debug_element') || null;
91+
}
92+
if (debugElement) {
93+
debugElement.appendChild(createTextNode(body));
94+
}
8495
}
85-
return function write (str) {
96+
return function writeTap (str) {
8697
buffer += str + '\n';
8798
if (!isSending) {
8899
isSending = true;

src/qtap.js

Lines changed: 85 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import util from 'node:util';
88
import browsers from './browsers.js';
99
import reporters from './reporters.js';
1010
import { ControlServer } from './server.js';
11+
import { BrowserStopSignal } from './util.js';
1112

1213
/**
1314
* @typedef {Object} Logger
@@ -102,14 +103,16 @@ function makeLogger (defaultChannel, printVerbose, verbose = false) {
102103
* @return {EventEmitter}
103104
*/
104105
function run (files, browserNames = 'detect', runOptions = {}) {
105-
if (typeof files === 'string') files = [files];
106-
if (typeof browserNames === 'string') browserNames = [browserNames];
107106
if (!files || !files.length) {
108107
throw new Error('Must pass one or more test files to run');
109108
}
110109
if (!browserNames || !browserNames.length) {
111110
throw new Error('Must pass one or more browser names or omit for the default');
112111
}
112+
// Remove duplicates by using a Set
113+
// TODO: Add test to verify duplicates are ignored
114+
files = Array.from(new Set(typeof files === 'string' ? [files] : files));
115+
browserNames = Array.from(new Set(typeof browserNames === 'string' ? [browserNames] : browserNames));
113116
const options = {
114117
cwd: process.cwd(),
115118
idleTimeout: 5,
@@ -124,11 +127,38 @@ function run (files, browserNames = 'detect', runOptions = {}) {
124127
const logger = makeLogger('qtap_main', options.printVerbose, options.verbose);
125128
const eventbus = new EventEmitter();
126129
const globalController = new AbortController();
130+
const globalSignal = globalController.signal;
127131

128132
if (options.reporter) {
129133
if (options.reporter in reporters) {
130134
logger.debug('reporter_init', options.reporter);
131-
reporters[options.reporter](eventbus);
135+
136+
// Create a safe event listener object, which will:
137+
// - Catch any errors from reporter functions,
138+
// so that emit() is safe in our internal code.
139+
// - Prevent reporter functions from tampering with the internal eventbus
140+
// object (can only call "on", not "emit"; cannot break "on" for other reporters)
141+
// to protect integrity of QTap, and other reporters.
142+
// - Print detailed errors in verbose mode. Any thrown erorrs here are likely
143+
// internal to QTap and not reported in detail to users by default.
144+
// - Re-throw as util.BrowserStopSignal so that ControlServer#launchBrowser
145+
// won't retry/rerun tests due to an internal error (likely deterministic).
146+
//
147+
// TODO: Write a test observing that reporter errors bail quickly, cleanly, and without retries.
148+
const safeEventConsumer = {
149+
on (event, fn) {
150+
eventbus.on(event, function () {
151+
try {
152+
fn.apply(null, arguments);
153+
} catch (e) {
154+
logger.warning('reporter_caught', e);
155+
// @ts-ignore - TypeScript @types/node lacks `Error(,options)`
156+
throw new BrowserStopSignal(`The "${options.reporter}" reporter encountered an error in the "${event}" event handler.` + (!options.verbose ? ' Run with --verbose to output a stack trace.' : ''), { cause: e });
157+
}
158+
});
159+
}
160+
};
161+
reporters[options.reporter](safeEventConsumer);
132162
} else {
133163
logger.warning('reporter_unknown', options.reporter);
134164
}
@@ -160,7 +190,6 @@ function run (files, browserNames = 'detect', runOptions = {}) {
160190
throw new Error(`Loading ${options.config} failed: ${String(e)}`, { cause: e });
161191
}
162192
}
163-
const globalSignal = globalController.signal;
164193

165194
const browerPromises = [];
166195
for (const browserName of browserNames) {
@@ -171,12 +200,43 @@ function run (files, browserNames = 'detect', runOptions = {}) {
171200
}
172201
browserFn.getDisplayName = () => browserFn.displayName || browserName;
173202
for (const server of servers) {
203+
await server.proxyBasePromise;
174204
// Each launchBrowser() returns a Promise that settles when the browser exits.
175205
// Launch concurrently, and await afterwards.
176206
browerPromises.push(server.launchBrowser(browserFn, browserName, globalSignal));
177207
}
178208
}
179209

210+
// The 'clients' event must be emitted:
211+
// * ... early, so that reporters can indicate the browser is starting.
212+
// Therefore, we must not await browerPromises until after this.
213+
// * ... exactly once, regardless of launch retries.
214+
// * ... after any custom display name is assigned to browserFn.displayName,
215+
// which may happens dynamically in the browserFn() call before any async logic.
216+
// This is used for example by the "detect" browser (to display to the selected browser),
217+
// and by the BrowserStack plugin (to expand strings like "firefox_latest").
218+
// Therefore, server.launchBrowser and server.launchBrowserAttempt must have
219+
// no async logic before it calls browserFn, as otherwise we'd emit it too soon.
220+
// eventbus.emit('clients', {
221+
// clientId,
222+
// testFile: this.testFile,
223+
// browserName,
224+
// displayName: browserFn.getDisplayName(),
225+
// });
226+
const clients = {};
227+
for (const server of servers) {
228+
for (const browser of server.browsers.values()) {
229+
clients[browser.clientId] = {
230+
clientId: browser.clientId,
231+
testFile: server.testFile + server.testFileQueryString,
232+
browserName: browser.browserName,
233+
displayName: browser.getDisplayName(),
234+
};
235+
}
236+
}
237+
logger.debug('clients_event', clients);
238+
eventbus.emit('clients', { clients: clients });
239+
180240
const finish = {
181241
ok: true,
182242
exitCode: 0,
@@ -209,27 +269,42 @@ function run (files, browserNames = 'detect', runOptions = {}) {
209269
}
210270
});
211271

212-
// Wait for all tests and browsers to finish/stop, regardless of errors thrown,
213-
// to avoid dangling browser processes.
272+
// Upon the first of any test server of browser error (i.e. file not found),
273+
// tell other test servers to stop their browsers early.
274+
let firstError;
275+
try {
276+
await Promise.all(browerPromises);
277+
} catch (e) {
278+
firstError = e;
279+
for (const server of servers) {
280+
// @ts-ignore - TypeScript @types/node lacks `Error(,options)`
281+
server.stopBrowsers(new Error('Cancelled because another test bailed', { cause: e }));
282+
}
283+
}
284+
// Re-await for clean shutdown, including for other failed servers
214285
await Promise.allSettled(browerPromises);
215-
216-
// Re-await, this time letting the first of any errors bubble up.
217-
for (const browerPromise of browerPromises) {
218-
await browerPromise;
286+
// Let the first error bubble up
287+
if (firstError) {
288+
throw firstError;
219289
}
220290

291+
logger.debug('finish_event');
221292
eventbus.emit('finish', finish);
222293
})();
223294
runPromise
224295
.finally(() => {
296+
logger.debug('runpromise_finally');
225297
for (const server of servers) {
226298
server.close();
227299
}
228300

301+
// Avoid dangling browser processes, in case we're here because
302+
// one of the browerPromise bailed out early.
229303
logger.debug('shared_cleanup', 'Invoke global signal to clean up shared resources');
230304
globalController.abort();
231305
})
232306
.catch((error) => {
307+
logger.debug('runpromise_catch');
233308
// Node.js automatically ensures users cannot forget to listen for the 'error' event.
234309
// For this reason, runWaitFor() is a separate method, because that converts the
235310
// 'error' event into a rejected Promise. If we created that Promise as part of run()

0 commit comments

Comments
 (0)