Skip to content
This repository was archived by the owner on Apr 12, 2024. It is now read-only.

fix($window): allow $window to be mocked in unit tests #15686

Closed
wants to merge 1 commit into from

Conversation

mattlewis92
Copy link
Contributor

What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)
Bug fix

What is the current behavior? (You can also link to an open issue here)
#15685

What is the new behavior (if this is a feature change)?
N/A

Does this PR introduce a breaking change?
No

Please check if the PR fulfills these requirements

Other information:

Copy link
Member

@gkalpak gkalpak left a comment

Choose a reason for hiding this comment

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

Can you also change the commit message to fix($jsonpCallbacks): ...?

@@ -11,7 +11,7 @@
*/
var $jsonpCallbacksProvider = /** @this */ function() {
this.$get = ['$window', function($window) {
var callbacks = $window.angular.callbacks;
var callbacks = $window.angular ? $window.angular.callbacks : {};
Copy link
Member

Choose a reason for hiding this comment

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

Actually, I think it makes more sense to just use angular directly:

-     var callbacks = $window.angular.callbacks;
+     var callbacks = angular.callbacks;

@mattlewis92
Copy link
Contributor Author

@gkalpak done 😄

@@ -10,8 +10,8 @@
* how they vary compared to the requested url.
*/
var $jsonpCallbacksProvider = /** @this */ function() {
this.$get = ['$window', function($window) {
var callbacks = $window.angular.callbacks;
this.$get = [function() {
Copy link
Member

Choose a reason for hiding this comment

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

We don't need the [...] now. It can be just this.$get = function() {...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

@mattlewis92
Copy link
Contributor Author

CI failure is a flake - the API limit on saucelabs has been hit

@gkalpak gkalpak closed this in 0ce9348 Feb 8, 2017
gkalpak pushed a commit that referenced this pull request Feb 8, 2017
AllenBW added a commit to AllenBW/manageiq-ui-service that referenced this pull request Feb 27, 2017
Adjust session tests for this: angular/angular.js#15686, should be able to remove in next angular release
AllenBW added a commit to AllenBW/manageiq-ui-service that referenced this pull request Mar 6, 2017
Adjust session tests for this: angular/angular.js#15686, should be able to remove in next angular release
ellimist pushed a commit to ellimist/angular.js that referenced this pull request Mar 15, 2017
AllenBW added a commit to AllenBW/manageiq-ui-service that referenced this pull request Mar 15, 2017
Adjust session tests for this: angular/angular.js#15686, should be able to remove in next angular release
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants