This repository was archived by the owner on Apr 12, 2024. It is now read-only.
feat(ngMock, ngMockE2E): add option to match latest definition for $h… #16560
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…ttpBackend request Closes angular#16251 Closes angular#11637
Narretz
force-pushed
the
feat-ngMock-match-latest
branch
from
f3fa83b to
35aaef3
Compare
gkalpak
approved these changes
May 17, 2018
petebacondarwin
suggested changes
May 17, 2018
| while ((definition = definitions[++i])) { | ||
| var i = matchLatestDefinition ? definitions.length : -1, definition; | ||
|
|
||
| while ((definition = definitions[matchLatestDefinition ? --i : ++i])) { |
There was a problem hiding this comment.
This is dangerous stuff due to the subtle difference between prefix and postfix operators. But given that we were doing this already I guess it'll be fine.
src/ngMock/angular-mocks.js
Outdated
| @@ -1507,6 +1509,15 @@ function createHttpBackendMock($rootScope, $timeout, $delegate, $browser) { | |||
| return chain; | |||
| }; | |||
|
|
|||
| $httpBackend.matchLatestDefinition = function(value) { | |||
|
|
|||
There was a problem hiding this comment.
is this blank line needed / in keeping with the rest of the code?
src/ngMock/angular-mocks.js
Outdated
| $httpBackend.matchLatestDefinition = function(value) { | ||
|
|
||
| if (isDefined(value)) { | ||
| matchLatestDefinition = value; |
There was a problem hiding this comment.
don't we normally return this when it is a setter to allow chaining?
test/ngMock/angular-mocksSpec.js
Outdated
| hb.flush(); | ||
| expect(callback).toHaveBeenCalledOnce(); | ||
| } | ||
| ); |
There was a problem hiding this comment.
Do we already have tests for the order when this is false?
test/ngMock/angular-mocksSpec.js
Outdated
| hb('GET', '/url1', null, callback); | ||
| expect(callback).not.toHaveBeenCalled(); | ||
| hb.flush(); | ||
| expect(callback).toHaveBeenCalledOnce(); |
There was a problem hiding this comment.
Should we check that it continues to work backwards if the last one doesn't match?
src/ngMock/angular-mocks.js
Outdated
| @@ -1507,6 +1509,15 @@ function createHttpBackendMock($rootScope, $timeout, $delegate, $browser) { | |||
| return chain; | |||
| }; | |||
|
|
|||
| $httpBackend.matchLatestDefinition = function(value) { | |||
There was a problem hiding this comment.
I think we needed docs here.
… for $httpBackend request
Narretz
force-pushed
the
feat-ngMock-match-latest
branch
from
235d954 to
ed92fb2
Compare
petebacondarwin
approved these changes
May 19, 2018
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
…ttpBackend request
Closes #16251
Closes #11637
What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)
Feature
What is the current behavior? (You can also link to an open issue here)
See #11637
What is the new behavior (if this is a feature change)?
Config option to change the behavior
Does this PR introduce a breaking change?
No
Please check if the PR fulfills these requirements
Other information:
First draft to discuss the API.
As noted in #16251, it's not possible (afaics) to put the option on the httpBackendProvider, because they cannot be decorated. It's also not useful to put the method on the ng.httpBackendProvider, because that's not where it's needed. The option to switch it directly from httpBackend could also be used to migrate tests step by step or use it only for new tests. Another option would be a new "$httpBackendMocksProvider" or similar (but I don't think that's very useful).