Skip to content

HTML Reporter: Add support for displaying early errors#1786

Merged
Krinkle merged 1 commit intomainfrom
html-early-error
Jul 23, 2024
Merged

HTML Reporter: Add support for displaying early errors#1786
Krinkle merged 1 commit intomainfrom
html-early-error

Conversation

@Krinkle
Copy link
Copy Markdown
Member

@Krinkle Krinkle commented Jul 23, 2024

Memory for 'error' event

The status quo, in both QUnit 1.x and 2.x (2.21 as of writing) is that "early" errors are only shown in the console and not shown in the UI by the HTML Reporter. This includes uncaught errors while your subject source code is being loaded/defined, and any global errors from any test filed before the first test begins.

In QUnit 2.21 today, we initialise the HTML Reporter as part of the qunit.js execution, which means we are listening to all events right away. That sounds good, except it is kind of "too early" in a way, because the onError handler doesn't know what to do with it, as it can't render it in the UI yet, because we don't create the UI until the DOM is ready.

My initial approach to fixing this for QUnit 3, was to add a little buffer inside HtmlReporter where, the early return in onError would stash the error in a this.earlyError field. Then, in onBegin(), if we have any this.earlyError, pass it through onError a second time.

   onBegin (beginDetails) {
     this.appendInterface(beginDetails);
     this.elementDisplay.className = 'running';
+
+    if (this.earlyError) {
+      this.onError(this.earlyError);
+      this.earlyError = null;
+    }
   }
   onError (error) {
     const testItem = this.elementTests && this.appendTest('global failure');
     if (!testItem) {
       // …
+      this.earlyError = error;
       return;
     }

This approach worked when I first started working on this patch a few weeks ago. However, commit 05e15ba moved the initialising of reporters (incl. HTML Reporter) to happen later, inside QUnit.start(), just in time before emitting runStart and invoking onBegin.

That's a problem, because it means that an error event that we previously received (but didn't do anything with), is now not being received at all, because we're not starting to listen for it until after it's already happened. Even the new prioritizeSignal doesn't help us here, because that only re-orders the event handlers for future events, it doesn't bring back the past.

We could work around this by reverting the HTML Reporter to unconditionally init early, and double-checking whether it's supposed to be enabled or not inside HtmlReporter.onRunStart. But.. that feels like a hack, and doesn't solve it for the other reporters.

Instead, this pull request applies the new MEMORY_EVENTS mechanism to error. That seems worthwhile regardless, for example, I noticed in grunt-contrib-qunit recently that it has a workaround that listens both for puppeteer pagerror and QUnit.on('error'), because Puppeteer's injected JS either runs before qunit.js is defined, or waits for DOMContentLoaded. That makes sense because on the consumer side, there is no good middle-ground there. Enabling "memory" for this QUnit event, would mean such workarounds will no longer be needed, thus making QUnit events more reliable.

inArray bug

This patch fixes a mistake in events.js that's been there since commit 8f25f26. It passed arguments to inArray() in the wrong order. This code is tested and was passing due to a lucky set of mistakes cancelling each other out.

var str = 'runEnd';
var array = [ 'runEnd' ];

array.includes(str); // true (intended)

str.includes(array); // true (actual), surprise!

In addition to Array.includes, there is also String.includes. This casts any argument to a string. So when called as string.includes(array), the array is implicitly coerced to a string. The default Array.toString in JavaScript returns Array.join(', '). For a single-value array, that is equivalent to the value, because ['runEnd'].toString() === 'runEnd', and
'runEnd'.includes('runEnd') === true, naturally. However, 'runEnd'.includes('error,runEnd') is not.

@Krinkle Krinkle force-pushed the html-early-error branch 2 times, most recently from 712b5e5 to 5f6821d Compare July 23, 2024 03:11
@Krinkle Krinkle force-pushed the html-early-error branch from 5f6821d to 722acf2 Compare July 23, 2024 03:20
@Krinkle Krinkle added this to the 3.0 release milestone Jul 23, 2024
@Krinkle Krinkle added Component: HTML Reporter Type: Enhancement New idea or feature request. labels Jul 23, 2024
@Krinkle Krinkle merged commit 15acb36 into main Jul 23, 2024
@Krinkle Krinkle deleted the html-early-error branch July 23, 2024 15:07
Krinkle added a commit that referenced this pull request Jan 26, 2025
Cherry-picked from 15acb36 (3.0.0-dev).
> HTML Reporter: Add support for displaying early errors
> Ref #1786.
Krinkle added a commit that referenced this pull request Jan 26, 2025
Cherry-picked from 15acb36 (3.0.0-dev).
> HTML Reporter: Add support for displaying early errors
> Ref #1786.
Krinkle added a commit that referenced this pull request Jan 26, 2025
Cherry-picked from 15acb36 (3.0.0-dev).
> HTML Reporter: Add support for displaying early errors
> Ref #1786.
Krinkle added a commit that referenced this pull request Jan 26, 2025
Cherry-picked from 15acb36 (3.0.0-dev).
> HTML Reporter: Add support for displaying early errors
> Ref #1786.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Development

Successfully merging this pull request may close these issues.

1 participant