Skip to content

Commit

Permalink
Core: Introduce QUnit.reporters.perf (factor PerfReporter from suit…
Browse files Browse the repository at this point in the history
…e.js)

This is in preparation for QUnit 3 where we'd like to make it
possible to enable this on Node.js, not just in browsers.

For now it remains unconditionally enabled, and limited to browsers
by virtue of using `window.performance`. The reason we don't enable
it unconditionally is due to its overhead and that it should remain
opt-in for QUnit CLI.

I'm still gathering feedback, but it's possible we'll make it opt-in
for browsers as well, depending how significant the overhead is and
how often people use this.

Another way might be to leave it on by default when using the HTML
reporter, but that when someone configures the list of reporters
explicitly (e.g. to enable "TAP" in the browser for a CI pipeline)
to then automatically turn off any default reporters ("html" and "perf").

In other words, the default is html+perf, and we could offer a way
in `QUnit.config` to enable tap declaratively in a way that, if you
want to also have the html reporter, you have to list it explicitly.
  • Loading branch information
Krinkle committed Apr 27, 2023
1 parent 52aabab commit b6671e8
Show file tree
Hide file tree
Showing 7 changed files with 102 additions and 64 deletions.
37 changes: 2 additions & 35 deletions src/core/utilities.js
Original file line number Diff line number Diff line change
@@ -1,45 +1,12 @@
import { window } from '../globals';
import Logger from '../logger';

export const toString = Object.prototype.toString;
export const hasOwn = Object.prototype.hasOwnProperty;
export const slice = Array.prototype.slice;

const nativePerf = getNativePerf();

// TODO: Consider using globalThis instead so that perf marks work
// in Node.js as well. As they can have overhead, we should also
// have a way to disable these, and/or make them an opt-in reporter
// in QUnit 3 and then support globalThis.
// For example: `QUnit.addReporter(QUnit.reporters.perf)`.
function getNativePerf () {
if (window &&
typeof window.performance !== 'undefined' &&
typeof window.performance.mark === 'function' &&
typeof window.performance.measure === 'function'
) {
return window.performance;
} else {
return undefined;
}
}

export const performance = {
now: nativePerf
? nativePerf.now.bind(nativePerf)
: Date.now,
measure: nativePerf
? function (comment, startMark, endMark) {
// `performance.measure` may fail if the mark could not be found.
// reasons a specific mark could not be found include: outside code invoking `performance.clearMarks()`
try {
nativePerf.measure(comment, startMark, endMark);
} catch (ex) {
Logger.warn('performance.measure could not be executed because of ', ex.message);
}
}
: function () {},
mark: nativePerf ? nativePerf.mark.bind(nativePerf) : function () {}
// eslint-disable-next-line compat/compat -- Checked
now: window && window.performance && window.performance.now ? window.performance.now.bind(window.performance) : Date.now
};

// Returns a new Array with the elements that are in a but not in b
Expand Down
2 changes: 2 additions & 0 deletions src/html-reporter/html.js
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,8 @@ export function escapeText (str) {
return;
}

QUnit.reporters.perf.init(QUnit);

const config = QUnit.config;
const hiddenTests = [];
let collapseNext = false;
Expand Down
6 changes: 4 additions & 2 deletions src/reporters.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
import ConsoleReporter from './reporters/ConsoleReporter.js';
import TapReporter from './reporters/TapReporter.js';
import PerfReporter from './reporters/PerfReporter.js';

export default {
console: ConsoleReporter,
tap: TapReporter
tap: TapReporter,
perf: PerfReporter,
console: ConsoleReporter
};
92 changes: 92 additions & 0 deletions src/reporters/PerfReporter.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,92 @@
import { window } from '../globals';
import Logger from '../logger';

// TODO: Consider using globalThis instead of window, so that the reporter
// works for Node.js as well. As this can add overhead, we should make
// this opt-in before we enable it for CLI.
//
// QUnit 3 will switch from `window` to `globalThis` and then make it
// no longer an implicit feature of the HTML Reporter, but rather let
// it be opt-in via `QUnit.config.reporters = ['perf']` or something
// like that.
const nativePerf = (
window &&
typeof window.performance !== 'undefined' &&
// eslint-disable-next-line compat/compat -- Checked
typeof window.performance.mark === 'function' &&
// eslint-disable-next-line compat/compat -- Checked
typeof window.performance.measure === 'function'
)
? window.performance
: undefined;

