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

[pull] main from qunitjs:main #1

Open
wants to merge 81 commits into
base: main
Choose a base branch
from
Open

[pull] main from qunitjs:main #1

wants to merge 81 commits into from

Conversation

pull[bot]
Copy link

@pull pull bot commented Aug 7, 2024

See Commits and Changes for more details.


Created by pull[bot]

Can you help keep this open source service alive? 💖 Please sponsor : )

Add a bit more detail.
@pull pull bot added the ⤵️ pull label Aug 7, 2024
Krinkle and others added 28 commits August 7, 2024 23:57
== pushFailure ==

This was setting `actual: null` and, left expected unset/undefined.
In HtmlReporter this idom was recognised to mean "don't show values",
based on hasOwn for `expected`. This worked because QUnit.log() passes
the internal assert result object as-is.

In TAP output, this was not skipped because Test.js#logAssertion
copies the object for the testEnd.errors array, and in doing so forges
an `expected` property to exist no matter what, thus with an implied
value of undefined. The hasOwn checks in TapReporter thus always
evaluate to true.

This meant TAP output for pushFailure() calls not only showed
redundant actual/expected entries, but actively created confusion
by setting them to different values, drawing attention to a supposed
difference that has no meaning

> actual: null
> expected: undefined

Fix by changing pushFailure to omit both actual and expected,
and change the condition in both reporters to skip rendering of values
based on both being strictly equal to `undefined`, instead of based
on `hasOwn('expected')`.

== onUncaughtException / `QUnit.on('error')` ==

For uncaught errors, we already omitted both actual and undefined,
the HtmlReporter thus already skipped them (for the reason above),
but in TAP output they showed, for the same reason as above as:

> actual: undefined
> expected: undefined

Which, while not as actively confusing, is at least redundant.
This is naturally fixed by the same change, which now omits this.
Follows-up #1789, which applied
this to traces under uncaught errors. We now apply this clean up to
traces under assertion failures as well.

Closes #1795.
When projects use the recommended structure for the HTML test suite,
in which qunit.js is loaded at the end of the `<body>` we now render
the UI immediately instead of waiting for all source code and test
suites to load first, which can take a while in large projects.

Closes #1793.
Follows-up d8c2a3a. This felt like a hacky compromise and doesn't
quite feel right either way. Let's leave flowing as before for now.

What I did add is to expose the version and user agent as data
attributes, to empower other themes and plugins to play with this
and perhaps come up with a better design.
Follows-up 05e15ba, which made this into a factory function,
but that has the downside of making the QUnit object not defined in
one object literal, which makes a few other things reasier to reason
about.

