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

httpBackend's verify methods don't catch unexpected http calls #15855

Closed
1 of 3 tasks
cmplank opened this issue Mar 23, 2017 · 9 comments
Closed
1 of 3 tasks

httpBackend's verify methods don't catch unexpected http calls #15855

cmplank opened this issue Mar 23, 2017 · 9 comments

Comments

@cmplank
Copy link

cmplank commented Mar 23, 2017

I'm submitting a ...

  • bug report
  • feature request
  • other (Please do not submit support requests here (see above))

Current behavior:
Starting in angular 1.6.0, when a unit test exercises an http call but doesn't specify an httpBackend expectation, neither the httpBackend.verifyNoOutstandingExpectation() nor the httpBackend.verifyNoOutstandingRequest() method throw an error. This problem still manifests in the latest snapshot.

Expected / new behavior:
Previously (on angular 1.5.11) each of the verify methods threw errors with the message "Unexpected request: GET No more request expected" when an http method was called without an accompanying expectation.

Minimal reproduction of the problem with instructions:
Write a unit test that exercises a $http call but don't tell httpBackend to expect it. Call httpBackend.verifyNoOutstandingExpectation() and httpBackend.verifyNoOutstandingRequest() and neither of them will throw an error and your test will pass.

Plunk on angular snapshot: http://plnkr.co/edit/DZZ2Nl?p=preview
Change the angular version to 1.5.11 and the test will pass.

Angular version: 1.6.3
This is an issue in 1.6.0 through the most recent snapshot of angular. An interesting note is that it is not the version of angular-mocks that makes the difference as I would have expected, but rather the version of core angular.

Browser: [all]
Happens in node/karma/jasmine tests and in Chrome jasmine runner (see plunk)

Anything else:

@Narretz
Copy link
Contributor

Narretz commented Mar 24, 2017

It looks like this is related to the error handler in your then function. If you remove the empty error handler, verify throws. Maybe because of c9dffde

@Narretz
Copy link
Contributor

Narretz commented Mar 27, 2017

@gkalpak could this be related to the changes in the $q error handling?

@gkalpak
Copy link
Member

gkalpak commented Mar 29, 2017

Yes. When there is an unexpected request, $httpBackend throws, which causes the Promise returned by $http.get() to be rejected. Before 1.6, a thrown error was also passed to the $exceptionHandler, which rethrows by default in tests (hence you get an exception thrown). In 1.6. (due to e13eeab), thrown errors are treated the same as regular rejections, i.e. just reject the promise and don't get passed to the $exceptionHandler.

If there is an error handler somewhere down the promise chain, then the rejection is passed to that and the handler is responsible for taking over. If there is no error handler though, a Possibly Unhandled Rejection is passed to the $exceptionHandler (due to c9dffde), which again rethrows by default in tests.

@gkalpak
Copy link
Member

gkalpak commented Mar 29, 2017

If we want to restore the previous behavior (which sounds reasonable for the case where there is an unexpected or pending request), then we need to explicitly call $excptionHandler from the mock $httpBackend.

@kenjiqq
Copy link

kenjiqq commented Apr 25, 2017

I would think this invalidates a lot of unit tests around the work that relied on this functionality. I know for us there are now a lot of test passing that should not be without anyone noticing.
This has forced us to downgrade to the previous version since our 4000 test suit suddenly is very lacking.

@gkalpak
Copy link
Member

gkalpak commented Apr 26, 2017

If anyone interested in taking a stab at it, please do 😃

@marcin-wosinek
Copy link
Contributor

marcin-wosinek commented Jul 21, 2017

something that seems to be closely related :
http://plnkr.co/edit/VMBlQVRLzExJVviBpDrs?p=preview

flash will fail, if there were not matching expect to the request

my bad, I've been just hitting the original issue

@marcin-wosinek
Copy link
Contributor

The unit tests that are meant to catch the issue:
marcin-wosinek@912175b

@marcin-wosinek
Copy link
Contributor

marcin-wosinek commented Aug 6, 2017

Just running $exceptionHandler from the mocked $httpBackend (8502070#diff-2a255ed5e9564e25ce6eb711b604f40fR1449) changes nothing - the handler just throws the same error, and seems that it's caught by $q try/catch anyway.

The PRed change is rather ugly (but it passes proposed tests); the possible improvement would be to have a specific error class that is rethrown by $q

I've tried $timeout/setTimeout tricks to throw the error outside the try/catch block, but it was ending to be thrown outside the test blocks too - rather unwanted behaviour, and difficult to be tested.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

5 participants