Skip to content

Commit

Permalink
Core: Always define globalThis.QUnit
Browse files Browse the repository at this point in the history
== Background

In QUnit 2.x, we always set the QUnit global in browser contexts, but
outside browsers, we only set the global if CJS/AMD wasn't detected.
Then, in the QUnit CLI, we set the global anyway, and we expect other
test runners to do the same, since that's how most QUnit tests are
written, including for Node.js targets.

In practice, the QUnit global would only be missing in custom test
runners that 1) target Node.js, 2) require/import qunit.js directly,
and 3) don't export it as a global.

I'm aware many communities import 'qunit' directly in each test file
for improved type support. That's great, and avoids needing to rely
on globals. But, that's not a requirement I want to impose on everyone,
especially for simple no-build-step and browser-facing projects.

== Why

* Improve portability between test runners.
  Remove the last edge case where the QUnit global can be undefined.
  Make it QUnit's responsiblity to define this reliably, as indeed it
  almost always already does. Remove this as undocumented requirement
  for specific test runners to patch up on their end.

  In light of Karma deprecation, and emergence of more general purpose
  TAP runners, I'd like to remove this as factor that might make/break
  QUnit support in one of those.
  • Loading branch information
Krinkle committed Jul 30, 2024
1 parent 5812597 commit 3814241
Show file tree
Hide file tree
Showing 6 changed files with 46 additions and 39 deletions.
5 changes: 3 additions & 2 deletions .eslintrc.json
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,7 @@
"sourceType": "script"
},
"env": {
"es2020": true,
"node": true
},
"rules": {
Expand All @@ -109,11 +110,11 @@
{
"files": ["test/cli/**"],
"env": {
"es2017": true,
"es2020": true,
"node": true
},
"parserOptions": {
"ecmaVersion": 2018
"ecmaVersion": 2020
},
"rules": {
"compat/compat": "off"
Expand Down
21 changes: 10 additions & 11 deletions build/tasks/test-on-node.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,25 +16,24 @@ module.exports = function (grunt) {
grunt.log.ok(`Ran ${totals.files} files`);
}

// Refresh the QUnit global to be used in other tests
global.QUnit = requireFresh('../../qunit/qunit.js');

done(!totals.failed);
});

function requireFresh (path) {
let resolvedPath = require.resolve(path);
function requireFreshQUnit () {
// Resolve current QUnit path and remove it from the require cache
// to avoid stacking the QUnit logs.
const path = '../../qunit/qunit.js';
const resolvedPath = require.resolve(path);
delete require.cache[resolvedPath];

// Clear any QUnit global from a previous test
delete globalThis.QUnit;

return require(path);
}

async function runQUnit (file) {
// Resolve current QUnit path and remove it from the require cache
// to avoid stacking the QUnit logs.
let QUnit = requireFresh('../../qunit/qunit.js');

// Expose QUnit to the global scope to be seen on the other tests.
global.QUnit = QUnit;
const QUnit = requireFreshQUnit();

QUnit.config.autostart = false;
await import('../../' + file);
Expand Down
12 changes: 4 additions & 8 deletions src/cli/run.js
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,8 @@ async function run (args, options) {

const files = utils.getFilesFromArgs(args);

QUnit = requireQUnit();
// Replace any previous instance, e.g. in watch mode
QUnit = globalThis.QUnit = requireQUnit();

if (options.filter) {
QUnit.config.filter = options.filter;
Expand All @@ -65,10 +66,6 @@ async function run (args, options) {
console.log(`Running tests with seed: ${QUnit.config.seed}`);
}

// TODO: Enable mode where QUnit is not auto-injected, but other setup is
// still done automatically.
global.QUnit = QUnit;

options.requires.forEach(requireFromCWD);

findReporter(options.reporter, QUnit.reporters).init(QUnit);
Expand Down Expand Up @@ -177,7 +174,7 @@ function abort (callback) {
process.off('exit', onExit);
running = false;

delete global.QUnit;
delete globalThis.QUnit;
QUnit = null;
if (callback) {
callback();
Expand All @@ -192,8 +189,7 @@ run.watch = function watch (args, options) {
const watch = require('node-watch');
const baseDir = process.cwd();

QUnit = requireQUnit();
global.QUnit = QUnit;
requireQUnit();
options.requires.forEach(requireFromCWD);

// Include TypeScript when in use (automatically via require.extensions),
Expand Down
39 changes: 25 additions & 14 deletions src/core/export.js
Original file line number Diff line number Diff line change
@@ -1,18 +1,29 @@
/* global module, exports */
import { window, document, globalThis } from './globals.js';

/**
* Available exports:
*
* globalThis:
* - browser (globalThis === window)
* - Web Worker (globalThis === self)
* - Node.js
* - SpiderMonkey (mozjs)
* - Rhino 7.14+
* - any other embedded JS engine
*
* CommonJS module.exports (commonjs2):
* - Node.js
*
* CommonJS exports (commonjs, https://wiki.commonjs.org/wiki/Modules):
* - Rhino
*/
export default function exportQUnit (QUnit) {
let exportedModule = false;

if (window && document) {
// QUnit may be defined when it is preconfigured but then only QUnit and QUnit.config may be defined.
if (window.QUnit && window.QUnit.version) {
if (globalThis.QUnit && globalThis.QUnit.version) {
throw new Error('QUnit has already been defined.');
}

window.QUnit = QUnit;

exportedModule = true;
}

// For Node.js
Expand All @@ -21,20 +32,20 @@ export default function exportQUnit (QUnit) {

// For consistency with CommonJS environments' exports
module.exports.QUnit = QUnit;

exportedModule = true;
}

// For CommonJS with exports, but without module.exports, like Rhino
if (typeof exports !== 'undefined' && exports) {
exports.QUnit = QUnit;

exportedModule = true;
}

// For other environments, including Web Workers (globalThis === self),
// SpiderMonkey (mozjs), and other embedded JavaScript engines
if (!exportedModule) {
// Ensure the global is available in all environments.
//
// For backward compatibility, we only enforce load-once in browsers above.
// In other environments QUnit is accessible via import/require() and may
// load multiple times. Callers may decide whether their secondary instance
// should be global or not.
if (!globalThis.QUnit || !globalThis.QUnit.version) {
globalThis.QUnit = QUnit;
}
}
2 changes: 1 addition & 1 deletion test/benchmark/micro.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/* eslint-env node */

global.QUnit = require('qunit');
require('qunit');
require('./micro-fixture.js');
require('./micro-bench.js');
6 changes: 3 additions & 3 deletions test/cli/fixtures/inception.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
const outerQUnit = global.QUnit;
delete global.QUnit;
const outerQUnit = globalThis.QUnit;
delete globalThis.QUnit;
const myQUnit = require('../../../src/cli/require-qunit')();
global.QUnit = outerQUnit;
globalThis.QUnit = outerQUnit;

const data = [];
myQUnit.on('runStart', function () {
Expand Down

0 comments on commit 3814241

Please sign in to comment.