In a way, it's more honest to say that start is the product of a
factory function, but I'd prefer to maintain the simplicity of an
uncoupled literal declaration the entire API, in particular in prep
for native ESM export (ref #1551).

I'll accept in return the internal responsiblity to not call start()
"incorrectly" (i.e. before it is ready). This responsibility does not
leak into, complicate, break, or otherwise change the public API, and
is mitigated by a runtime detection, for the benefit of future
contributors and maintainers to QUnit.
Avoid dumping the entire QUnit object.
Extracted from #1798 to minimize
the diff.
Node.js fetch() is non-experimental as of Node.js 18.0.0.

Test Plan:
* Run `node build/review-package.js 3.0.0-alpha.2`.
* This should download _temp/package.json and offer it for diffing.
* This should then download _temp/qunit.js and offer it for diffing.
* Fix huge color contrast issue with the dark red background
  behind similarly dark colors, especially in `.counts`.

* Remove white padding between test item edge and assert list
  assertion green/red shading. Instead, add some border-spacing
  to the assert item table.

* Adjust various colors slightly to comply with WCAG guidelines.
  https://developer.mozilla.org/en-US/docs/Web/Accessibility/Understanding_WCAG/Perceivable/Color_contrast

* Fix broken `.qunit-test.pass .test-expected { color: #999; }`
  override which was only applicable to passing "todo" tests as no
  other passing tests have failing assertions under them. This was
  meant to mute the colors of these failures to avoid distraction.
  However the CSS cascade has changed over time such that these
  styles no longer applied.

  This is likely masked by the fact that passing tests (including
  passing todos) are collapsed by default, so there's very little
  attention to these anyway.

  Restore these overrides, and extend them to the "diff" and
  "source" lines as well.

Easy way to identify color usage:

```
$ git grep '#[0-9A-F][0-9A-F]*' src/core/qunit.css | sed 's/^.*\(#[0-9A-F]*\).*/\1/' | sort | uniq -c
```

Ref #1774.
Closes #1803.
Prepend "/blog/", and ensure trailing slash for consistency with
our other URLs.
Enable use of development mode in Amethyst theme and Typesearch lib.

Ref qunitjs/jekyll-theme-amethyst@2750b88222
Ref qunitjs/jekyll-theme-amethyst@1436028533
The cost of supporting IE9-10, given we support IE11 is tiny it is
only a handful of lines of code. In order to reduce the size of the
3.0 release and get it out the door, bring this back.

This also allows it to be used by the latest jQuery 3.x which supports
IE9 still.

Note that an ESM bundle was introduced in QUnit 3.0.0-alpha.4,
which is native ES6 and drops support for IE 9, IE 10 and IE 11, and
indeed all down-compilation for ES5-era browsers. You can use that
in order to get the same benefit in terms of bundle size.

Having said that, QUnit has one of the leanest bundle sizes of any
test framework even with IE9 support included.
…example

This was still an example of internal stacks as used by Node 12-14.
That's insignificant for the most part, except for the greying feature
recently added. By updating these fixtures, we'll improve test coverage.
Follows-up 5f810e5. It set the page_rank low correctly, but the
order matters. First match decides.

I based this on https://github.com/jquery/jqueryui.com/blob/b53c29bcfe80618608c94fa3aba19356c92fa5c2/docsearch.config.json, where the important one is listed first, but
I failed to realize that the order isn't sensitive there since the
content is on two separate domains. For us the order matters as one
is a subset of the other.
Caught by the daily hydra-link-checker job.

Follows-up 5f810e5.
First introduced in QUnit 2.16.0 with f8948c9.
* Rename the About menu to "Community".
* Move "Browser Support" to Guides.
* Write "Support" and "Chat" sections.
* Add "Support & Chat" link to the Community menu.
* Enable Bluesky link in theme footer.

Also sort out the mess that's become of the app.element.io client
which for unknown reasons no longer allows guest viewing, it just
throws up a login wall instead. As workaround, use the app.gitter.im
link, which hosts the same Element Web client, but does still permit
guest viewing.

The reason to keep both is that the gitter.im client is configured
to only allow login via GitHub or GitLab account. This domain is
not registered as universal link for Element native apps, and also
offers no (easy) way to create (or login with) a Matrix.org account.
This is testing regular config, not preconfig. Copy-pasta.

Follows-up 05e15ba.
== Indentation ==

When reporting a failing test, and when the actual/expected data
is an array or object, we just embedded the result of JSON.stringify
as-is without indenting each line correctly. This meant that the
property keys were oddly aligned with the outer object, and that
the closing bracket was actually outside the `---` diagnostic block.

Example:

```
not ok 1 example
  ---
  message: foo
  severitity: failed
  actual: {
  "x": 1,
  "y": 2
}
  expected: {
  "x": 1,
  "y": 3
}
  ...
```

The formatting of the property keys is purely cosmetic and the spec [1]
doesn't care about that. (Although humans do!) But, what the spec
does require is that the diagnostic YAML block ends when there
is a line that doesn't start with two spaces.

As a result, parsing QUnit CLI output with tap-parser [2] did not work
very well when you look closely. It correctly understands the overall
summary and the name and status of each test result. But, the
diagnostic data is lost (interpreted as non-TAP extra lines, equivalent
to random console.log lines).

[1]: https://testanything.org/tap-version-13-specification.html
[2]: https://github.com/tapjs/tapjs/tree/tap-parser%4018.0.0/src/parser

== Colors ==

Reporting of SKIP and TODO directives was also not skip compliant.
When the formatting of "skip" and "todo" was introduced back in 2017,
with qunitjs/js-reporters#95, it made two
mistakes:

```
not ok 1 # TODO hello
ok 2 # SKIP world
```

1. The color code started right before the "#" character,
   like `not ok 1 <cyan># TODO hello</cyan>`.

   This does not follow the spec [1], because a test is only
   considered a todo if "<space>#<space>TODO" appears after the
   (optional) test name, and this isn't the case for us.

   Because of this, todo tests were parsed by tap-parser as genuine
   test failures, thus breaking the end-user result.

2. The test name was printed after "#" instead of before.
   This meant that strictly speaking, the name of the test was the
   empty string "", and the "reason" was set to the test name.

   This is not a big deal. So long as the (optional) reason is
   preserved and presented also by the next TAP tool in chain,
   the user will still be able to see it.
…selector" (2022)

Preserving here for improved discovery and easy reference.

* The first, quickly composed of verbatim quotes from the 2016 issues/PRs.

* The second, rewritten based on two linked issue tracker comments
  by me that I wrote effectively as a blog post already, which I
  did for this exact purpose.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant