qunit icon indicating copy to clipboard operation
qunit copied to clipboard

Update HTML Reporter to use QUnit.on() instead of legacy events

Open Krinkle opened this issue 3 years ago • 2 comments

Would help identify any gaps or inconveniences which we can then feed back into the CRI standard at https://github.com/js-reporters/js-reporters.

Krinkle avatar Oct 02 '20 01:10 Krinkle

There is no CRI event for individual assertions. Given we collapse tests by default in the interface, perhaps it's okay to lose this immediacy in the HTML DOM. Batching the DOM/layout to only happen at the end of each test might actually speed up execution as well.

The reporter currently uses testDone() which exposes the testId it uses to associate parts of the DOM, but callback doesn't expose the array of assertions.

The testEnd event, on the other hand, does have the assertion details, but doesn't have QUnit's testId.

I wrote more at https://github.com/qunitjs/qunit/issues/1118#issuecomment-702488633 about untangling testId from Core, for the benefit of plugins that change the HTML output.

I believe testId is probably something we only use in the HTML runner right now. On the CLI one wouldn't know that ID to pass even if we supported it there as argument. (The QUnit.config object is supported on Node, implicitly, which one could use from a boostrap file with --require, but that seems unusual, and something we'd breaking in QUnit 3.0 in favour of something better that we'd ship in QUnit 2.x.)

Thinking out loud:

  • Perhaps QUnit.config.filter could support being a a callback function (or array of such), that QUnit gives an object similar to the testStart event data (test name, suite name, and full name). - Relates to https://github.com/qunitjs/qunit/issues/1259.
  • The HTML runner would use this to decide which tests should run, based on its query params. This could use the same hashing function we use today, or something else.

Krinkle avatar Oct 02 '20 02:10 Krinkle

The reporter currently uses testDone() which exposes the testId it uses to associate parts of the DOM, but callback doesn't expose the array of assertions.

Internally, HTML Reporter has no strong need for using the testId, that is we could just as easily have them without HTML id attributes, or use its own local auto-incrementing number instead. However, while not officially documented, the testId is used by some UI plugins in order to find the DOM node that represents the test result and then add additional visualisations to it. For example:

https://github.com/JamesMGreene/qunit-composite/blob/v2.0.0/qunit-composite.js#L198

		testId = data.testId,
		current = document.getElementById( "qunit-test-output-" + testId ),

https://github.com/Krinkle/node-colorfactory/blob/v1.0.0/test/testinit.js#L24-L28

			renderColors(colors, '#qunit-test-output-' + QUnit.config.current.testId, label);

If we want HTML Reporter to use on('testEnd') instead of testDone() and not depend on this kind of internal state, we'd have to do so in QUnit 3 so as to not unexpected break these plugins.

We should then also, in a future 2.x minor release introduce something that faccilitates this kind of plugin in another way. I'm struggling to come up with something that's compatible with having the HTML Reporter not rely on consuming internal state though.

For example, if we recommended that plugins use something like QUnit.config.current.testElement by reference instead of querying by ID on their own, that'd work and be better than what we have, and allow us to remove those HTML IDs entirely. Except, how would we assign this property. For the HTML Reporter to create this element and assign it to QUnit.config.current it would need a reference to the Test object we place there, which is even more internal than the testId concept and naturally wouldn't be available through the testEnd event either.

As far as I'm concerned, there's no urgency for making the HTML Reporter a pure reporter instead of a QUnit plugin. For one, the HTML Runner portion of it and the toolbar etc would always be a plugin that uses the tighter integration as today. The only urgency in some sense is that we'd like to avoid major versions where we can, and we have one coming up, so if any breaking changes are needed, now is the time.

To be continued :)

Krinkle avatar Apr 29 '22 00:04 Krinkle