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: Make Karma work in ES modules mode #4550

Merged
merged 4 commits into from
Dec 16, 2019
Merged

Conversation

mgol
Copy link
Member

@mgol mgol commented Nov 27, 2019

Summary

Make Karma work in ES modules mode

Also, run such a suite in CI to make sure modules are working as expected
when used directly.

We could consider doing a version of this PR for AMD for 3.x-stable, I don't see any fundamental roadblocks.

Checklist

@mgol mgol added this to the 4.0.0 milestone Nov 27, 2019
@mgol mgol requested a review from timmywil November 27, 2019 15:34
@mgol mgol self-assigned this Nov 27, 2019
@mgol mgol force-pushed the karma-esmodules branch 2 times, most recently from ef9152f to 5a8e212 Compare November 27, 2019 16:06
@mgol
Copy link
Member Author

mgol commented Nov 27, 2019

The issue to solve before landing this is that one test fails in esmodules mode:

HeadlessChrome 78.0.3904 (Mac OS X 10.15.1) selector pseudo - misc FAILED
	Tokenization stressor (a[class*=blog]:not(:has(*, :contains(!)), :contains(!)), br:contains(]), p:contains(]), :not(:empty):not(:parent):not(.qunit-source))
	Expected: [
	  <p id="ap"></p>,
	  <a href="https://diveintomark.org/" class="blog" hreflang="en" id="mark"></a>,
	  <a id="yahoo" href="https://www.yahoo.com/" class="blogTest"></a>,
	  <a href="https://simon.incutio.com/" class="blog link" id="simon"></a>
	]
	Actual: [
	  <p class="qunit-source qunit-collapsed"></p>,
	  <p id="ap"></p>,
	  <a href="https://diveintomark.org/" class="blog" hreflang="en" id="mark"></a>,
	  <a id="yahoo" href="https://www.yahoo.com/" class="blogTest"></a>,
	  <a href="https://simon.incutio.com/" class="blog link" id="simon"></a>
	]
	    at match (test/data/testinit.js:56:9)
	    at Assert.QUnit.assert.t (test/data/testinit.js:68:2)
	    at Object.<anonymous> (test/unit/selector.js:1095:10)
	    at runTest (node_modules/qunit/qunit/qunit.js:3044:30)
	    at Test.run (node_modules/qunit/qunit/qunit.js:3030:6)
	    at node_modules/qunit/qunit/qunit.js:3257:12

I don't understand why as <p class="qunit-source qunit-collapsed"></p> doesn't seem to match any of the selector parts provided...

@mgol mgol force-pushed the karma-esmodules branch 4 times, most recently from 057f819 to c29f6bf Compare November 30, 2019 21:54
@mgol
Copy link
Member Author

mgol commented Nov 30, 2019

What's also interesting is that the above test fails in Chrome but not in Firefox (headless or not).

@mgol
Copy link
Member Author

mgol commented Nov 30, 2019

OK, I know why it only failed in Chrome - this is because the .qunit-source element contains an error message from a browser and in Chrome it contains ]. I fixed the selector to avoid the issue.

Interesting that it only happens in esmodules mode, though.

@mgol mgol force-pushed the karma-esmodules branch 2 times, most recently from 93ba5c0 to 1d83394 Compare December 2, 2019 18:59
});
// Workaround: Remove call to `window.__karma__.loaded()`
// in favour of calling `window.__karma__.start()` from `loadTests()`
// because tests such as unit/ready.js should run after document ready.
Copy link
Member

Choose a reason for hiding this comment

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

Why have extra indentation here?

Copy link
Member Author

Choose a reason for hiding this comment

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

Ouch, it should be using spaces, not tabs, as the rest of the file.

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed.

@mgol
Copy link
Member Author

mgol commented Dec 9, 2019

@timmywil @gibson042 I realized I broke local PHP esmodules mode in #4554 (AMD mode works but that's less useful on master). I fixed those issues in this PR.

@mgol
Copy link
Member Author

mgol commented Dec 9, 2019

@gibson042 I pushed two new commits; the last one only reformats the Karma HTML files to use tabs as other files do; otherwise my IDE was listening to .editorconfig, constantly inserting tabs among existing spaces.

@mgol
Copy link
Member Author

mgol commented Dec 9, 2019

I pushed another commit that makes Karma AMD tests invoked on Travis. Based on this PR, it should be easy to enable Karma AMD testing on 3.x, finally getting rid of accidental failures that were introduced there from time to time.

