-
Notifications
You must be signed in to change notification settings - Fork 27.4k
fix($window): allow $window to be mocked in unit tests #15686
Conversation
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.
Can you also change the commit message to fix($jsonpCallbacks): ...
?
src/ng/jsonpCallbacks.js
Outdated
@@ -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 : {}; |
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.
Actually, I think it makes more sense to just use angular
directly:
- var callbacks = $window.angular.callbacks;
+ var callbacks = angular.callbacks;
dd8efbf
to
fffb1bd
Compare
@gkalpak done 😄 |
src/ng/jsonpCallbacks.js
Outdated
@@ -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() { |
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 don't need the [...]
now. It can be just this.$get = function() {...
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.
Done
fffb1bd
to
c5f47a9
Compare
CI failure is a flake - the API limit on saucelabs has been hit |
c5f47a9
to
fca0638
Compare
Adjust session tests for this: angular/angular.js#15686, should be able to remove in next angular release
Adjust session tests for this: angular/angular.js#15686, should be able to remove in next angular release
Adjust session tests for this: angular/angular.js#15686, should be able to remove in next angular release
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
Docs have been added / updated (for bug fixes / features)Other information: