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.

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

feat(html-reporter): add "Rerun all failed tests" #1626

Closed
wants to merge 1 commit into from

Conversation

buschtoens
Copy link
Contributor

@buschtoens buschtoens commented Jun 24, 2021

Closes #1474.

New Feature: "Rerun all failed tests" Link

Appends a "Rerun all failed tests" link to the progress report in the qunit-testresult-display header, that uses setUrl({ ... }) to generate a URL that includes the IDs of all failed tests as a list of testId query parameters.

Screenshots

During test execution

Screenshot 2021-09-05 at 17 43 40

After completion

Screenshot 2021-09-05 at 17 41 13

How?

To achieve this, the stats.failedTests property was changed from a number to a string[] array that keeps track of all failed test IDs.

Considered Alternatives

  • Keep stats.failedTests a number and add an extra failedTestIds: string[] array.
    I disliked this because of the double book-keeping, which could lead to stats.failedTests and failedTestIds getting out of sync.
  • Lazily query the IDs of all failed tests from the DOM on demand.
    Based on the snippet I included in my original issue [Feature] Rerun all failed tests #1474, I could have also generated the "Rerun all failed tests" link on demand, like so:
    const failedTestIds = [
      ...document.querySelectorAll('.fail[id^="qunit-test-output"]')
    ].map(n => n.id.slice('qunit-test-output-'.length));
    const href = setUrl({ testId: failedTestIds });
    However, that seemed quite hacky and would require more complex code to intercept interactions with the <a> tag to set the href attribute in time, where the current stats.failedTests solution is very straightforward.

Further Refactors

Common Utility Functions

Furthermore the registered hooks for QUnit.done(...) and QUnit.testStart(...) were refactored to use common utility functions:

getProgressHtml(runtime: number, stats: Stats, total: number): string
  • When called from Qunit.testStart(...), the count of completed tests is lower than total and the return value looks like this:

    209 / 978 tests completed in 398101 milliseconds, with 2 failed, 6 skipped, and 0 todo.

  • When called from Qunit.done(...), the count of completed tests is equal to total and the return value looks like this:

    978 tests completed in 703624 milliseconds, with 2 failed, 6 skipped, and 0 todo.

getAssertionsSummaryHtml(total: number, passed: number, failed: number): string

Called from Qunit.done(...) to print the summary of assertions. Example:

<span class="passed">116</span> assertions of
<span class="total">116</span> passed,
<span class="failed">0</span> failed.
getRerunFailedHtml(failedTests: string[]): string

Called from both Qunit.done(...) and Qunit.testStart(...) to generate a link to rerun all (so far) failed tests. Example:

<a href="http://localhost:1337/tests?testId=883cd3ae&testId=b2f8d939">
  Rerun all 2 failed tests
</a>

Returns an empty string (""), if failedTests is empty ([]).

<br> Insertion

I also refactored how <br> tags are inserted between these fragments. Instead of including the <br> in the return value of e.g. getProgressHtml(...) and in the fragment arrays, they are inserted by .filter(Boolean).join("<br">), when the fargments are concatenated into the final HTML string, which has the added advantage of conveniently filtering out optional fragments first.

Recommended Refactors

Avoid innerHTML

I would have also liked to change the inner workings to construct real DOM directly instead of using element.innerHTML = "...", but I did not want to change more than necessary, as the above already are quite significant refactors.

Closes qunitjs#1474.

Appends a "Rerun all failed tests" link to the progress report in the
`qunit-testresult-display` header, that uses `setUrl({ ... })` to generate a
URL that includes the IDs of all failed tests as a list of `testId` query
parameters.

To achieve this, the `stats.failedTests` property was changed from a `number` to
a `string[]` array that keeps track of all failed test IDs.

Considered alternatives:

- Keep `stats.failedTests` a `number` and add an extra `failedTestIds: string[]`
  array. I disliked this because of the double book-keeping, which could lead to
  `stats.failedTests` and `failedTestIds` getting out of sync.
- Lazily query the IDs of all failed tests from the DOM on demand. Based on the
  snippet I included in my original issue qunitjs#1474, I could have also generated the
  "Rerun all failed tests" link on demand, like so:
  ```js
  const failedTestIds = [
    ...document.querySelectorAll('.fail[id^="qunit-test-output"]')
  ].map(n => n.id.slice('qunit-test-output-'.length));
  const href = setUrl({ testId: failedTestIds });
  ```
  However, that seemed quite hacky and would require more complex code to
  intercept interactions with the `<a>` tag to set the `href` attribute in time,
  where the current `stats.failedTests` solution is very straightforward.

Furthermore the registered hooks for `QUnit.done(...)` and
`QUnit.testStart(...)` were refactored to use common utility functions:

- `getProgressHtml(runtime: number, stats: Stats, total: number): string`
  - When called from `Qunit.testStart(...)`, the count of completed tests is
    lower than `total` and the return value looks like this:
    `209 / 978 tests completed in 398101 milliseconds, with 2 failed, 6 skipped, and 0 todo.`
  - When called from `Qunit.done(...)`, the count of completed tests is equal to
    `total` and the return value looks like this:
    `978 tests completed in 703624 milliseconds, with 2 failed, 6 skipped, and 0 todo.`
- `getAssertionsSummaryHtml(total: number, passed: number, failed: number): string`
  Called from `Qunit.done(...)` to print the summary of assertions. Example:
  ```html
  <span class="passed">116</span> assertions of
  <span class="total">116</span> passed,
  <span class="failed">0</span> failed.
  ```
- `getRerunFailedHtml(failedTests: string[]): string`
  Called from both `Qunit.done(...)` and `Qunit.testStart(...)` to generate a
  link to rerun all (so far) failed tests. Example:
  ```html
  <a href="http://localhost:1337/tests?testId=883cd3ae&testId=b2f8d939">
    Rerun all 2 failed tests
  </a>
  ```
  Returns an empty string (`""`), if `failedTests` is empty (`[]`).

I also refactored how `<br>` tags are inserted between these fragments. Instead
of including the `<br>` in the return value of e.g. `getProgressHtml(...)` and
in the fragment arrays, they are inserted by `.filter(Boolean).join("<br">)`,
when the fargments are concatenated into the final HTML string, which has the
added advantage of conveniently filtering out optional fragments first.

I would have also liked to change the inner workings to construct real DOM
directly instead of using `element.innerHTML = "..."`, but I did not want to
change more than necessary, as the above already are quite significant
refactors.
@buschtoens
Copy link
Contributor Author

buschtoens commented Jun 24, 2021

I wasn't really sure where to add tests for this. I think test/reporter-html/reporter-html.js is the best candidate, but it doesn't really have broad test coverage to begin with. If you think, it's necessary, I can try adding some test cases for this change in there.

@Krinkle
Copy link
Member

Krinkle commented Jun 24, 2021

@buschtoens Thanks, this looks great. I agree with your decision tree and overall approach.

I think a test would be useful here, but perhaps better as a standalone HTML file to make it less subject to change. The test suite would e.g. have 2 fake failing tests with a known test ID, and a third test asserting that the link is generated as expected. It then needs the "usual" trickery with done() (see events.js) to replace the (expected) failures with a success.

In the hopefully not-too-distant future this'll be easier to test when the HTML Reporter is more separated (#1118), at which point you'd instantiate it within a test, give it a new element to add its UI to, feed it some fixed events, and see what it does to the element (without affecting the "main" runner etc.).

@Krinkle Krinkle self-assigned this Jun 24, 2021
@Krinkle
Copy link
Member

Krinkle commented Sep 5, 2021

Merged in f5333b2.

@Krinkle Krinkle closed this Sep 5, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

[Feature] Rerun all failed tests
2 participants