@mgol mgol removed the Needs review label Dec 16, 2019
@mgol mgol merged commit 341c6d1 into jquery:master Dec 16, 2019
@mgol mgol deleted the karma-esmodules branch December 16, 2019 18:33
mgol added a commit to mgol/jquery that referenced this pull request Dec 16, 2019
Also, run such a suite in CI to make sure modules are working as expected
when used directly.

(partially cherry picked from 341c6d1)

Ref jquerygh-4550
mgol added a commit to mgol/jquery that referenced this pull request Dec 16, 2019
Also, run such a suite in CI to make sure modules are working as expected
when used directly.

(partially cherry picked from 341c6d1)

Ref jquerygh-4550
mgol added a commit to mgol/jquery that referenced this pull request Dec 16, 2019
PR jquerygh-4550 added support for running ES modules & AMD tests via Karma. This
required reading the `esmodules` & `amd` props from both `QUnit.config` &
`QUnit.urlParams`. By picking these two properties manually, the `dev` one
stopped being respected while ones handled directly by QUnit were fine (like
`hidepassed`). Instead of maintaining the full list of options, the code now
iterates over QUnit URL config and handles the fallbacks in a more generic way.

Apart from that, all jQuery source & test files are now read directly from disk
instead of being cached by Karma so that one can run `grunt karma:chrome-debug`
& work on a fix without restarting that Karma run after each change. A similar
effect could have been achieved by setting `autoWatch` to `true` but then the
main Karma page runs tests in an iframe by default when
`grunt karma:chrome-debug` is run instead of relying on the current debug flow.

Ref jquerygh-4550
mgol added a commit to mgol/jquery that referenced this pull request Dec 16, 2019
PR jquerygh-4550 added support for running ES modules & AMD tests via Karma. This
required reading the `esmodules` & `amd` props from both `QUnit.config` &
`QUnit.urlParams`. By picking these two properties manually, the `dev` one
stopped being respected while ones handled directly by QUnit were fine (like
`hidepassed`). Instead of maintaining the full list of options, the code now
iterates over QUnit URL config and handles the fallbacks in a more generic way.

Apart from that, all jQuery source & test files are now read directly from disk
instead of being cached by Karma so that one can run `grunt karma:chrome-debug`
& work on a fix without restarting that Karma run after each change. A similar
effect could have been achieved by setting `autoWatch` to `true` but then the
main Karma page runs tests in an iframe by default when
`grunt karma:chrome-debug` is run instead of relying on the current debug flow.

Ref jquerygh-4550
mgol added a commit to mgol/jquery that referenced this pull request Dec 16, 2019
PR jquerygh-4550 added support for running ES modules & AMD tests via Karma. This
required reading the `esmodules` & `amd` props from both `QUnit.config` &
`QUnit.urlParams`. By picking these two properties manually, the `dev` one
stopped being respected while ones handled directly by QUnit were fine (like
`hidepassed`). Instead of maintaining the full list of options, the code now
iterates over QUnit URL config and handles the fallbacks in a more generic way.

Apart from that, all jQuery source & test files are now read directly from disk
instead of being cached by Karma so that one can run `grunt karma:chrome-debug`
& work on a fix without restarting that Karma run after each change. A similar
effect could have been achieved by setting `autoWatch` to `true` but then the
main Karma page runs tests in an iframe by default when
`grunt karma:chrome-debug` is run instead of relying on the current debug flow.

Ref jquerygh-4550
mgol added a commit to mgol/jquery that referenced this pull request Dec 17, 2019
PR jquerygh-4550 added support for running ES modules & AMD tests via Karma. This
required reading the `esmodules` & `amd` props from both `QUnit.config` &
`QUnit.urlParams`. By picking these two properties manually, the `dev` one
stopped being respected while ones handled directly by QUnit were fine (like
`hidepassed`). Instead of maintaining the full list of options, the code now
iterates over QUnit URL config and handles the fallbacks in a more generic way.

Apart from that, all jQuery source & test files are now read directly from disk
instead of being cached by Karma so that one can run `grunt karma:chrome-debug`
& work on a fix without restarting that Karma run after each change. A similar
effect could have been achieved by setting `autoWatch` to `true` but then the
main Karma page runs tests in an iframe by default when
`grunt karma:chrome-debug` is run instead of relying on the current debug flow.

Ref jquerygh-4550
mgol added a commit to mgol/jquery that referenced this pull request Dec 17, 2019
PR jquerygh-4550 added support for running ES modules & AMD tests via Karma. This
required reading the `esmodules` & `amd` props from both `QUnit.config` &
`QUnit.urlParams`. By picking these two properties manually, the `dev` one
stopped being respected while ones handled directly by QUnit were fine (like
`hidepassed`). Instead of maintaining the full list of options, the code now
iterates over QUnit URL config and handles the fallbacks in a more generic way.

