-
Notifications
You must be signed in to change notification settings - Fork 27.4k
feat(ngMocks): Describe unflushed http requests #15928
Conversation
The current implementation of $httpBackend.verifyNoOutstandingRequest gives an integer number describing how many requests are unflushed. While it's superficially easy to solve test errors from that message by simply adding an additional $httpBackend.flush(), if a developer is truly not expecting the code to make further requests this is not ideal. This change explicitly prints out which additional requests remain unflushed in the error message, helping her determine if the code needs changing, or if an additional flush is appropriate. Before this change: Unflushed requests: 1 After this change: Unflushed requests: GET /some Closes angular#10596
548c0a0
to
7bd05f7
Compare
The test failure from travis seems, to my novice eyes, unrelated - it looks like a flaky test around ngHide? |
Hi @jakewins , thanks for the PR. I think this is a good change. Can you change the output so that the number of unflushed requests is still included? And expand the tests to include more than one unflushed request? |
@Narretz for sure. Is the preferred approach to |
@jakewins Additional commits are fine, I will squas wehn I merge |
src/ngMock/angular-mocks.js
Outdated
var unflushedDescriptions = []; | ||
for (var i = 0, len = responses.length; i < len; ++i) { | ||
unflushedDescriptions.push(responses[i].description); | ||
} |
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.
Nit: var unflushedDescriptions = responses.map(function(res) { return res.description; });
is shorter.
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.
LGTM
The current implementation of $httpBackend.verifyNoOutstandingRequest gives an integer number describing how many requests are unflushed. While it's superficially easy to solve test errors from that message by simply adding an additional $httpBackend.flush(), if a developer is truly not expecting the code to make further requests this is not ideal. This change explicitly prints out which additional requests remain unflushed in the error message, helping her determine if the code needs changing, or if an additional flush is appropriate. Before this change: Unflushed requests: 1 After this change: Unflushed requests: 1 GET /some Closes #10596 Closes #15928
The current implementation of $httpBackend.verifyNoOutstandingRequest
gives an integer number describing how many requests are unflushed.
While it's superficially easy to solve test errors from that message
by simply adding an additional $httpBackend.flush(), if a developer
is truly not expecting the code to make further requests this is
not ideal.
This change explicitly prints out which additional requests remain
unflushed in the error message, helping her determine if the code
needs changing, or if an additional flush is appropriate.
Before this change:
After this change:
Closes #10596
What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)
A feature improving the output of ngMocks "Unflushed requests" error message.
What is the current behavior? (You can also link to an open issue here)
The error message includes the number of requests remaining unflushed.
What is the new behavior (if this is a feature change)?
The error message now includes a brief description of which requests remain unflushed.
Does this PR introduce a breaking change?
No - unless the error messages are considered public API.
Please check if the PR fulfills these requirements
Other information: