Skip to content

Commit

Permalink
build: Add type checking
Browse files Browse the repository at this point in the history
Workarounds for TypeScript shortcomings:

* `@typedef` can't be namespaced in JS files, thus mandating awkward
  constructs like `/** @import { Logger } from './qtap.js' */`.

* There is no way to apply a type to a `catch` clause in JS files.
  The workaround is to re-assign it to a local variable and type that
  like `catch (e) { /* @type {any} */ const err = e;`.

* There is no way to disable type checking for an entire block,
  function, or otherwise multiple lines, thus mandating lots of
  `// @ts-ignore` lines for the same problem, or to move the code
  out into a separate file where the entire file is ignored (as I
  ended up doing for client.js).

  Reported 8y ago, microsoft/TypeScript#19573.

* `@types/node` lacks Error.code or Error.stack definitinos,
  thus regularly requiring use of the above workarounds.
  Reported 3y ago, DefinitelyTyped/DefinitelyTyped#59692.

* The `noUnusedLocals` rule is broken, making a paradoxal claim.

  ```
  /** @import { Logger } from './qtap.js' */

  /**
    * @param {Logger} logger
    */
  ```
  The above makes sense, and passes, until noUnusedLocals is enabled.
  Then it fails:

  ```
  error TS6133: 'Logger' is declared but its value is never read.
  /** @import { Logger } from './qtap.js' */
  ```

  Removing the import, of course, causes a more severe error instead:
  ```
  error TS2304: Cannot find name 'Logger'.
  * @param {Logger} logger
  ```

  Reported 2y ago, closed as wontfix.
  microsoft/TypeScript#54042

* The `noUnusedParameters` errors on unused arguments before arguments
  that are required and used. It appears to be unconfigurable and thus
  serves no purpose. Leave disabled in favor of ESLint no-unused-vars.
  • Loading branch information
Krinkle committed Jan 9, 2025
1 parent db897a3 commit 7b7b508
Show file tree
Hide file tree
Showing 10 changed files with 165 additions and 92 deletions.
8 changes: 4 additions & 4 deletions bin/qtap.js
Original file line number Diff line number Diff line change
Expand Up @@ -29,11 +29,11 @@ program
.option('--timeout <number>',
'Fail if a browser is quiet for more than this many seconds.',
function (val) {
val = Number(val);
if (val < 0 || !Number.isFinite(val)) {
const num = Number(val);
if (num < 0 || !Number.isFinite(num)) {
throw new InvalidArgumentError('Not a number.');
}
return val;
return num;
},
3
)
Expand All @@ -49,7 +49,7 @@ const opts = program.opts();
if (opts.version) {
const packageFile = new URL('../package.json', import.meta.url);
const fs = await import('node:fs');
const version = JSON.parse(fs.readFileSync(packageFile)).version;
const version = JSON.parse(fs.readFileSync(packageFile).toString()).version;
console.log(version);
} else if (!program.args.length) {
program.help();
Expand Down
1 change: 1 addition & 0 deletions eslint.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ export default [
...compat.extends('eslint-config-semistandard'),
{
rules: {
'no-unused-vars': ['error', { args: 'after-used', argsIgnorePattern: '^_', vars: 'all' }],
'comma-dangle': 'off',
'multiline-ternary': 'off',
'no-throw-literal': 'off',
Expand Down
12 changes: 8 additions & 4 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,8 @@
"scripts": {
"lint": "eslint --cache .",
"lint-fix": "eslint --cache --fix .",
"test": "node bin/qtap.js -v test/pass.html && npm run -s lint"
"types": "tsc",
"test": "node bin/qtap.js -v test/pass.html && npm run -s lint && npm run -s types"
},
"dependencies": {
"@tapjs/tap-finished": "0.0.3",
Expand All @@ -30,8 +31,11 @@
"which": "5.0.0"
},
"devDependencies": {
"eslint": "^8.13.0",
"eslint-config-semistandard": "^17.0.0",
"semistandard": "^17.0.0"
"@types/node": "22.10.5",
"@types/which": "3.0.4",
"eslint": "8.13.0",
"eslint-config-semistandard": "17.0.0",
"semistandard": "17.0.0",
"typescript": "5.7.3"
}
}
16 changes: 16 additions & 0 deletions src/browsers.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import path from 'node:path';
import which from 'which';
import safari from './safari.js';
import { concatGenFn, LocalBrowser } from './util.js';
/** @import { Logger } from './qtap.js' */

const QTAP_DEBUG = process.env.QTAP_DEBUG === '1';

Expand All @@ -24,6 +25,10 @@ const WINDOWS_DIRS = new Set([
'C:\\Program Files'
].filter(Boolean));

/**
* @param {Object<string,string|boolean|number>} prefs
* @return {string}
*/
function createFirefoxPrefsJs (prefs) {
let js = '';
for (const key in prefs) {
Expand Down Expand Up @@ -105,6 +110,11 @@ function * getEdgePaths () {
}
}

/**
* @param {string} url
* @param {AbortSignal} signal
* @param {Logger} logger
*/
async function firefox (url, signal, logger) {
const profileDir = LocalBrowser.makeTempDir();
const args = [url, '-profile', profileDir, '-no-remote', '-wait-for-browser'];
Expand Down Expand Up @@ -145,6 +155,12 @@ async function firefox (url, signal, logger) {
await LocalBrowser.spawn(getFirefoxPaths(), args, signal, logger);
}

/**
* @param {Generator<string>} paths
* @param {string} url
* @param {AbortSignal} signal
* @param {Logger} logger
*/
async function chromium (paths, url, signal, logger) {
// https://github.com/GoogleChrome/chrome-launcher/blob/main/docs/chrome-flags-for-tools.md
const dataDir = LocalBrowser.makeTempDir();
Expand Down
51 changes: 51 additions & 0 deletions src/client.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
/* eslint-disable no-undef, no-var -- Browser code */
// @ts-nocheck

export function qtapClientHead () {
// Support QUnit 3.0+: Enable TAP reporter, declaratively.
window.qunit_config_reporters_tap = true;

// See ARCHITECTURE.md#qtap-internal-client-send
var qtapNativeLog = console.log;
var qtapBuffer = '';
var qtapShouldSend = true;
function qtapSend () {
var body = qtapBuffer;
qtapBuffer = '';
qtapShouldSend = false;

var xhr = new XMLHttpRequest();
xhr.onload = xhr.onerror = () => {
qtapShouldSend = true;
if (qtapBuffer) {
qtapSend();
}
};
xhr.open('POST', '{{QTAP_URL}}', true);
xhr.send(body);
}
console.log = function qtapLog (str) {
if (typeof str === 'string') {
qtapBuffer += str + '\n';
if (qtapShouldSend) {
qtapShouldSend = false;
setTimeout(qtapSend, 0);
}
}
return qtapNativeLog.apply(this, arguments);
};

// TODO: Forward console.warn, console.error, and onerror to server.
// TODO: Report window.onerror as TAP comment, visible by default.
// TODO: Report console.warn/console.error in --verbose mode.
window.addEventListener('error', function (error) {
console.log('Script error: ' + (error.message || 'Unknown error'));
});
}

export function qtapClientBody () {
// Support QUnit 2.16 - 2.22: Enable TAP reporter, procedurally.
if (typeof QUnit !== 'undefined' && QUnit.reporters && QUnit.reporters.tap && (!QUnit.config.reporters || !QUnit.config.reporters.tap)) {
QUnit.reporters.tap.init(QUnit);
}
}
49 changes: 35 additions & 14 deletions src/qtap.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,24 @@ import browsers from './browsers.js';
import { ControlServer } from './server.js';
import { globalController, globalSignal } from './util.js';

/**
* @typedef {Object} Logger
* @property {Function} channel
* @property {Function} debug
* @property {Function} warning
*/

/**
* @param {string} defaultChannel
* @param {Function} printError
* @param {?Function} [printDebug]
* @return {Logger}
*/
function makeLogger (defaultChannel, printError, printDebug = null) {
/**
* @param {Array<any>} params
* @returns {string}
*/
const paramsFmt = (params) => params
.flat()
.map(param => typeof param === 'string' ? param : util.inspect(param, { colors: false }))
Expand All @@ -30,26 +47,29 @@ function makeLogger (defaultChannel, printError, printDebug = null) {
return channel(defaultChannel);
}

/**
* @typedef {Object} qtap.RunOptions
* @property {string} [config] Path to JS file that defines additional browsers.
* @property {Object<string,Function>} [options.config.browsers] Refer to API.md for
* how to define a browser launch function.
* @property {number} [timeout=3] Fail if a browser is idle for this many seconds.
* @property {number} [connectTimeout=60] How long a browser may initially take
to launch and open the URL, in seconds.
* @property {boolean} [verbose=false]
* @property {Function} [printInfo=console.log]
* @property {Function} [printError=console.error]
* @property {string} [root=process.cwd()] Root directory to find files in
* and serve up. Ignored if testing from URLs.
/**
* @param {string[]} browserNames One or more browser names, referring either
* to a built-in browser launcher from QTap, or to a key in the optional
* `config.browsers` object.
* @param {string[]} files Files and/or URLs.
* @param {Object} [options]
* @param {string} [options.config] Path to JS file that defines additional browsers.
* @param {Object<string,Function>} [options.config.browsers] Refer to API.md for
* how to define a browser launch function.
* @param {number} [options.timeout=3] Fail if a browser is idle for this many seconds.
* @param {number} [options.connectTimeout=60] How long a browser may initially take
to launch and open the URL, in seconds.
* @param {boolean} [options.verbose=false]
* @param {Function} [options.printInfo=console.log]
* @param {Function} [options.printError=console.error]
* @param {string} [options.root=process.cwd()] Root directory to find files in
* and serve up. Ignored if testing from URLs.
* @return {number} Exit code. 0 is success, 1 is failed.
* @param {qtap.RunOptions} [options]
* @return {Promise<number>} Exit code. 0 is success, 1 is failed.
*/
async function run (browserNames, files, options) {
async function run (browserNames, files, options = {}) {
const logger = makeLogger(
'qtap_main',
options.printError || console.error,
Expand Down Expand Up @@ -146,6 +166,7 @@ async function run (browserNames, files, options) {

// TODO: Return exit status, to ease programmatic use and testing.
// TODO: Add parameter for stdout used by reporters.
return 0;
}

export default {
Expand Down
16 changes: 11 additions & 5 deletions src/safari.js
Original file line number Diff line number Diff line change
@@ -1,11 +1,13 @@
import which from 'which';
import { globalSignal, LocalBrowser } from './util.js';
/** @import { Logger } from './qtap.js' */

async function findAvailablePort () {
const net = await import('node:net');
return new Promise((resolve, reject) => {
return new Promise((resolve, _reject) => {
const srv = net.createServer();
srv.listen(0, () => {
// @ts-ignore - Not null after listen()
const port = srv.address().port;
srv.close(() => resolve(port));
});
Expand Down Expand Up @@ -41,7 +43,8 @@ let sharedSafariDriverPort = null;
*
* @param {string} url
* @param {AbortSignal} signal
* @param {qtap-Logger} logger
* @param {Logger} logger
* @returns {Promise<void>}
*/
async function safari (url, signal, logger) {
// Step 1: Start safaridriver
Expand All @@ -68,6 +71,7 @@ async function safari (url, signal, logger) {
method: method,
body: JSON.stringify(body)
});
/** @type {any} */
const data = await resp.json();
if (!resp.ok) {
throw `HTTP ${resp.status} ${data?.value?.error}, ${data?.value?.message || ''}`;
Expand All @@ -82,8 +86,10 @@ async function safari (url, signal, logger) {
session = await webdriverReq('POST', '/session/', { capabilities: { browserName: 'safari' } });
// Connected!
break;
} catch (e) {
if (e && (e.code === 'ECONNREFUSED' || (e.cause && e.cause.code === 'ECONNREFUSED'))) {
} catch (err) {
/** @type {any} - TypeScript can't set type without reassign, and @types/node lacks Error.code */
const e = err;
if (e.code === 'ECONNREFUSED' || (e.cause && e.cause.code === 'ECONNREFUSED')) {
// Wait for safaridriver to be ready, try again in another 10ms-200ms, upto ~2s in total.
logger.debug('safaridriver_waiting', `Attempt #${i}: ${e.code || e.cause.code}. Try again in ${i * 10}ms.`);
await new Promise(resolve => setTimeout(resolve, i * 10));
Expand All @@ -103,7 +109,7 @@ async function safari (url, signal, logger) {
}

// Step 4: Close the tab once we receive an 'abort' signal.
await new Promise((resolve, reject) => {
return await new Promise((resolve, reject) => {
signal.addEventListener('abort', async () => {
try {
await webdriverReq('DELETE', `/session/${session.sessionId}`);
Expand Down
Loading

0 comments on commit 7b7b508

Please sign in to comment.