-
Notifications
You must be signed in to change notification settings - Fork 12k
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
fix(tests): add global scripts in karma plugin #3543
fix(tests): add global scripts in karma plugin #3543
Conversation
@YonatanKra can you also review this approach, and confirm it works for your scenario? I opted to add these changes in the karma plugin to ensure that karma can be ran separately from the |
20c5cb6
to
24aed4d
Compare
A similar approach would work for serving |
// Unshift elements onto the beginning of the files array. | ||
// It's important to not replace the array, because | ||
// karma already has a reference to the existing array. | ||
Array.prototype.unshift.apply(config.files, globalScriptPatterns); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
config.files.unshift(...globalScriptsPatterns)
is simpler.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can't do that in the karma plugin since it's not preprocessed and node4 does not support the spread operator (without a flag).
I hope to tackle that in #3605 by making the plugin be in TypeScript.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds good.
Note: renamed and lazy global scripts are not supported. Fix angular#2897
24aed4d
to
86a4eef
Compare
@Meligy I had to remove your test setup code because I couldn't have tests afterwards without manually cleaning up the previous ones:
Our test scripts already reset code to it's initial state after each test so it's better to have one file for each test. |
That's perfect. I didn't want to change / introduce file convention in previous commit, but the current way in this PR is super awesome. |
LGTM. Cheers! |
Note: renamed and lazy global scripts are not supported. Fix angular#2897
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
Note: renamed and lazy global scripts are not supported.
Fix #2897