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

Core: Convert "No tests were run." from fake test to error #1790

Merged
merged 1 commit into from
Jul 27, 2024

Conversation

Krinkle
Copy link
Member

@Krinkle Krinkle commented Jul 27, 2024

  • Remove hacky re-entrance from ProcessingQueue.done() -> test() + advance() -> done(), existed only for this purpose.

    This also removes the need for the 99aee51 workaround, which avoided a crash by infinite loop.

  • Remove unused internal test injection to ProcessingQueue, existed only for this purpose.

  • Remove "omit stack trace" logic in test.js, existed only for this purpose. To keep output for the "No tests" error similarly clean and distraction-free, the TAP reporter treats error stack traces with a similar cleaner since Core: Exclude or grey internal frames from stack traces in TAP reporter #1789.

  • Remove unused internal validTest mechanism existed only for this purpose.

    This was originally impossible to trigger externally because it required setting validTest to a private symbol. In QUnit 1.16 this was simplified as part of commit 3f08a1a, to take any boolean true value to ease some implementation details, however it remained internal in purpose. A search today for /validTest:/ and /validTest = / over public GitHub-hosted repositories, shows that (fortunatley) nobody has started relying on this. I found only copies of QUnit itself.

As a nice side-effect, fixtures like async-module-error.tap.txt now no longer display a useless "No tests" after an uncaught error that already bailed the test run. This happened previously because errors are not tests, and so technically there was no test. Now that "No tests" is itself considered a bail out, TAP absorbs this after the first error, just like it already does for other cascading errors.

* Remove hacky re-entrance from
  ProcessingQueue.done() -> test() + advance() -> done(),
  existed only for this purpose.

  This also removes the need for the 99aee51 workaround, which
  avoided a crash by infinite loop.

* Remove unused internal `test` injection to ProcessingQueue,
  existed only for this purpose.

* Remove "omit stack trace" logic in test.js,
  existed only for this purpose. To keep output for the "No tests"
  error similarly clean and distraction-free, the TAP reporter
  treats error stack traces with a similar cleaner since
  qunitjs#1789.

* Remove unused internal `validTest` mechanism
  existed only for this purpose.

  This was originally impossible to trigger externally because it
  required setting `validTest` to a private symbol. In QUnit 1.16
  this was simplified as part of commit 3f08a1a, to take any
  boolean true value to ease some implementation details, however it
  remained internal in purpose. A search today for `/validTest:/`
  and `/validTest = /` over public GitHub-hosted repositories, shows
  that (fortunatley) nobody has started relying on this. I found only
  copies of QUnit itself.

As a nice side-effect, fixtures like async-module-error.tap.txt
now no longer display a useless "No tests" after an uncaught error
that already bailed the test run. This happened previously because
errors are not tests, and so technically there was no test. No that
"No tests" is itself considered a bail out, TAP absorbs this after
the first error, just like it alreayd does for other cascading
errors.
@Krinkle Krinkle merged commit 62ca15a into qunitjs:main Jul 27, 2024
10 checks passed
@Krinkle Krinkle deleted the no-tests-error branch July 27, 2024 22:23
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.

1 participant