Apart from that, all jQuery source & test files are now read directly from disk
instead of being cached by Karma so that one can run `grunt karma:chrome-debug`
& work on a fix without restarting that Karma run after each change. A similar
effect could have been achieved by setting `autoWatch` to `true` but then the
main Karma page runs tests in an iframe by default when
`grunt karma:chrome-debug` is run instead of relying on the current debug flow.

Ref jquerygh-4550
mgol added a commit to mgol/jquery that referenced this pull request Jan 7, 2020
PR jquerygh-4550 added support for running ES modules & AMD tests via Karma. This
required reading the `esmodules` & `amd` props from both `QUnit.config` &
`QUnit.urlParams`. By picking these two properties manually, the `dev` one
stopped being respected while ones handled directly by QUnit were fine (like
`hidepassed`). Instead of maintaining the full list of options, the code now
iterates over QUnit URL config and handles the fallbacks in a more generic way.

Apart from that, all jQuery source & test files are now read directly from disk
instead of being cached by Karma so that one can run `grunt karma:chrome-debug`
& work on a fix without restarting that Karma run after each change. A similar
effect could have been achieved by setting `autoWatch` to `true` but then the
main Karma page runs tests in an iframe by default when
`grunt karma:chrome-debug` is run instead of relying on the current debug flow.

Ref jquerygh-4550
mgol added a commit that referenced this pull request Jan 7, 2020
PR gh-4550 added support for running ES modules & AMD tests via Karma. This
required reading the `esmodules` & `amd` props from both `QUnit.config` &
`QUnit.urlParams`. By picking these two properties manually, the `dev` one
stopped being respected while ones handled directly by QUnit were fine (like
`hidepassed`). Instead of maintaining the full list of options, the code now
iterates over QUnit URL config and handles the fallbacks in a more generic way.

Apart from that, all jQuery source & test files are now read directly from disk
instead of being cached by Karma so that one can run `grunt karma:chrome-debug`
& work on a fix without restarting that Karma run after each change. A similar
effect could have been achieved by setting `autoWatch` to `true` but then the
main Karma page runs tests in an iframe by default when
`grunt karma:chrome-debug` is run instead of relying on the current debug flow.

Closes gh-4574
Ref gh-4550
mgol added a commit to mgol/jquery that referenced this pull request Jan 8, 2020
Also, run such a suite in CI to make sure modules are working as expected
when used directly.

(partially cherry picked from 341c6d1)

Ref jquerygh-4550
Ref jquerygh-4574
mgol added a commit to mgol/jquery that referenced this pull request Jan 8, 2020
Also, run such a suite in CI to make sure modules are working as expected
when used directly.

(partially cherry picked from 341c6d1)
(partially cherry picked from 437f389)

Ref jquerygh-4550
Ref jquerygh-4574
mgol added a commit to mgol/jquery that referenced this pull request Jan 8, 2020
Also, run such a suite in CI to make sure modules are working as expected
when used directly.

(partially cherry picked from 341c6d1)
(partially cherry picked from 437f389)

Ref jquerygh-4550
Ref jquerygh-4574
gaohuia pushed a commit to gaohuia/jquery that referenced this pull request Jan 9, 2020
PR jquerygh-4550 added support for running ES modules & AMD tests via Karma. This
required reading the `esmodules` & `amd` props from both `QUnit.config` &
`QUnit.urlParams`. By picking these two properties manually, the `dev` one
stopped being respected while ones handled directly by QUnit were fine (like
`hidepassed`). Instead of maintaining the full list of options, the code now
iterates over QUnit URL config and handles the fallbacks in a more generic way.

Apart from that, all jQuery source & test files are now read directly from disk
instead of being cached by Karma so that one can run `grunt karma:chrome-debug`
& work on a fix without restarting that Karma run after each change. A similar
effect could have been achieved by setting `autoWatch` to `true` but then the
main Karma page runs tests in an iframe by default when
`grunt karma:chrome-debug` is run instead of relying on the current debug flow.

Closes jquerygh-4574
Ref jquerygh-4550
@mgol mgol mentioned this pull request Jan 20, 2020
2 tasks
mgol added a commit that referenced this pull request Jan 21, 2020
Also, run such a suite in CI to make sure modules are working as expected
when used directly.

(partially cherry picked from 341c6d1)
(partially cherry picked from 437f389)

Closes gh-4595
Ref gh-4550
Ref gh-4574
@lock lock bot locked as resolved and limited conversation to collaborators Jun 24, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Development

Successfully merging this pull request may close these issues.

None yet

3 participants