refactor(mocks) : changed the implementations of expect any URL #8462
Conversation
…undefined/null to *
|
I'm sorry, but I wasn't able to verify your Contributor License Agreement (CLA) signature. CLA signature is required for any code contributions to AngularJS. Please sign our CLA and ensure that the CLA signature email address and the email address in this PR's commits match. If you signed the CLA as a corporation, please let us know the company's name. Thanks a bunch! PS: If you signed the CLA in the past then most likely the email addresses don't match. Please sign the CLA again or update the email address in the commit of this PR. |
|
CLA signature verified! Thank you! Someone from the team will now triage your PR and it will be processed based on the determined priority (doc updates and fixes with tests are prioritized over other changes). |
| @@ -1548,7 +1548,8 @@ function MockHttpExpectation(method, url, data, headers) { | |||
| }; | |||
|
|
|||
| this.matchUrl = function(u) { | |||
| if (!url) return true; | |||
| if (url === undefined) return false; | |||
There was a problem hiding this comment.
I would go further and put an assertion in the MockHttpExpectation function that throws an explicit error if the url is undefined.
Then you don't need this line here at all because by the time you get here you are guaranteed that the URL is not undefined.
And also the user gets a clear error message if they failed to provide a URL.
|
Closes #8442 |
|
Ok I changed it to throw new error, when URL is undefined |
|
you mean when url is null :p |
|
Nah...when URL is null, it will let it through as ANY URL. When undefined it will throw new error. :) |
|
Oh, you're right, I misread =) Anyways, I'm torn --- I don't think it really makes sense to break this behaviour --- you can never really request an undefined URL, it might be better to just make the documentation explicit that the URL is optional. I wouldn't personally mind requiring an explicit request for "any URL", but I don't really see what the benefit of it is, other than causing people to examine and fix their tests when upgrading |
|
I get that it's a rare case, but why I requested this is because in our project we have Rest URLs defined in a seperate file and by our mistake we had few tests that injected the factory that returned the content of this file to a test and used those URLs e.g. Obj.login.url.url1. At some point those variables were named bit better for example to a Obj.login. Then first variable that was implemented returned undefined, but tests were still passing despite that structure of object was changed. Silly mistake, but those tests did not actually work properly :). That's why I requested this. |
petebacondarwin
force-pushed
the
master
branch
from
02dc2aa to
fd2d6c0
Compare
|
Please see: #8442 (comment) |
jeffbcross
force-pushed
the
master
branch
4 times, most recently
from
e8dc429 to
e83fab9
Compare
petebacondarwin
force-pushed
the
master
branch
from
4dd5a20 to
998c61c
Compare
|
I totally agree with @IgorMinar - IMO it feels natural to expect that omitted URL matches any request. I don't see any reason of introducing a breaking change here. @Kapaacius would you be updating your PR based on the suggestion? |
While the `url` parameter is optional for `$httpBackend.when`, `$httpBackend.expect` and related shortcut methods, it should not have the value of `undefined` if it has been provided. This change ensures that an error is thrown in those cases. Closes angular#8442 Closes angular#8462 Closes angular#10934 BREAKING CHANGE It is no longer valid to explicitly pass `undefined` as the `url` argument to any of the `$httpBackend.when...()` and `$httpBackend.expect...()` methods. While this argument is optional, it must have a defined value if it is provided. Previously passing an explicit `undefined` value was ignored but this lead to invalid tests passing unexpectedly.
petebacondarwin
modified the milestones:
1.5.x - migration-facilitation,
1.3.x - superluminal-nudge
While the `url` parameter is optional for `$httpBackend.when`, `$httpBackend.expect` and related shortcut methods, it should not have the value of `undefined` if it has been provided. This change ensures that an error is thrown in those cases. Closes angular#8442 Closes angular#8462 Closes angular#10934 BREAKING CHANGE It is no longer valid to explicitly pass `undefined` as the `url` argument to any of the `$httpBackend.when...()` and `$httpBackend.expect...()` methods. While this argument is optional, it must have a defined value if it is provided. Previously passing an explicit `undefined` value was ignored but this lead to invalid tests passing unexpectedly.
While the `url` parameter is optional for `$httpBackend.when`, `$httpBackend.expect` and related shortcut methods, it should not have the value of `undefined` if it has been provided. This change ensures that an error is thrown in those cases. Closes angular#8442 Closes angular#8462 Closes angular#10934 Closes angular#12777 BREAKING CHANGE It is no longer valid to explicitly pass `undefined` as the `url` argument to any of the `$httpBackend.when...()` and `$httpBackend.expect...()` methods. While this argument is optional, it must have a defined value if it is provided. Previously passing an explicit `undefined` value was ignored but this lead to invalid tests passing unexpectedly.
Changed $httpBackend.expect any URL ( expect('GET') ) from undefined / null to * ( expect('GET', '*') )