Skip to content

Commit 3be61a7

Browse files
committed
qtap: Improve bailout, refactor browser launch, fix Safari multi file
Major: * qtap,server: Streamline error handling and signal handling to handle various kinds of errors that previously went unhandled, bypassing teardown, or caused us to get 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 "finish" and "clientresult", and other tests are cancelled. With this in place, other uncaught errors we can handle other errors 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 for each reporter, to attribute the reporter in the error message. This also protects our EventEmitter object from a reporter tampering with "emit" or "on". The proxy only implements "on" because there is no use case for reporters calling "emit", and it is simpler this way. * tap-finished: Add "bailout" as finish signal, and fix stuckness when calling end(). * server: Separate browser launch from TAP consumption. Starting the tapParser right away inside the launch attempt meant we threw an exception for browser connect timeout, we have no way to produce a finalResult / clientresult event. This is why bailout was separate, but this is messy to deal with in reporters and also makes partial results hard to transfer. Instead, handle our bailout the same way as TAP bailout: As part of the clientresult event, not as a separate event. Upstream tap-parser emits finalResult#bailout, we can do the same in clientresult. For this to work we need to make tap-finished trigger on bailout, which it didn't until now. The benefit of this is also that there is no longer any risk of multiple browser attempts communicating over the same TAP stream and producing duplicate or conflicting results because we do not attach the TAP stream until after the browser has connected. For additional disambiguation we also mint the clientId during the attempt, instead of sharing it across retries. * browsers: Fix safari() bug when running multiple test files. * reporter: Flesh out more of the default reporter, and refine the events we emit to ease the work of reporters. * Remove separate "bail" event to simplify reporter code. Make this part of "finish" and "clientresult" events instead. * Emit all "assert" events, not just the first. Minor: * browsers: Rename Headless Firefox => Firefox Headless. * client: Set `window.qunit_config_reporters_html = false`. * client: Add basic visual `<pre>` output in debug mode. * server: Change clientId format from "client_1" to "client_S1_C1" to make verbose logs more useful. This makes it obvious which clients are for the same testFile, and thus ControlServer. * qtap: De-duplicate browsers and test files.
1 parent a44acc4 commit 3be61a7

30 files changed

+1434
-581
lines changed

API.md

Lines changed: 51 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -155,70 +155,77 @@ 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.
165+
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").
165171

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.
172+
### Event: `'error'`
173+
174+
* `error {Error|string}`
167175

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.
176+
### Event: `'finish'`
177177

178-
## QTap reporter events
178+
Summary event based on the `clientresult` events. This is mutually exclusive with `error`.
179179

180-
A client will emit each of these events only once, except `consoleerror` which may be emitted any number of times.
180+
* `event.ok {boolean}` Aggregate status of each client's results. If any failed or bailed, this is false.
181+
* `event.exitCode {number}` Suggested exit code, 0 for success, 1 for failed or bailed.
182+
* `event.total {number}` Aggregated from `clientresult` events.
183+
* `event.passed {number}` Aggregated from `clientresult` events.
184+
* `event.failed {number}` Aggregated from `clientresult` events.
181185

182-
### Event: `'client'`
186+
## QTap client events
183187

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.
188+
These are emitted once per client, except `clientconsole` and `clientassert` which may be emitted many times by a client during a test run.
185189

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").
190+
### Event: `'clientonline'`
190191

191-
### Event: `'online'`
192+
The `clientonline` event is emitted when a browser has successfully started and opened the test file. If a browser fails to start or connect, then the `error` event is emitted instead.
192193

193-
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.
194+
* `event.clientId {string}`
195+
* `event.testFile {string}`
196+
* `event.browserName {string}`
197+
* `event.displayName {string}`
194198

195-
* `event.clientId <string>`
199+
### Event: `'clientresult'`
196200

197-
### Event: `'result'`
201+
The `clientresult` event is emitted when a browser has completed a test run. This includes if it bailed mid-run, such as when a test run times out.
198202

199-
The `result` event is emitted when a browser has completed a test run. This is mutually exclusive with the `bail` event.
203+
* `event.clientId {string}`
204+
* `event.ok {boolean}`
205+
* `event.total {number}`
206+
* `event.passed {number}`
207+
* `event.failed {number}`
208+
* `event.skips {array}` Details about skipped tests (count as passed).
209+
* `event.todos {array}` Details about todo tests (count as passed).
210+
* `event.failures {array}` Details about failed tests.
211+
* `event.bailout {false|string}`
200212

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.
213+
### Event: `'clientconsole'`
209214

210-
### Event: `'bail'`
215+
The `clientconsole` event relays any warnings and uncaught errors from the browser console. These are for debug purposes only, and do not cause a test run to fail per-se. A complete and successful test run, may nonetheless print warnings or errors to the console.
211216

212-
The `bail` event is emitted when a browser was unable to start or complete a test run.
217+
Note that test frameworks such as QUnit may catch global errors during a test
213218

214-
* `event.clientId <string>`
215-
* `event.reason <string>`
219+
It is recommended that reporters only display console errors if a test run failed (i.e. there was a failed test result, or an uncaught error).
216220

217-
### Event: `'consoleerror'`
221+
* `event.clientId {string}`
222+
* `event.message {string}`
218223

219-
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.
224+
### Event: `'assert'`
220225

221-
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).
226+
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.
222227

223-
* `event.clientId <string>`
224-
* `event.message <string>`
228+
* `event.clientId {string}`
229+
* `event.ok {boolean}`
230+
* `event.fullname {string}`
231+
* `event.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

bin/qtap.js

Lines changed: 40 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
#!/usr/bin/env node
22
'use strict';
33

4+
import util from 'node:util';
5+
46
import { program, InvalidArgumentError } from 'commander';
57
import qtap from '../src/qtap.js';
68

@@ -36,8 +38,8 @@ program
3638
+ 'This is ignored when testing by URL instead of file.'
3739
)
3840
.option('--timeout <number>',
39-
'The maximum timeout of any single test.\n'
40-
+ 'The test is stopped if it takes longer than this between results.',
41+
'The maximum duration of a single unit test.\n'
42+
+ 'The test is stopped if the browser is idle longer than this between results.',
4143
function (val) {
4244
const num = Number(val);
4345
if (num < 0 || !Number.isFinite(num)) {
@@ -58,7 +60,11 @@ program
5860
},
5961
60
6062
)
61-
.option('-r, --reporter <reporter>', 'One of "minimal", "dynamic", or "none".', 'minimal')
63+
.option('-r, --reporter <reporter>',
64+
'Set one or more reporters.\n'
65+
+ 'Available: "minimal", "dynamic", "none", or a custom reporter.',
66+
'minimal'
67+
)
6268
.option('-w, --watch', 'Watch files for changes and re-run the test suite.')
6369
.option('-d, --debug', 'Enable debug mode. This keeps the browser open,\n'
6470
+ 'and for local browsers it will launch visibly instead of headless.')
@@ -90,7 +96,37 @@ if (opts.version) {
9096
});
9197
process.exit(result.exitCode);
9298
} catch (e) {
93-
console.error(e);
99+
console.log(
100+
'\n'
101+
+ util.styleText('bgRedBright',
102+
util.styleText('redBright', '__')
103+
+ util.styleText(['whiteBright', 'bold'], 'ERROR')
104+
+ util.styleText('redBright', '__')
105+
)
106+
+ '\n'
107+
);
108+
if (e instanceof qtap.QTapError && e.qtapClient) {
109+
console.error(util.styleText('grey',
110+
`Bail out from ${e.qtapClient.testFile} in ${e.qtapClient.browser}:`
111+
));
112+
}
113+
if (e instanceof qtap.QTapError) {
114+
// Omit internal stack trace for QTapError, not relevant to end-users,
115+
// by printing `e.toString()` instead of `e`.
116+
//
117+
// Omit internal "BrowserStopSignal" prefix from messages.
118+
const message = e.name === 'BrowserStopSignal' ? e.message : e.toString();
119+
// Bold first line, grey the rest
120+
// "foo" > "<b>foo</b>"
121+
// "foo \n bar \n baz" > "<b>foo</b> \n <grey>bar \n baz</grey>"
122+
const formatted = message
123+
.replace(/^.+?$/m, (m) => util.styleText('bold', m))
124+
.replace(/\n(^.+)$/ms, (m, rest) => '\n' + util.styleText('grey', rest));
125+
console.error(formatted);
126+
} else {
127+
// Print including full stack trace
128+
console.error(e);
129+
}
94130
process.exit(1);
95131
}
96132
}

eslint.config.js

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,14 +25,15 @@ export default [
2525
}
2626
},
2727
{
28-
files: ['test/*.js'],
28+
files: ['test/**/*.js'],
2929
languageOptions: {
3030
globals: {
3131
QUnit: 'readonly'
3232
}
3333
},
3434
...qunitRecommended,
3535
rules: {
36+
'no-var': 'off',
3637
'object-property-newline': 'off',
3738
}
3839
},

package.json

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,15 +26,15 @@
2626
"commander": "12.1.0",
2727
"tap-parser": "18.0.0",
2828
"which": "5.0.0",
29-
"yaml": "^2.4.1"
29+
"yaml": "^2.8.1"
3030
},
3131
"devDependencies": {
3232
"@types/node": "22.10.5",
3333
"@types/which": "3.0.4",
3434
"eslint": "~8.57.1",
3535
"eslint-config-semistandard": "~17.0.0",
3636
"eslint-plugin-qunit": "^8.1.2",
37-
"qunit": "2.24.1",
37+
"qunit": "2.24.3",
3838
"semistandard": "~17.0.0",
3939
"typescript": "5.7.3"
4040
},

src/browsers.js

Lines changed: 2 additions & 4 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 = [
@@ -215,8 +215,6 @@ const detect = async function (url, signals, logger, debugMode) {
215215
};
216216

217217
export default {
218-
LocalBrowser,
219-
220218
detect,
221219
firefox,
222220
chrome,

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;

0 commit comments

Comments
 (0)