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

Build:Tests: Fix various issues with running tests on Apple Silicon Macs #2157

Merged
merged 8 commits into from
May 10, 2023

Conversation

mgol
Copy link
Member

@mgol mgol commented Mar 30, 2023

I wasn't able to run UI tests on my new Apple Silicon Mac so I updated all dependencies and fixed all the issues I found. Tests now work fine for me and we have the latest QUnit which is also nice.

@mgol mgol added this to the 1.13.3 milestone Mar 30, 2023
@mgol mgol requested a review from fnagel March 30, 2023 11:02
@mgol mgol force-pushed the apple-silicon-updates branch 3 times, most recently from 13b52b9 to 6be875c Compare March 30, 2023 11:21
@mgol mgol self-assigned this Mar 30, 2023
mgol added 8 commits May 10, 2023 00:45
Self-closing tags are reported by newer versions of the htmllint
Grunt plugin. They also don't make sense in our HTML files
since they are not XHTML-compliant and they run in HTML mode
anyway.

Ref jquerygh-2157
Some tests were not properly destroying tooltips which made tests
start to fail with the new QUnit.

Ref jquerygh-2157
`QUnit.jsDump` was renamed to `QUnit.dump` in QUnit 2.0.

Ref jquerygh-2157
Also, workaround issues with QUnit Chrome bridge: the Chrome bridge
from `grunt-contrib-qunit` is now getting injected into every single
iframe, including an empty one that has no intention of running QUnit
tests. Since that bridge requires QUnit, it fails with an error
in such cases. Workaround the issue by wrapping the bridge in
another function that bails early if QUnit is not defined.

Ref jquerygh-2157
Also, fix htmllint lang exclusion patterns.

Ref jquerygh-2157
Changes:
* add `tests/lib/vendor/**/*` to `.eslintignore`
* move `qunit-composite` to `tests/lib` so that we can modify it
* move `qunit-assert-classes` to `tests/lib` so that we can modify it
* move `qunit-assert-close` to `tests/lib` so that we can modify it
* replace `assert.push` with `assert.pushResult`
* remove usage of `QUnit.extend`

Closes jquerygh-2157
@mgol mgol merged commit 546214e into jquery:main May 10, 2023
5 checks passed
@mgol mgol deleted the apple-silicon-updates branch May 10, 2023 08:53
mgol added a commit that referenced this pull request May 10, 2023
Self-closing tags are reported by newer versions of the htmllint
Grunt plugin. They also don't make sense in our HTML files
since they are not XHTML-compliant and they run in HTML mode
anyway.

Ref gh-2157
mgol added a commit that referenced this pull request May 10, 2023
Some tests were not properly destroying tooltips which made tests
start to fail with the new QUnit.

Ref gh-2157
mgol added a commit that referenced this pull request May 10, 2023
`QUnit.jsDump` was renamed to `QUnit.dump` in QUnit 2.0.

Ref gh-2157
mgol added a commit that referenced this pull request May 10, 2023
mgol added a commit that referenced this pull request May 10, 2023
Also, workaround issues with QUnit Chrome bridge: the Chrome bridge
from `grunt-contrib-qunit` is now getting injected into every single
iframe, including an empty one that has no intention of running QUnit
tests. Since that bridge requires QUnit, it fails with an error
in such cases. Workaround the issue by wrapping the bridge in
another function that bails early if QUnit is not defined.

Ref gh-2157
mgol added a commit that referenced this pull request May 10, 2023
Also, fix htmllint lang exclusion patterns.

Ref gh-2157
Copy link
Member

@fnagel fnagel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Uhhh, that is a big one! Nice work here!! Sooner or later we'll gotten in trouble with all those old dependencies.

For a proper review, this one is a little too big. I took a brief look at everything and I think its fine. Merged anyway :-D

And again: sorry for the delay!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

None yet

2 participants