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

Separate HTML runner and HTML reporter? #1118

Closed
platinumazure opened this issue Mar 20, 2017 · 3 comments
Closed

Separate HTML runner and HTML reporter? #1118

platinumazure opened this issue Mar 20, 2017 · 3 comments
Assignees
@Krinkle
Labels
Component: HTML Reporter Type: Enhancement New idea or feature request.
Milestone
3.0 release

Comments

@platinumazure
Copy link
Contributor

(Ditching the issue template since this is a meta-issue)

Now that we're working on a QUnit Node CLI and integrating with js-reporters, I think we need to see if we can refactor the HTML Reporter space to split that into HTML "runner" and HTML "reporter".

Conceptually, here is what I would like to see long-term:

  • HTML runner focuses on running QUnit in a browser, decoupling itself from reporting result. It takes on the outermost container of current HTML reporter but the internals should be black box. HTML runner also handles configuration (including url-based config) and test/module filtering.
  • HTML reporter simply outputs the inner divs it currently does for every test.
  • Create a new reporter which simply takes plaintext from another reporter and puts it in an HTML block. Voila, HTML runner now integrates with any plaintext reporter.
  • Create a strategy for allowing plaintext reporters to gain an HTML/"enhanced" version for better integration? Not sure of the best way to handle that to be honest.
@gibson042
Copy link
Member

I like this, and note also the (loosely) related #947.

@leobalter
Copy link
Member

Me too. If we split both we might be able adapt into a HTML js-reporter as well.

giphy

@Krinkle Krinkle assigned Krinkle and unassigned Krinkle Apr 3, 2017
@Krinkle Krinkle closed this as completed Aug 24, 2020
@Krinkle Krinkle reopened this Oct 2, 2020
@Krinkle Krinkle added the Type: Enhancement New idea or feature request. label Oct 2, 2020
@Krinkle
Copy link
Member

Krinkle commented Oct 2, 2020

I'm looking into this as part of #1486, and running into an obstacle with testId. We currrently get this from Core and use it in the HTML reporter as part of the HTML element IDs, which are intended to be part of the public API (ref #1751) so that QUnit plugins can use them to display additional visualisations.

For QUnit's own use, we could trivially generate a different set of IDs (e.g. lazily, from the testStart event).

For plugins, we may need to change it so that the plugin reacts to the HTML rather than influencing it from the inside. For example, the way a plugin finds the HTML element currently is via (contextual) assert.test.testId.

Straw-man proposal:

  • HTML Reporter: Generate new test IDs here.
  • HTML Reporter: Emit custom events (not part of js-reporters/CRI) for the purpose of augment the display of a completed test and a completed assertion. Analogous to QUnit.testDone (CRI testEnd) and QUnit.log (has no CRI event), from which we'd provide references to the DOM elements in question.

Challenges this brings about:

  • Can we actually deprecate the core testId property? It seems we'd need to keep this at least privately for the URL parameter and re-running tests. However, "rerun" is an intimate part of the user interface. It's not like the checkboxes and module dropdown that are neatly separated in the would-be "HTML Runner" component.
  • How would our HTML Reporter operate based on CRI events whilst still being able to do QUnit-specific things like re-running tests. Maybe it's fine to have inline conditionals that would be QUnit-specific and if it were to be its own project at some point, support for other custom features of individual testing frameworks could be added. Maybe that's not so bad, and still a good enough improvement over the status quo.

Perhaps it's not worth separating the two, but we could still add support for disabling the bulk of the output and running a plain text CRI reporter in its place. In any event, that could of course be a good first step either way.

@Krinkle Krinkle mentioned this issue Oct 2, 2020
Closed
@Krinkle Krinkle mentioned this issue Jun 24, 2021
Closed
@Krinkle Krinkle mentioned this issue Mar 28, 2022
Open
@Krinkle Krinkle mentioned this issue May 29, 2023
Closed
Krinkle added a commit to Krinkle/qunit that referenced this issue Jul 12, 2023
@Krinkle
Docs: Write a page for QUnit.config.modules
1ed93c4 
Used by HTML Reporter, and by plugins such as `steal-qunit`.

Ref qunitjs#1118
Krinkle added a commit that referenced this issue Jul 12, 2023
@Krinkle
Docs: Write a page for QUnit.config.modules
8444f81 
Used by HTML Reporter, and by plugins such as `steal-qunit`.

Ref #1118
Krinkle added a commit that referenced this issue Sep 24, 2023
@Krinkle
HTML Reporter: Move internal urlparams import out of html.js
8e5f59b 
As prep to extract a reporter-agnostic "HTML Runner" from the
HTML Reporter, move the `urlparams.js` import out of html.js.

It is important (and covered by `test/reporter-html/hidepassed.html`)
that the `QUnit.begin()` callback in urlparams.js continues to run
before the one in html.js.

