Core: Add QUnit.reporters.perf (factor PerfReporter from suite.js)
#1714
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Krinkle
changed the title
QUnit.reporters.perf (factor PerfReporter from suit…QUnit.reporters.perf (factor PerfReporter from suit…
Krinkle
changed the title
QUnit.reporters.perf (factor PerfReporter from suit…QUnit.reporters.perf (factor PerfReporter from suite.js)
No longer dynamically enumerate reporters, which would include the newly added invisible "perf" reporter that is currently browser-only.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.configto enable tap declaratively in a way that, if you want to also have the html reporter, you have to list it explicitly.Feedback welcome here, or at #1498.