Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat: disable html reporter entirely for headless CI #1711

Closed
runspired opened this issue Feb 15, 2023 · 5 comments
Closed

feat: disable html reporter entirely for headless CI #1711

runspired opened this issue Feb 15, 2023 · 5 comments
Assignees
@Krinkle
Labels
Component: Core For module, test, hooks, and reporters. Component: HTML Reporter Type: Enhancement New idea or feature request.
Milestone
3.0 release

Comments

@runspired
Copy link

memory growth over time for the html reporter still occurs even when hidepassed is true as the hidden DOM nodes are retained. For test suites with massive quantities of tests this can have a significant effect.
@Krinkle
Copy link
Member

Krinkle commented Feb 15, 2023

@runspired Can you try removing the <div id="qunit"> element? That should effectively disable the HTML Reporter. I'm hoping in QUnit 3 to offer a configuration option like QUnit.config.reporters where you could explicitly add/replace the reporter, e.g. using HTML+TAP, or TAP only when using a CI listener/aggregator.

@runspired
Copy link
Author

I ended up using pnpm patch to rewrite a small bit to enable this. Can put up a PR with that if you're interested, more just to see than to push forward.

@Krinkle
Copy link
Member

Krinkle commented Feb 16, 2023

@runspired Are you using an additional package for this? The HTML Reporter element is populated, but not created, by QUnit. It is typically declared in your test.html file, or equivalent as generated by a higher-level test runner (like karma-qunit).

As mentioned, we'll make this easier in the next release, but I'm checking to make sure I understand what you needed to patch and whether there's a way I can document today that doesn't involve a patch.

@runspired
Copy link
Author

We're using it with testem and ember. The same html file gets re-used in both CI and browser. We could potentially find a way to juggle the launch file, but it's a bit hefty and I think the easier lift for us is going to be to either upstream the ability to disable it (as we've found this useful even in-browser for memory-leak testing) or to continue patching (it's a fairly tiny patch).

@Krinkle Krinkle added the Type: Enhancement New idea or feature request. label Oct 14, 2023
@Krinkle Krinkle added Component: HTML Reporter Component: Core For module, test, hooks, and reporters. labels Mar 3, 2024
@Krinkle
Copy link
Member

Krinkle commented Jun 18, 2024

@runspired If we introduced an option like QUnit.config.reporters.html = false;, would that work? You'd need to set it very early on, and in a way where it will only apply when Testem is running automatically and not when serving the HTML file for debugging. Would you likely want to set this by default within ember-qunit?

When using Testem standalone (see test/integration), a bridge script must be loaded from the /testem.js URL, which would only exist when the HTML is served by Testem. Do you tend to ignore this 404, or is this handled by ember-cli?

@Krinkle Krinkle self-assigned this Jun 18, 2024
@Krinkle Krinkle modified the milestones: 3.0 release, 3.x release Jun 18, 2024
Krinkle added a commit to Krinkle/qunit that referenced this issue Jun 22, 2024
@Krinkle
HTML Reporter: Convert to HtmlReporter class
3718c72 
=== What ===

Turn html.js into an HtmlReporter class where the HtmlReporter.init
static method contains (most) of the browser runner logic that we
would run even if the HtmlReporter were turned off.

This patch primarily deals with making html.js itself free of
side-effects by delaying any browser setup logic to the init()
function.

In a future patch we can take this further and move the logic to a
"browser runner" function of sorts, such that what is left in the
reporter can be made conditional on the presence of `div[id=qunit]`
and/or `QUnit.config.reporters.html`. This is tracked in
qunitjs#1118 and
qunitjs#1711.

Ref qunitjs#1486.

=== Why ===

We need html.js to be side-effect free in order to add native ESM
support, tracked at qunitjs#1551.

To support native ESM exports, we need to introduce a separate
distribution file since ESM export syntax isn't allowed in scripts.
I expect numerous projects to involve mixed ESM imports and CJS require
usage, especially when integration layers and plugins are involved.

To avoid a split-brain problem, this means we need to somehow have
both export the same one. The way I intend to do this is by detecting
if QUnit was already defined, and if so, discard our local definition
and use that one instead. This means we need to only init the browser
runner only if we're the first one (e.g. window.onerror, QUnit.begin
for fixture, QUnit.testDone for HTML reporting, etc).

To accomplish that, start by making html.js side-effect free, deferring
its init code to a (for now) unconditional call at the end of qunit.js.
Krinkle added a commit to Krinkle/qunit that referenced this issue Jun 23, 2024
@Krinkle
HTML Reporter: Convert to HtmlReporter class
fa26a80 
=== What ===

Turn html.js into an HtmlReporter class where the HtmlReporter.init
static method contains (most) of the browser runner logic that we
would run even if the HtmlReporter were turned off.

This patch primarily deals with making html.js itself free of
side-effects by delaying any browser setup logic to the init()
function.

In a future patch we can take this further and move the logic to a
"browser runner" function of sorts, such that what is left in the
reporter can be made conditional on the presence of `div[id=qunit]`
and/or `QUnit.config.reporters.html`. This is tracked in
qunitjs#1118 and
qunitjs#1711.

Ref qunitjs#1486.

=== Why ===

We need html.js to be side-effect free in order to add native ESM
support, tracked at qunitjs#1551.

To support native ESM exports, we need to introduce a separate
distribution file since ESM export syntax isn't allowed in scripts.
I expect numerous projects to involve mixed ESM imports and CJS require
usage, especially when integration layers and plugins are involved.

To avoid a split-brain problem, this means we need to somehow have
both export the same one. The way I intend to do this is by detecting
if QUnit was already defined, and if so, discard our local definition
and use that one instead. This means we need to only init the browser
runner only if we're the first one (e.g. window.onerror, QUnit.begin
for fixture, QUnit.testDone for HTML reporting, etc).

To accomplish that, start by making html.js side-effect free, deferring
its init code to a (for now) unconditional call at the end of qunit.js.
Krinkle added a commit to Krinkle/qunit that referenced this issue Jun 23, 2024
@Krinkle
HTML Reporter: Convert to HtmlReporter class
b36498b 
=== What ===

Turn html.js into an HtmlReporter class where the HtmlReporter.init
static method contains (most) of the browser runner logic that we
would run even if the HtmlReporter were turned off.

This patch primarily deals with making html.js itself free of
side-effects by delaying any browser setup logic to the init()
function.

In a future patch we can take this further and move the logic to a
"browser runner" function of sorts, such that what is left in the
reporter can be made conditional on the presence of `div[id=qunit]`
and/or `QUnit.config.reporters.html`. This is tracked in
qunitjs#1118 and
qunitjs#1711.

Ref qunitjs#1486.

=== Why ===

We need html.js to be side-effect free in order to add native ESM
support, tracked at qunitjs#1551.

To support native ESM exports, we need to introduce a separate
distribution file since ESM export syntax isn't allowed in scripts.
I expect numerous projects to involve mixed ESM imports and CJS require
usage, especially when integration layers and plugins are involved.

To avoid a split-brain problem, this means we need to somehow have
both export the same one. The way I intend to do this is by detecting
if QUnit was already defined, and if so, discard our local definition
and use that one instead. This means we need to only init the browser
runner only if we're the first one (e.g. window.onerror, QUnit.begin
for fixture, QUnit.testDone for HTML reporting, etc).

To accomplish that, start by making html.js side-effect free, deferring
its init code to a (for now) unconditional call at the end of qunit.js.
@Krinkle Krinkle mentioned this issue Jun 23, 2024
Closed
Krinkle added a commit that referenced this issue Jun 23, 2024
@Krinkle
HTML Reporter: Convert to HtmlReporter class
e1e03e6 
=== What ===

Turn html.js into an HtmlReporter class where the HtmlReporter.init
static method contains (most) of the browser runner logic that we
would run even if the HtmlReporter were turned off.

This patch primarily deals with making html.js itself free of
side-effects by delaying any browser setup logic to the init()
function.

In a future patch we can take this further and move the logic to a
"browser runner" function of sorts, such that what is left in the
reporter can be made conditional on the presence of `div[id=qunit]`
and/or `QUnit.config.reporters.html`. This is tracked in
#1118 and
#1711.

Ref #1486.

Intentional changes:

* Use of previousFailure is no longer conditional on QUnit.config.reorder
  in order to make it more stateless. The message already did not state
  that it relates to config.reorder, and stating that it previously failed
  is accurate either way.

* Calling ev.preventDefault() is no longer conditional.
  This has been supported since IE9. QUnit 2 already required IE9+,
  and QUnit 3.0 will require IE11.
  jQuery removed similar check in jQuery 2.2.0, with commit
  jquery/jquery@a873558436.

=== Why ===

We need html.js to be side-effect free in order to add native ESM
support, tracked at #1551.

To support native ESM exports, we need to introduce a separate
distribution file since ESM export syntax isn't allowed in scripts.
I expect numerous projects to involve mixed ESM imports and CJS require
usage, especially when integration layers and plugins are involved.

To avoid a split-brain problem, this means we need to somehow have
both export the same one. The way I intend to do this is by detecting
if QUnit was already defined, and if so, discard our local definition
and use that one instead. This means we need to only init the browser
runner only if we're the first one (e.g. window.onerror, QUnit.begin
for fixture, QUnit.testDone for HTML reporting, etc).

To accomplish that, start by making html.js side-effect free, deferring
its init code to a (for now) unconditional call at the end of qunit.js.
Krinkle added a commit to Krinkle/qunit that referenced this issue Jul 3, 2024
@Krinkle
HTML Reporter: Disable if no id=qunit element, remove legacy markup s…
470a046 
…upport

* Expose `options.element` as injectable constructor option.

* If not given, and if no element exists by runStart event, then
  don't listen to any events. This should make headless execution
  significantly faster.

  Ref qunitjs#1711.

* As proof of concept, convert one of the HTML Reporter tests to use
  this injection approach, thus proving that the class no longer relies
  on any global state, and can be tested as a constructable instance,
  without the global runner having to be dedicated to the same
  test scenario.

  In future patches, I hope to convert more of the HTML Reporter
  tests to be simply test functions in test/main/ that can be run
  all together instead of requiring separate page loads.
Krinkle added a commit to Krinkle/qunit that referenced this issue Jul 3, 2024
@Krinkle
HTML Reporter: Disable if no id=qunit element, remove legacy markup s…
4c09baf 
…upport

* Expose `options.element` as injectable constructor option.

* If not given, and if no element exists by runStart event, then
  don't listen to any events. This should make headless execution
  significantly faster.

  Ref qunitjs#1711.

* As proof of concept, convert one of the HTML Reporter tests to use
  this injection approach, thus proving that the class no longer relies
  on any global state, and can be tested as a constructable instance,
  without the global runner having to be dedicated to the same
  test scenario.

  In future patches, I hope to convert more of the HTML Reporter
  tests to be simply test functions in test/main/ that can be run
  all together instead of requiring separate page loads.
Krinkle added a commit that referenced this issue Jul 18, 2024
@Krinkle
Test: Add test for disabling HtmlReporter via element=null
d871f85 
Ref #1711.
Krinkle added a commit to Krinkle/qunit that referenced this issue Jul 21, 2024
@Krinkle
[WIP] Core: Support HTML/TAP preconfig via QUnit.config.reporters
de30bc3 
Allow disabling of HTML Reporter even if `<div id="qunit">` exists.

Ref qunitjs#1711.
@Krinkle Krinkle modified the milestones: 3.x release, 3.0 release Jul 21, 2024
Krinkle added a commit to Krinkle/qunit that referenced this issue Jul 22, 2024
@Krinkle
[WIP] Core: Support HTML/TAP preconfig via QUnit.config.reporters
8870890 
Allow disabling of HTML Reporter even if `<div id="qunit">` exists.

Ref qunitjs#1711.
Krinkle added a commit to Krinkle/qunit that referenced this issue Jul 22, 2024
@Krinkle
[WIP] Core: Support HTML/TAP preconfig via QUnit.config.reporters
045074d 
Allow disabling of HTML Reporter even if `<div id="qunit">` exists.

Ref qunitjs#1711.
Krinkle added a commit to Krinkle/qunit that referenced this issue Jul 22, 2024
@Krinkle
Core: Support HTML/TAP preconfig via QUnit.config.reporters
c4ed2a2 
Allow disabling of HTML Reporter even if `<div id="qunit">` exists.

Ref qunitjs#1711.
Krinkle added a commit to Krinkle/qunit that referenced this issue Jul 22, 2024
@Krinkle
Core: Support HTML/TAP preconfig via QUnit.config.reporters
2265922 
Allow disabling of HTML Reporter even if `<div id="qunit">` exists.

Ref qunitjs#1711.
Krinkle added a commit to Krinkle/qunit that referenced this issue Jul 22, 2024
@Krinkle
Core: Support HTML/TAP preconfig via QUnit.config.reporters
05e15ba 
Allow disabling of HTML Reporter even if `<div id="qunit">` exists.

Ref qunitjs#1711.
@Krinkle Krinkle closed this as completed Aug 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@Krinkle Krinkle

Labels
Component: Core For module, test, hooks, and reporters. Component: HTML Reporter Type: Enhancement New idea or feature request.
Milestone
3.0 release
Development

No branches or pull requests
2 participants
@Krinkle @runspired