When only moving the import statement, this breaks because diff.js
was also importing html.js for re-use of the `escapeText` function.
As such the order in reporter.js was not what it appeared.

It appeared as:

- fixture
- diff
- html
  - urlparams

But was actually:

- fixture
- diff
  - html
    - urlparams
- html (redundant)

Ref #1118
Krinkle added a commit that referenced this issue Sep 24, 2023
@Krinkle
HTML Reporter: Move internal diff.js import to core.js
8e36011 
Ref #1118.
Krinkle added a commit that referenced this issue Sep 24, 2023
@Krinkle
HTML Reporter: Move internal diff.js import to core.js
daf3f57 
Ref #1118.
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 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 Krinkle self-assigned this Jun 23, 2024
@Krinkle Krinkle added this to the 3.0 release milestone Jun 23, 2024
Krinkle added a commit that referenced this issue Jun 23, 2024
@Krinkle
Browser: Finish extraction of browser runner from HTML Reporter
5625fca 
This refactors the initFixture and initUrlconfig code to be an exported
callable so that it is safe to define without side-effects and can then
be called conditionally.

The previous code was already conditionally with an inline check
for window/document. This is now centralised in prep for making it
further conditional on whether or not QUnit was already exported,
thus making it safe to load QUnit twice (e.g. ESM and CJS without
split-brain conflict). Tracked at
#1551.

Follows-up e1e03e6.

Closes #1118.
Krinkle added a commit that referenced this issue Jun 28, 2024
@Krinkle
Docs: New guide for Browser Runner and HTML Reporter
8646a55 
This restores the first half of the Cookbook that Jörn originally
wrote on the original qunitjs.com website. We removed this during the
redesign in 2020, but we hadn't found a new place for everything.

qunitjs/qunitjs.com#151
https://github.com/qunitjs/qunitjs.com/blob/v2.10.2/pages/cookbook.html

* Update config doc pages to reflect whether something is part of
  Browser Runner (active in any browser environment), or the
  HTML Reporter. These have been separated now per
  #1118, as QUnit.reporters.html,
  and deals purely with what happens inside the optional `div[id=qunit]`
  element.

* Move browser-related content from intro.md to the new browser.md.
  We should probably do the same for Node.js/CLI as well, and focus
  this only on env-agnostic getting started. This would be easier
  if the example use export/import ESM instead of having to assume
  either module.exports (Node.js) or globals (browser).

Co-authored-by: Jörn Zaefferer <[email protected]>

Fixes #1748.
Fixes #1747.
Krinkle added a commit that referenced this issue Jun 28, 2024
@Krinkle
Docs: New guide for Browser Runner and HTML Reporter
1b43add 
This restores the first half of the Cookbook that Jörn originally
wrote on the original qunitjs.com website. We removed this during the
redesign in 2020, but we hadn't found a new place for everything.

qunitjs/qunitjs.com#151
https://github.com/qunitjs/qunitjs.com/blob/v2.10.2/pages/cookbook.html

* Update config doc pages to reflect whether something is part of
  Browser Runner (active in any browser environment), or the
  HTML Reporter. These have been separated now per
  #1118, as QUnit.reporters.html,
  and deals purely with what happens inside the optional `div[id=qunit]`
  element.

* Move browser-related content from intro.md to the new browser.md.
  We should probably do the same for Node.js/CLI as well, and focus
  this only on env-agnostic getting started. This would be easier
  if the example use export/import ESM instead of having to assume
  either module.exports (Node.js) or globals (browser).

Co-authored-by: Jörn Zaefferer <[email protected]>

Fixes #1748.
Fixes #1747.
Krinkle added a commit that referenced this issue Jun 28, 2024
@Krinkle @jzaefferer
Docs: New guide for Browser Runner and HTML Reporter
68d30ec 
This restores the first half of the Cookbook that Jörn originally
wrote on the original qunitjs.com website. We removed this during the
redesign in 2020, but we hadn't found a new place for everything.

qunitjs/qunitjs.com#151
https://github.com/qunitjs/qunitjs.com/blob/v2.10.2/pages/cookbook.html

* Update config doc pages to reflect whether something is part of
  Browser Runner (active in any browser environment), or the
  HTML Reporter. These have been separated now per
  #1118, as QUnit.reporters.html,
  and deals purely with what happens inside the optional `div[id=qunit]`
  element.

* Move browser-related content from intro.md to the new browser.md.
  We should probably do the same for Node.js/CLI as well, and focus
  this only on env-agnostic getting started. This would be easier
  if the example use export/import ESM instead of having to assume
  either module.exports (Node.js) or globals (browser).

Fixes #1748.
Fixes #1747.

Co-authored-by: Jörn Zaefferer <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@Krinkle Krinkle

Labels
Component: HTML Reporter Type: Enhancement New idea or feature request.
Milestone
3.0 release
Development

No branches or pull requests
4 participants
@Krinkle @platinumazure @leobalter @gibson042