Skip to content

Commit

Permalink
fix(common): include query parameters for open HTTP requests in `veri…
Browse files Browse the repository at this point in the history
…fy` (#44917)

When `HttpTestingController.verify` is used to verify that there are not open,
unexpected requests it would throw an error with the method and URL of all pending
requests, excluding the query parameters. This is confusing, as e.g. `expectOne`
matches a URL including its query parameters and `expectOne` does include the
query parameters when it reports when no request could be matched.

This commit changes the error that is reported by `verify` to include the query
parameters.

Closes #19974

PR Close #44917
  • Loading branch information
JoostK authored and thePunderWoman committed Feb 1, 2022
1 parent 812c1ba commit b4e4617
Show file tree
Hide file tree
Showing 2 changed files with 26 additions and 13 deletions.
21 changes: 8 additions & 13 deletions packages/common/http/testing/src/backend.ts
Expand Up @@ -95,13 +95,7 @@ export class HttpClientTestingBackend implements HttpBackend, HttpTestingControl
let message = `Expected one matching request for criteria "${description}", found none.`;
if (this.open.length > 0) {
// Show the methods and URLs of open requests in the error, for convenience.
const requests = this.open
.map(testReq => {
const url = testReq.request.urlWithParams;
const method = testReq.request.method;
return `${method} ${url}`;
})
.join(', ');
const requests = this.open.map(describeRequest).join(', ');
message += ` Requests received are: ${requests}.`;
}
throw new Error(message);
Expand Down Expand Up @@ -135,12 +129,7 @@ export class HttpClientTestingBackend implements HttpBackend, HttpTestingControl
}
if (open.length > 0) {
// Show the methods and URLs of open requests in the error, for convenience.
const requests = open.map(testReq => {
const url = testReq.request.urlWithParams.split('?')[0];
const method = testReq.request.method;
return `${method} ${url}`;
})
.join(', ');
const requests = open.map(describeRequest).join(', ');
throw new Error(`Expected no open requests, found ${open.length}: ${requests}`);
}
}
Expand All @@ -158,3 +147,9 @@ export class HttpClientTestingBackend implements HttpBackend, HttpTestingControl
}
}
}

function describeRequest(testRequest: TestRequest): string {
const url = testRequest.request.urlWithParams;
const method = testRequest.request.method;
return `${method} ${url}`;
}
18 changes: 18 additions & 0 deletions packages/common/http/testing/test/request_spec.ts
Expand Up @@ -91,4 +91,22 @@ describe('HttpClient TestRequest', () => {
' Requests received are: GET /some-other-url?query=world, POST /and-another-url.');
}
});

it('throws if there are open requests when verify is called', () => {
const mock = new HttpClientTestingBackend();
const client = new HttpClient(mock);

client.get('/some-other-url?query=world').subscribe();
client.post('/and-another-url', {}).subscribe();

try {
mock.verify();
fail();
} catch (error) {
expect(error.message)
.toBe(
'Expected no open requests, found 2:' +
' GET /some-other-url?query=world, POST /and-another-url');
}
});
});

0 comments on commit b4e4617

Please sign in to comment.