fix(common): include query parameters for open HTTP requests in verify (#44917)

JoostK · thePunderWoman · commit a2cbffc06f25 · 2022-02-01T21:41:43.000Z
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
diff --git a/packages/common/http/testing/src/backend.ts b/packages/common/http/testing/src/backend.ts
@@ -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);
@@ -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}`);
     }
   }
@@ -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}`;
+}
diff --git a/packages/common/http/testing/test/request_spec.ts b/packages/common/http/testing/test/request_spec.ts
@@ -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');
+    }
+  });
 });

Original file line number	Diff line number	Diff line change
`@@ -95,13 +95,7 @@ export class HttpClientTestingBackend implements HttpBackend, HttpTestingControl`
`95`	`95`	let message = `Expected one matching request for criteria "${description}", found none.`;
`96`	`96`	`if (this.open.length > 0) {`
`97`	`97`	`// Show the methods and URLs of open requests in the error, for convenience.`
`98`		`- const requests = this.open`
`99`		`- .map(testReq => {`
`100`		`- const url = testReq.request.urlWithParams;`
`101`		`- const method = testReq.request.method;`
`102`		- return `${method} ${url}`;
`103`		`- })`
`104`		`- .join(', ');`
	`98`	`+ const requests = this.open.map(describeRequest).join(', ');`
`105`	`99`	message += ` Requests received are: ${requests}.`;
`106`	`100`	`}`
`107`	`101`	`throw new Error(message);`
`@@ -135,12 +129,7 @@ export class HttpClientTestingBackend implements HttpBackend, HttpTestingControl`
`135`	`129`	`}`
`136`	`130`	`if (open.length > 0) {`
`137`	`131`	`// Show the methods and URLs of open requests in the error, for convenience.`
`138`		`- const requests = open.map(testReq => {`
`139`		`- const url = testReq.request.urlWithParams.split('?')[0];`
`140`		`- const method = testReq.request.method;`
`141`		- return `${method} ${url}`;
`142`		`- })`
`143`		`- .join(', ');`
	`132`	`+ const requests = open.map(describeRequest).join(', ');`
`144`	`133`	throw new Error(`Expected no open requests, found ${open.length}: ${requests}`);
`145`	`134`	`}`
`146`	`135`	`}`
`@@ -158,3 +147,9 @@ export class HttpClientTestingBackend implements HttpBackend, HttpTestingControl`
`158`	`147`	`}`
`159`	`148`	`}`
`160`	`149`	`}`
	`150`	`+`
	`151`	`+function describeRequest(testRequest: TestRequest): string {`
	`152`	`+ const url = testRequest.request.urlWithParams;`
	`153`	`+ const method = testRequest.request.method;`
	`154`	+ return `${method} ${url}`;
	`155`	`+}`