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

Tests: Allow Karma to load unminfied source #4128

Merged
merged 8 commits into from Sep 7, 2018

Conversation

gibson042
Copy link
Member

Summary

Makes Karma behave more like the old tests by exposing a "load unminified" checkbox.

Checklist

@timmywil timmywil added this to the 3.4.0 milestone Sep 3, 2018
@timmywil
Copy link
Member

timmywil commented Sep 3, 2018

@gibson042 The grunt karma:main task seems to stall.

@gibson042
Copy link
Member Author

@timmywil That was surprisingly tricky to address, but should be good to go now.

Copy link
Member

@mgol mgol left a comment

Choose a reason for hiding this comment

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

The AMD mode doesn't seem to work. For example, choosing the support module and enabling AMD mode leads to an all-red outcome. Moreover, most modules are not available under the dropdown when the AMD mode is selected. Pressing "Rerun" on a specific test doesn't fire tests just for it but also for many others, etc.

// except when running tests in Karma (See Gruntfile)
// Tests are always loaded async
// except when running tests in Karma (See Gruntfile)
if ( !window.__karma__ ) {
Copy link
Member

Choose a reason for hiding this comment

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

A double space incorrectly got inserted after __karma__.

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.

@gibson042 gibson042 force-pushed the 2018-07-karma-debug-unminified branch from 782e643 to 22d3349 Compare September 5, 2018 15:42
@gibson042
Copy link
Member Author

Right, AMD behavior is at odds with the way the tests are currently loaded in Karma. I just removed that mode, because it doesn't provide much value over e.g. grunt karma:chrome-debug anyway.

@mgol
Copy link
Member

mgol commented Sep 5, 2018

I just removed that mode, because it doesn't provide much value over e.g. grunt karma:chrome-debug anyway.

What do you mean by that?

@gibson042
Copy link
Member Author

I typed the wrong example command, and looking back it wasn't really coherent anyway. Please disregard.

However, we still don't have AMD mode in Karma, and I'm disinclined to force that in scope for this PR (there's no loss, since current master lacks both AMD and unminified). Someone else can make those changes later if they so desire.

@mgol
Copy link
Member

mgol commented Sep 5, 2018

I agree, adding AMD support is a separate feature request.

Copy link
Member

@mgol mgol left a comment

Choose a reason for hiding this comment

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

Looks good to me! A really nice improvement.

Copy link
Member

@timmywil timmywil left a comment

Choose a reason for hiding this comment

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

LGTM

@gibson042 gibson042 merged commit dfa92cc into jquery:master Sep 7, 2018
@lock lock bot locked as resolved and limited conversation to collaborators Mar 6, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants