After writing some ~1k tests using $httpBackend
, I've come to the conclusion that the current fashion of requests needing to be "expected" first is difficult to work with. Couple reasons:
In general, it would be better to have an API that accepts all requests generated from the source code, and allows you to flush specific requests with their data when needed.
To provide some more details, examples, and use cases, here is a more detailed description of the above points.
For example, here's roughly what I have for one test. This test checks source code which:
A. Calls an endpoint /user
, and when that is done,
B. Calls /account-details
, and when that is done,
C. Shows the information.
Example Source Code (please forgive the contrived example):
``` javascript
showSpinner();
loadUser()
.then( loadAccountDetails )
.finally( hideSpinner );
function loadUser() {
return $http( { url: '/user' } ).then( function( response ) {
$scope.username = response.data.username;
return response.data;
} );
}
function loadAccountDetails( userObj ) {
return $http( { url: '/account-details?userId=' + userObj.id } ).then( function( response ) {
$scope.phoneNumber = response.data.phoneNumber;
return response.data;
} );
}
```
Test with current expectGET()
:
``` javascript
it( "should display all the information when all requests are complete", function() {
$httpBackend.expectGET( '/user' ).respond( 200, { id: 1, username: 'User 1' } ); // must expect before page is created, since it makes the request immediately
var page = createPage();
page.expectLoadingSpinnerVisible(); // spinner visible while waiting for data
$httpBackend.expectGET( '/account-details?userId=1' ).respond( 200, { phone: 'phoneNum' } ); // must expect this before flushing first request, since this is called when '/user' is complete
$httpBackend.flush( 1 ); // flush the '/user' request
page.expectLoadingSpinnerVisible(); // loading spinner still visible while waiting for account details
$httpBackend.flush(); // now flush the '/account-details' request
page.expectLoadingSpinnerHidden();
page.expectPhoneNumber( 'phoneNum' );
} );
```
This test would be much more readable with something like respondToGET()
:
``` javascript
it( "should display all the information when all requests are complete", function() {
var page = createPage();
page.expectLoadingSpinnerVisible(); // spinner visible while waiting for data
$httpBackend.respondToGET( '/user' ).with( 200, { id: 1, username: 'User 1' } );
page.expectLoadingSpinnerVisible(); // loading spinner still visible while waiting for account details
$httpBackend.respondToGET( '/account-details?userId=1' ).with( 200, { phone: 'phoneNum' } );
page.expectLoadingSpinnerHidden();
page.expectPhoneNumber( 'phoneNum' );
} );
```
Let's say we have a page or component that loads data for two sections in parallel. Each section should display its data as soon as the corresponding request is complete.
|----------------| |----------------|
| | | |
| Account | | Recent |
| Details | | History |
| Section | | Section |
| | | |
|----------------| |----------------|
``` javascript
it( "should display spinners over both areas, and when requests are complete, show the information", function() {
$httpBackend.expectGET( '/account-details' ).respond( 200, { phone: 'phoneNum' } ); // must expect before page is created, since it makes the request immediately
$httpBackend.expectGET( '/recent-history' ).respond( 200, [ ... ] ); // must expect before page is created, since it makes the request immediately
var page = createPage();
page.expectAccountDetailsSpinnerVisible(); // spinner visible while waiting for data
page.expectRecentHistorySpinnerVisible(); // spinner visible while waiting for data
$httpBackend.flush( 1 ); // I can only flush the '/account-details' request
// here - not the '/recent-history' request
page.expectAccountDetailsSpinnerHidden();
page.expectPhoneNum( 'phoneNum' );
page.expectRecentHistorySpinnerVisible(); // spinner still visible while waiting for recent history data
$httpBackend.flush( 1 ); // now can flush 2nd request, '/recent-history'
page.expectRecentHistorySpinnerHidden();
page.expectRecentHistory( [ ... ] );
} );
```
Again, a simpler API which would allow flexibility on which request is resolved first (and also results in shorter code) would be:
``` javascript
it( "should display spinners over both areas, and when requests are complete, show the information", function() {
var page = createPage();
page.expectAccountDetailsSpinnerVisible(); // spinner visible while waiting for data
page.expectRecentHistorySpinnerVisible(); // spinner visible while waiting for data
$httpBackend.respondToGET( '/recent-history' ).with( 200, [ ... ] ); // I have the ability to resolve this one first
page.expectRecentHistorySpinnerHidden();
page.expectRecentHistory( [ ... ] );
page.expectAccountDetailsSpinnerVisible(); // spinner visible while waiting for account-details data
$httpBackend.respondToGET( '/account-details' ).with( 200, { phone: 'phoneNum' } );
page.expectAccountDetailsSpinnerHidden();
page.expectPhoneNum( 'phoneNum' );
} );
```
Here's what I'm thinking for an API:
To maintain backward compatibility with throwing errors for "unexpected requests," it seems like we'd need to put $httpBackend
into an "accept all / respond" mode so we can respond to the requests later:
beforeEach( inject( function( $httpBackend ) {
$httpBackend.respondMode();
} ) );
Then we would flush requests when we need to with:
respondToGET( url, [headers], [keys] ).with( [statusCode], data )
respondToPOST( url, [data], [headers], [keys] ).with( [statusCode], data )
.Thoughts?
That's a lot of text. While I appreciate details, can you give a short version that mentions the main issue and your proposed solution?
Yes, good call. Short version: Instead of
it( "should make a request", function() {
$httpBackend.expectGET( '/recent-history' ).respond( 200, { ... } );
$http( { url: '/recent-history' } ).then( successFn );
$httpBackend.flush();
} );
Allow us to do something like:
it( "should make a request", function() {
$http( { url: '/recent-history' } ).then( successFn );
$httpBackend.respondToGET( '/recent-history' ).with( 200, { ... } );
} );
Okay, thanks for the short version. Since this seems basically to be a "cosmetic" change, I wonder if it would be better to make that available as a third-party module. Also, I think the original behavior has some merits, as in tests you want to mock the data / responses before actually making them, instead after the fact.
So as I mentioned in my "long version," mocking the data / responses before actually making the requests is exactly what is difficult to work with. What are the merits that you see for the pre-defined model vs. responding to requests afterwards? I personally don't see any (or at least none that couldn't be easily solved with a helper function).
Tbh, our tests have become so hairy trying to work with multiple http requests on pages, and we're constantly breaking tests by moving requests into different orderings in the model/service layer and what-not. But we still want the ability to check that all requests have been satisfied, so we must still use expectGET()
instead of whenGET()
.
Btw, I'm not sure that this could be a third-party module, as everything seems pretty baked into the ng mock source code. Thoughts?