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

HTML Reporter: Fix display: none regression on global error message #1652

Merged
merged 1 commit into from
Sep 9, 2021

Conversation

Krinkle
Copy link
Member

@Krinkle Krinkle commented Sep 8, 2021

Follows-up 8f5e7ed, which fixed the bug where in the "No tests" error (or indeed any global error after tests are done running) could cause the test runner to get stuck.

That change maintained backward compatibility by continuing to count these global errors as "failed tests", so that CI reporters listening for QUnit.done() on QUnit.on("runEnd") continue to get the same negative signal. And, it continued to visually render them as a failed test if they happen during test execution or as the last/only event.

But... it inserted the element into the DOM without making visible, which fooled our internal unit tests.

Fix #1651.

Follows-up 8f5e7ed, which fixed the bug where in the "No tests" error
(or indeed any global error after tests are done running) could cause
the test runner to get stuck.

That change maintained backward compatibility by continuing to count
these global errors as "failed tests", so that CI reporters listening
for `QUnit.done()` on `QUnit.on("runEnd")` continue to get the same
negative signal. And, it continued to visually render them as a failed
test if they happen during test execution or as the last/only event.

But... it inserted the element into the DOM without making visible,
which fooled our internal unit tests.

Fix qunitjs#1651.
@Krinkle Krinkle merged commit 4853df6 into qunitjs:main Sep 9, 2021
@Krinkle Krinkle deleted the onerror-invisible branch September 9, 2021 00:16
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.

2.17.0 no longer throws an error in the browser runner when a filter does not match any tests
1 participant