const perf = {
measure: nativePerf
? function (comment, startMark, endMark) {
// `performance.measure` may fail if the mark could not be found.
// reasons a specific mark could not be found include: outside code invoking `performance.clearMarks()`
try {
nativePerf.measure(comment, startMark, endMark);
} catch (ex) {
Logger.warn('performance.measure could not be executed because of ', ex.message);
}
}
: function () {},
mark: nativePerf ? nativePerf.mark.bind(nativePerf) : function () {}
};

export default class PerfReporter {
constructor (runner, options = {}) {
this.perf = options.perf || perf;

runner.on('runStart', this.onRunStart.bind(this));
runner.on('runEnd', this.onRunEnd.bind(this));
runner.on('suiteStart', this.onSuiteStart.bind(this));
runner.on('suiteEnd', this.onSuiteEnd.bind(this));
runner.on('testStart', this.onTestStart.bind(this));
runner.on('testEnd', this.onTestEnd.bind(this));
}

static init (runner, options) {
return new PerfReporter(runner, options);
}

onRunStart () {
this.perf.mark('qunit_suite_0_start');
}

onSuiteStart (suiteStart) {
const suiteLevel = suiteStart.fullName.length;
this.perf.mark(`qunit_suite_${suiteLevel}_start`);
}

onSuiteEnd (suiteEnd) {
const suiteLevel = suiteEnd.fullName.length;
const suiteName = suiteEnd.fullName.join(' – ');

this.perf.mark(`qunit_suite_${suiteLevel}_end`);
this.perf.measure(`QUnit Test Suite: ${suiteName}`,
`qunit_suite_${suiteLevel}_start`,
`qunit_suite_${suiteLevel}_end`
);
}

onTestStart () {
this.perf.mark('qunit_test_start');
}

onTestEnd (testEnd) {
this.perf.mark('qunit_test_end');
const testName = testEnd.fullName.join(' – ');

this.perf.measure(`QUnit Test: ${testName}`,
'qunit_test_start',
'qunit_test_end'
);
}

onRunEnd () {
this.perf.mark('qunit_suite_0_end');
this.perf.measure('QUnit Test Run', 'qunit_suite_0_start', 'qunit_suite_0_end');
}
}
13 changes: 0 additions & 13 deletions src/reports/suite.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,6 @@ export default class SuiteReport {
start (recordTime) {
if (recordTime) {
this._startTime = performance.now();

const suiteLevel = this.fullName.length;
performance.mark(`qunit_suite_${suiteLevel}_start`);
}

return {
Expand All @@ -40,16 +37,6 @@ export default class SuiteReport {
end (recordTime) {
if (recordTime) {
this._endTime = performance.now();

const suiteLevel = this.fullName.length;
const suiteName = this.fullName.join(' – ');

performance.mark(`qunit_suite_${suiteLevel}_end`);
performance.measure(
suiteLevel === 0 ? 'QUnit Test Run' : `QUnit Test Suite: ${suiteName}`,
`qunit_suite_${suiteLevel}_start`,
`qunit_suite_${suiteLevel}_end`
);
}

return {
Expand Down
12 changes: 0 additions & 12 deletions src/reports/test.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ export default class TestReport {
start (recordTime) {
if (recordTime) {
this._startTime = performance.now();
performance.mark('qunit_test_start');
}

return {
Expand All @@ -35,17 +34,6 @@ export default class TestReport {
end (recordTime) {
if (recordTime) {
this._endTime = performance.now();
if (performance) {
performance.mark('qunit_test_end');

const testName = this.fullName.join(' – ');

performance.measure(
`QUnit Test: ${testName}`,
'qunit_test_start',
'qunit_test_end'
);
}
}

return extend(this.start(), {
Expand Down
4 changes: 2 additions & 2 deletions test/cli/fixtures/expected/tap-outputs.js
Original file line number Diff line number Diff line change
Expand Up @@ -172,14 +172,14 @@ ok 5 A-Test > derp
'qunit --reporter does-not-exist':
`# stderr
No reporter found matching "does-not-exist".
Built-in reporters: console, tap
Built-in reporters: console, perf, tap
Extra reporters found among package dependencies: npm-reporter
# exit code: 1`,

'qunit --reporter':
`# stderr
Built-in reporters: console, tap
Built-in reporters: console, perf, tap
Extra reporters found among package dependencies: npm-reporter
# exit code: 1`,
Expand Down

0 comments on commit b6671e8

Please sign in to comment.