From 7c77c9f2afaf255fb9a43c51faeeb8ee11216966 Mon Sep 17 00:00:00 2001 From: Deyaaeldeen Almahallawi Date: Thu, 16 Apr 2026 23:49:10 +0000 Subject: [PATCH 01/17] Improve test coverage to ~100% for @azure/core-client-rest Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../test/internal/clientHelpers.spec.ts | 16 ++- .../test/internal/getClient.spec.ts | 114 ++++++++++++++++++ .../keyCredentialAuthenticationPolicy.spec.ts | 31 +++++ .../internal/operationOptionHelpers.spec.ts | 23 ++++ 4 files changed, 183 insertions(+), 1 deletion(-) create mode 100644 sdk/core/core-client-rest/test/internal/keyCredentialAuthenticationPolicy.spec.ts create mode 100644 sdk/core/core-client-rest/test/internal/operationOptionHelpers.spec.ts diff --git a/sdk/core/core-client-rest/test/internal/clientHelpers.spec.ts b/sdk/core/core-client-rest/test/internal/clientHelpers.spec.ts index 43efcd8504f0..76cb26e178f2 100644 --- a/sdk/core/core-client-rest/test/internal/clientHelpers.spec.ts +++ b/sdk/core/core-client-rest/test/internal/clientHelpers.spec.ts @@ -2,7 +2,7 @@ // Licensed under the MIT License. import { describe, it, assert } from "vitest"; -import { createDefaultPipeline } from "../../src/clientHelpers.js"; +import { createDefaultPipeline, getCachedDefaultHttpsClient } from "../../src/clientHelpers.js"; import { bearerTokenAuthenticationPolicyName } from "@azure/core-rest-pipeline"; import { keyCredentialAuthenticationPolicyName } from "../../src/keyCredentialAuthenticationPolicy.js"; import type { TokenCredential } from "@azure/core-auth"; @@ -88,4 +88,18 @@ describe("clientHelpers", () => { "pipeline shouldn have keyCredentialAuthenticationPolicyName", ); }); + + describe("getCachedDefaultHttpsClient", () => { + it("should return an HttpClient", () => { + const client = getCachedDefaultHttpsClient(); + assert.isDefined(client); + assert.isFunction(client.sendRequest); + }); + + it("should return the same instance on subsequent calls", () => { + const client1 = getCachedDefaultHttpsClient(); + const client2 = getCachedDefaultHttpsClient(); + assert.strictEqual(client1, client2, "should return cached instance"); + }); + }); }); diff --git a/sdk/core/core-client-rest/test/internal/getClient.spec.ts b/sdk/core/core-client-rest/test/internal/getClient.spec.ts index fe68cc1e472b..a28597473d88 100644 --- a/sdk/core/core-client-rest/test/internal/getClient.spec.ts +++ b/sdk/core/core-client-rest/test/internal/getClient.spec.ts @@ -293,6 +293,120 @@ describe("getClient", () => { .get(); }); + describe("HTTP methods", () => { + it("should support post method", async () => { + let policyExecuted = false; + const client = getClient("https://example.org", { httpClient }); + const validationPolicy: PipelinePolicy = { + name: "validationPolicy", + sendRequest: (req, next) => { + policyExecuted = true; + assert.equal(req.method, "POST"); + return next(req); + }, + }; + client.pipeline.addPolicy(validationPolicy, { afterPhase: "Serialize" }); + await client.pathUnchecked("/foo").post(); + assert.isTrue(policyExecuted, "Validation policy should have executed"); + }); + + it("should support put method", async () => { + let policyExecuted = false; + const client = getClient("https://example.org", { httpClient }); + const validationPolicy: PipelinePolicy = { + name: "validationPolicy", + sendRequest: (req, next) => { + policyExecuted = true; + assert.equal(req.method, "PUT"); + return next(req); + }, + }; + client.pipeline.addPolicy(validationPolicy, { afterPhase: "Serialize" }); + await client.pathUnchecked("/foo").put(); + assert.isTrue(policyExecuted, "Validation policy should have executed"); + }); + + it("should support patch method", async () => { + let policyExecuted = false; + const client = getClient("https://example.org", { httpClient }); + const validationPolicy: PipelinePolicy = { + name: "validationPolicy", + sendRequest: (req, next) => { + policyExecuted = true; + assert.equal(req.method, "PATCH"); + return next(req); + }, + }; + client.pipeline.addPolicy(validationPolicy, { afterPhase: "Serialize" }); + await client.pathUnchecked("/foo").patch(); + assert.isTrue(policyExecuted, "Validation policy should have executed"); + }); + + it("should support delete method", async () => { + let policyExecuted = false; + const client = getClient("https://example.org", { httpClient }); + const validationPolicy: PipelinePolicy = { + name: "validationPolicy", + sendRequest: (req, next) => { + policyExecuted = true; + assert.equal(req.method, "DELETE"); + return next(req); + }, + }; + client.pipeline.addPolicy(validationPolicy, { afterPhase: "Serialize" }); + await client.pathUnchecked("/foo").delete(); + assert.isTrue(policyExecuted, "Validation policy should have executed"); + }); + + it("should support head method", async () => { + let policyExecuted = false; + const client = getClient("https://example.org", { httpClient }); + const validationPolicy: PipelinePolicy = { + name: "validationPolicy", + sendRequest: (req, next) => { + policyExecuted = true; + assert.equal(req.method, "HEAD"); + return next(req); + }, + }; + client.pipeline.addPolicy(validationPolicy, { afterPhase: "Serialize" }); + await client.pathUnchecked("/foo").head(); + assert.isTrue(policyExecuted, "Validation policy should have executed"); + }); + + it("should support options method", async () => { + let policyExecuted = false; + const client = getClient("https://example.org", { httpClient }); + const validationPolicy: PipelinePolicy = { + name: "validationPolicy", + sendRequest: (req, next) => { + policyExecuted = true; + assert.equal(req.method, "OPTIONS"); + return next(req); + }, + }; + client.pipeline.addPolicy(validationPolicy, { afterPhase: "Serialize" }); + await client.pathUnchecked("/foo").options(); + assert.isTrue(policyExecuted, "Validation policy should have executed"); + }); + + it("should support trace method", async () => { + let policyExecuted = false; + const client = getClient("https://example.org", { httpClient }); + const validationPolicy: PipelinePolicy = { + name: "validationPolicy", + sendRequest: (req, next) => { + policyExecuted = true; + assert.equal(req.method, "TRACE"); + return next(req); + }, + }; + client.pipeline.addPolicy(validationPolicy, { afterPhase: "Serialize" }); + await client.pathUnchecked("/foo").trace(); + assert.isTrue(policyExecuted, "Validation policy should have executed"); + }); + }); + describe("when pipeline is passed via options", () => { it("should use the provided pipeline when passed via second parameter (options only)", async () => { let customPolicyInvoked = false; diff --git a/sdk/core/core-client-rest/test/internal/keyCredentialAuthenticationPolicy.spec.ts b/sdk/core/core-client-rest/test/internal/keyCredentialAuthenticationPolicy.spec.ts new file mode 100644 index 000000000000..93ec92a1bae1 --- /dev/null +++ b/sdk/core/core-client-rest/test/internal/keyCredentialAuthenticationPolicy.spec.ts @@ -0,0 +1,31 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +import { describe, it, assert } from "vitest"; +import { + keyCredentialAuthenticationPolicy, + keyCredentialAuthenticationPolicyName, +} from "../../src/keyCredentialAuthenticationPolicy.js"; +import { createHttpHeaders, createPipelineRequest } from "@azure/core-rest-pipeline"; + +describe("keyCredentialAuthenticationPolicy", () => { + it("should set the api key header on the request", async () => { + const credential = { key: "test-api-key" }; + const headerName = "x-api-key"; + const policy = keyCredentialAuthenticationPolicy(credential, headerName); + + assert.equal(policy.name, keyCredentialAuthenticationPolicyName); + + const request = createPipelineRequest({ + url: "https://example.org", + headers: createHttpHeaders(), + }); + + const response = await policy.sendRequest(request, async (req) => { + assert.equal(req.headers.get(headerName), "test-api-key"); + return { headers: createHttpHeaders(), status: 200, request: req }; + }); + + assert.equal(response.status, 200); + }); +}); diff --git a/sdk/core/core-client-rest/test/internal/operationOptionHelpers.spec.ts b/sdk/core/core-client-rest/test/internal/operationOptionHelpers.spec.ts new file mode 100644 index 000000000000..cb57f1ed076a --- /dev/null +++ b/sdk/core/core-client-rest/test/internal/operationOptionHelpers.spec.ts @@ -0,0 +1,23 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +import { describe, it, assert } from "vitest"; +import { operationOptionsToRequestParameters } from "../../src/operationOptionHelpers.js"; + +describe("operationOptionsToRequestParameters", () => { + it("should convert empty operation options to request parameters", () => { + const result = operationOptionsToRequestParameters({}); + assert.isDefined(result); + assert.isUndefined(result.abortSignal); + assert.isUndefined(result.onResponse); + assert.isObject(result); + }); + + it("should pass through abort signal", () => { + const abortController = new AbortController(); + const result = operationOptionsToRequestParameters({ + abortSignal: abortController.signal, + }); + assert.equal(result.abortSignal, abortController.signal); + }); +}); From 8dd12fa6b3acec091fba08e606ab78105d00b992 Mon Sep 17 00:00:00 2001 From: Deyaaeldeen Almahallawi Date: Fri, 17 Apr 2026 01:15:38 +0000 Subject: [PATCH 02/17] Strengthen isDefined-only assertions with value checks Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../test/internal/clientHelpers.spec.ts | 27 +++++++++---------- 1 file changed, 12 insertions(+), 15 deletions(-) diff --git a/sdk/core/core-client-rest/test/internal/clientHelpers.spec.ts b/sdk/core/core-client-rest/test/internal/clientHelpers.spec.ts index 76cb26e178f2..d0e9bcab65a1 100644 --- a/sdk/core/core-client-rest/test/internal/clientHelpers.spec.ts +++ b/sdk/core/core-client-rest/test/internal/clientHelpers.spec.ts @@ -31,12 +31,11 @@ describe("clientHelpers", () => { const pipeline = createDefaultPipeline(mockBaseUrl); const policies = pipeline.getOrderedPolicies(); - assert.isDefined(policies, "default pipeline should contain policies"); + assert.isNotEmpty(policies, "default pipeline should contain policies"); - assert.isDefined( - policies.find((p) => p.name === apiVersionPolicyName), - `Pipeline policy not found in the default pipeline: ${apiVersionPolicyName}`, - ); + const apiVersionPolicy = policies.find((p) => p.name === apiVersionPolicyName); + assert.isDefined(apiVersionPolicy, `Pipeline policy not found in the default pipeline: ${apiVersionPolicyName}`); + assert.strictEqual(apiVersionPolicy!.name, apiVersionPolicyName); }); it("should throw if key credentials but no Api Header Name", () => { @@ -56,17 +55,16 @@ describe("clientHelpers", () => { ); const policies = pipeline.getOrderedPolicies(); - assert.isDefined(policies, "default pipeline should contain policies"); + assert.isNotEmpty(policies, "default pipeline should contain policies"); assert.isUndefined( policies.find((p) => p.name === bearerTokenAuthenticationPolicyName), "pipeline shouldn't have bearerTokenAuthenticationPolicyName", ); - assert.isDefined( - policies.find((p) => p.name === keyCredentialAuthenticationPolicyName), - "pipeline shouldn have keyCredentialAuthenticationPolicyName", - ); + const keyCredPolicy = policies.find((p) => p.name === keyCredentialAuthenticationPolicyName); + assert.isDefined(keyCredPolicy, "pipeline should have keyCredentialAuthenticationPolicyName"); + assert.strictEqual(keyCredPolicy!.name, keyCredentialAuthenticationPolicyName); }); it("should create a default pipeline with TokenCredential", () => { @@ -76,12 +74,11 @@ describe("clientHelpers", () => { const pipeline = createDefaultPipeline(mockBaseUrl, mockCredential); const policies = pipeline.getOrderedPolicies(); - assert.isDefined(policies, "default pipeline should contain policies"); + assert.isNotEmpty(policies, "default pipeline should contain policies"); - assert.isDefined( - policies.find((p) => p.name === bearerTokenAuthenticationPolicyName), - "pipeline should have bearerTokenAuthenticationPolicyName", - ); + const bearerPolicy = policies.find((p) => p.name === bearerTokenAuthenticationPolicyName); + assert.isDefined(bearerPolicy, "pipeline should have bearerTokenAuthenticationPolicyName"); + assert.strictEqual(bearerPolicy!.name, bearerTokenAuthenticationPolicyName); assert.isUndefined( policies.find((p) => p.name === keyCredentialAuthenticationPolicyName), From 073e24547db0f4fc8fc671c4a4fa804738ea9846 Mon Sep 17 00:00:00 2001 From: Deyaaeldeen Almahallawi Date: Fri, 17 Apr 2026 16:55:35 +0000 Subject: [PATCH 03/17] Format clientHelpers.spec.ts Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../core-client-rest/test/internal/clientHelpers.spec.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/sdk/core/core-client-rest/test/internal/clientHelpers.spec.ts b/sdk/core/core-client-rest/test/internal/clientHelpers.spec.ts index d0e9bcab65a1..440026a6121e 100644 --- a/sdk/core/core-client-rest/test/internal/clientHelpers.spec.ts +++ b/sdk/core/core-client-rest/test/internal/clientHelpers.spec.ts @@ -34,7 +34,10 @@ describe("clientHelpers", () => { assert.isNotEmpty(policies, "default pipeline should contain policies"); const apiVersionPolicy = policies.find((p) => p.name === apiVersionPolicyName); - assert.isDefined(apiVersionPolicy, `Pipeline policy not found in the default pipeline: ${apiVersionPolicyName}`); + assert.isDefined( + apiVersionPolicy, + `Pipeline policy not found in the default pipeline: ${apiVersionPolicyName}`, + ); assert.strictEqual(apiVersionPolicy!.name, apiVersionPolicyName); }); From 5f6a356ce2bf55bc051555ed4f2990fc9c53ff41 Mon Sep 17 00:00:00 2001 From: Deyaaeldeen Almahallawi Date: Fri, 17 Apr 2026 21:35:24 +0000 Subject: [PATCH 04/17] Improve operationOptionsToRequestParameters test to exercise requestOptions promotion Replace trivial empty-object test with one that passes requestOptions fields (timeout, headers, skipUrlEncoding, etc.) and asserts they get promoted to root-level RequestParameters. Also test onResponse passthrough. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../internal/operationOptionHelpers.spec.ts | 30 ++++++++++++++----- 1 file changed, 23 insertions(+), 7 deletions(-) diff --git a/sdk/core/core-client-rest/test/internal/operationOptionHelpers.spec.ts b/sdk/core/core-client-rest/test/internal/operationOptionHelpers.spec.ts index cb57f1ed076a..5b5787d0a99a 100644 --- a/sdk/core/core-client-rest/test/internal/operationOptionHelpers.spec.ts +++ b/sdk/core/core-client-rest/test/internal/operationOptionHelpers.spec.ts @@ -5,19 +5,35 @@ import { describe, it, assert } from "vitest"; import { operationOptionsToRequestParameters } from "../../src/operationOptionHelpers.js"; describe("operationOptionsToRequestParameters", () => { - it("should convert empty operation options to request parameters", () => { - const result = operationOptionsToRequestParameters({}); - assert.isDefined(result); - assert.isUndefined(result.abortSignal); - assert.isUndefined(result.onResponse); - assert.isObject(result); + it("promotes requestOptions fields to root-level request parameters", () => { + const onUpload = () => {}; + const onDownload = () => {}; + const result = operationOptionsToRequestParameters({ + requestOptions: { + allowInsecureConnection: true, + timeout: 5000, + skipUrlEncoding: true, + onUploadProgress: onUpload, + onDownloadProgress: onDownload, + headers: { "x-custom": "value" }, + }, + }); + assert.equal(result.allowInsecureConnection, true); + assert.equal(result.timeout, 5000); + assert.equal(result.skipUrlEncoding, true); + assert.equal(result.onUploadProgress, onUpload); + assert.equal(result.onDownloadProgress, onDownload); + assert.deepEqual(result.headers, { "x-custom": "value" }); }); - it("should pass through abort signal", () => { + it("passes through abortSignal and onResponse", () => { const abortController = new AbortController(); + const onResponse = () => {}; const result = operationOptionsToRequestParameters({ abortSignal: abortController.signal, + onResponse, }); assert.equal(result.abortSignal, abortController.signal); + assert.equal(result.onResponse, onResponse); }); }); From d3eaa17a7905f3b058c406337800f6db9b8e152e Mon Sep 17 00:00:00 2001 From: Deyaaeldeen Almahallawi Date: Fri, 17 Apr 2026 23:38:56 +0000 Subject: [PATCH 05/17] test(core-client-rest): remove redundant strictEqual assertions after find() The find() callback already matches by policy name, so asserting strictEqual on .name after isDefined is redundant. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- sdk/core/core-client-rest/test/internal/clientHelpers.spec.ts | 3 --- 1 file changed, 3 deletions(-) diff --git a/sdk/core/core-client-rest/test/internal/clientHelpers.spec.ts b/sdk/core/core-client-rest/test/internal/clientHelpers.spec.ts index 440026a6121e..814d90a3a833 100644 --- a/sdk/core/core-client-rest/test/internal/clientHelpers.spec.ts +++ b/sdk/core/core-client-rest/test/internal/clientHelpers.spec.ts @@ -38,7 +38,6 @@ describe("clientHelpers", () => { apiVersionPolicy, `Pipeline policy not found in the default pipeline: ${apiVersionPolicyName}`, ); - assert.strictEqual(apiVersionPolicy!.name, apiVersionPolicyName); }); it("should throw if key credentials but no Api Header Name", () => { @@ -67,7 +66,6 @@ describe("clientHelpers", () => { const keyCredPolicy = policies.find((p) => p.name === keyCredentialAuthenticationPolicyName); assert.isDefined(keyCredPolicy, "pipeline should have keyCredentialAuthenticationPolicyName"); - assert.strictEqual(keyCredPolicy!.name, keyCredentialAuthenticationPolicyName); }); it("should create a default pipeline with TokenCredential", () => { @@ -81,7 +79,6 @@ describe("clientHelpers", () => { const bearerPolicy = policies.find((p) => p.name === bearerTokenAuthenticationPolicyName); assert.isDefined(bearerPolicy, "pipeline should have bearerTokenAuthenticationPolicyName"); - assert.strictEqual(bearerPolicy!.name, bearerTokenAuthenticationPolicyName); assert.isUndefined( policies.find((p) => p.name === keyCredentialAuthenticationPolicyName), From 008fdce422162b462774cbdf5d270878c8cfa57a Mon Sep 17 00:00:00 2001 From: Deyaaeldeen Almahallawi Date: Sat, 18 Apr 2026 00:41:32 +0000 Subject: [PATCH 06/17] fix: remove getCachedDefaultHttpsClient test (function removed in main) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../test/internal/clientHelpers.spec.ts | 16 +--------------- 1 file changed, 1 insertion(+), 15 deletions(-) diff --git a/sdk/core/core-client-rest/test/internal/clientHelpers.spec.ts b/sdk/core/core-client-rest/test/internal/clientHelpers.spec.ts index 814d90a3a833..19a18e57231c 100644 --- a/sdk/core/core-client-rest/test/internal/clientHelpers.spec.ts +++ b/sdk/core/core-client-rest/test/internal/clientHelpers.spec.ts @@ -2,7 +2,7 @@ // Licensed under the MIT License. import { describe, it, assert } from "vitest"; -import { createDefaultPipeline, getCachedDefaultHttpsClient } from "../../src/clientHelpers.js"; +import { createDefaultPipeline } from "../../src/clientHelpers.js"; import { bearerTokenAuthenticationPolicyName } from "@azure/core-rest-pipeline"; import { keyCredentialAuthenticationPolicyName } from "../../src/keyCredentialAuthenticationPolicy.js"; import type { TokenCredential } from "@azure/core-auth"; @@ -85,18 +85,4 @@ describe("clientHelpers", () => { "pipeline shouldn have keyCredentialAuthenticationPolicyName", ); }); - - describe("getCachedDefaultHttpsClient", () => { - it("should return an HttpClient", () => { - const client = getCachedDefaultHttpsClient(); - assert.isDefined(client); - assert.isFunction(client.sendRequest); - }); - - it("should return the same instance on subsequent calls", () => { - const client1 = getCachedDefaultHttpsClient(); - const client2 = getCachedDefaultHttpsClient(); - assert.strictEqual(client1, client2, "should return cached instance"); - }); - }); }); From 07cec1370337ea09f5bb9b0a3ddbadf6e4f66e65 Mon Sep 17 00:00:00 2001 From: Deyaaeldeen Almahallawi Date: Sat, 18 Apr 2026 02:52:35 +0000 Subject: [PATCH 07/17] fix: fix misleading test name and weak assertions in getClient tests Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../core-client-rest/test/internal/getClient.spec.ts | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/sdk/core/core-client-rest/test/internal/getClient.spec.ts b/sdk/core/core-client-rest/test/internal/getClient.spec.ts index a28597473d88..c28778468dc7 100644 --- a/sdk/core/core-client-rest/test/internal/getClient.spec.ts +++ b/sdk/core/core-client-rest/test/internal/getClient.spec.ts @@ -103,7 +103,7 @@ describe("getClient", () => { }); }); - it("should encode url when not skip query parameter encoding and api version parameter exists", async () => { + it("should preserve comma in query parameter when encoding is enabled and api version parameter exists", async () => { const apiVersion = "2021-11-18"; const client = getClient("https://example.org", { apiVersion, httpClient }); const validationPolicy: PipelinePolicy = { @@ -163,8 +163,11 @@ describe("getClient", () => { client.pipeline.addPolicy(retryPolicy, { phase: "Retry" }); assert(client); const policies = client.pipeline.getOrderedPolicies(); - assert.isTrue(policies.indexOf(policy2) < policies.indexOf(retryPolicy)); - assert.isTrue(policies.indexOf(retryPolicy) < policies.indexOf(policy1)); + const policy2Index = policies.indexOf(policy2); + const retryPolicyIndex = policies.indexOf(retryPolicy); + const policy1Index = policies.indexOf(policy1); + assert.isBelow(policy2Index, retryPolicyIndex); + assert.isBelow(retryPolicyIndex, policy1Index); }); it("should use the client setting for `allowInsecureConnection` when the request setting is undefined", async () => { From 643d0bf2587b86e6a3ced1c0e710973699acb38a Mon Sep 17 00:00:00 2001 From: Deyaaeldeen Almahallawi Date: Sat, 18 Apr 2026 07:44:16 +0000 Subject: [PATCH 08/17] fix: strengthen stream error tests and fix misleading test name MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - browser/node streams: add assert.fail after await to catch missing errors - browser streams: assert.equal(done, false) → assert.isFalse(done) - createRestError: fix misleading 'standard error' test name Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../core-client-rest/test/internal/browser/streams.spec.ts | 4 +++- .../core-client-rest/test/internal/createRestError.spec.ts | 2 +- sdk/core/core-client-rest/test/internal/node/streams.spec.ts | 2 ++ 3 files changed, 6 insertions(+), 2 deletions(-) diff --git a/sdk/core/core-client-rest/test/internal/browser/streams.spec.ts b/sdk/core/core-client-rest/test/internal/browser/streams.spec.ts index 4d8182fe4966..034b98b4c975 100644 --- a/sdk/core/core-client-rest/test/internal/browser/streams.spec.ts +++ b/sdk/core/core-client-rest/test/internal/browser/streams.spec.ts @@ -50,7 +50,7 @@ describe("[Browser] Streams", () => { const reader = result.body!.getReader(); // Read the first chunk const chunk = await reader.read(); - assert.equal(chunk.done, false); + assert.isFalse(chunk.done); expect(fetchMock).toHaveBeenCalledOnce(); }); @@ -75,6 +75,7 @@ describe("[Browser] Streams", () => { try { await client.pathUnchecked("/foo").get(); + assert.fail("Expected an error to be thrown"); } catch (e: any) { assert.match(e.message, /ExpectedException/); } @@ -88,6 +89,7 @@ describe("[Browser] Streams", () => { try { await client.pathUnchecked("/foo").get().asBrowserStream(); + assert.fail("Expected an error to be thrown"); } catch (e: any) { assert.match(e.message, /ExpectedException/); } diff --git a/sdk/core/core-client-rest/test/internal/createRestError.spec.ts b/sdk/core/core-client-rest/test/internal/createRestError.spec.ts index 6e570c873b25..3bea6fd3ee86 100644 --- a/sdk/core/core-client-rest/test/internal/createRestError.spec.ts +++ b/sdk/core/core-client-rest/test/internal/createRestError.spec.ts @@ -58,7 +58,7 @@ describe("createRestError", () => { assert.equal(error.message, "message"); }); - it("should create a rest error from an error message and a PathUnchecked response with standard error", () => { + it("should create a rest error from an error message and a PathUnchecked response with top-level error properties", () => { const response = { status: "400", headers: {}, diff --git a/sdk/core/core-client-rest/test/internal/node/streams.spec.ts b/sdk/core/core-client-rest/test/internal/node/streams.spec.ts index 1f4612af9d1f..b44305458be3 100644 --- a/sdk/core/core-client-rest/test/internal/node/streams.spec.ts +++ b/sdk/core/core-client-rest/test/internal/node/streams.spec.ts @@ -75,6 +75,7 @@ describe("[Node] Streams", () => { try { await client.pathUnchecked("/foo").get(); + assert.fail("Expected an error to be thrown"); } catch (e: any) { assert.equal(e.message, "ExpectedException"); } @@ -90,6 +91,7 @@ describe("[Node] Streams", () => { try { await client.pathUnchecked("/foo").get().asNodeStream(); + assert.fail("Expected an error to be thrown"); } catch (e: any) { assert.equal(e.message, "ExpectedException"); } From 03c8634f2cba103e5f30835bb3d3d3786b96db9d Mon Sep 17 00:00:00 2001 From: Deyaaeldeen Almahallawi Date: Sat, 18 Apr 2026 16:06:52 +0000 Subject: [PATCH 09/17] fix: strengthen assertions in core-client-rest tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - operationOptionHelpers: equal(x, true) → isTrue - clientHelpers: isDefined(policies) → isAbove(policies.length, 0) - getClient: assert(client) → assert.isDefined(client) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- sdk/core/core-client-rest/test/internal/clientHelpers.spec.ts | 2 +- sdk/core/core-client-rest/test/internal/getClient.spec.ts | 2 +- .../test/internal/operationOptionHelpers.spec.ts | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/sdk/core/core-client-rest/test/internal/clientHelpers.spec.ts b/sdk/core/core-client-rest/test/internal/clientHelpers.spec.ts index 19a18e57231c..3e220500cfc6 100644 --- a/sdk/core/core-client-rest/test/internal/clientHelpers.spec.ts +++ b/sdk/core/core-client-rest/test/internal/clientHelpers.spec.ts @@ -14,7 +14,7 @@ describe("clientHelpers", () => { const pipeline = createDefaultPipeline(mockBaseUrl); const policies = pipeline.getOrderedPolicies(); - assert.isDefined(policies, "default pipeline should contain policies"); + assert.isAbove(policies.length, 0, "default pipeline should contain policies"); assert.isUndefined( policies.find((p) => p.name === bearerTokenAuthenticationPolicyName), diff --git a/sdk/core/core-client-rest/test/internal/getClient.spec.ts b/sdk/core/core-client-rest/test/internal/getClient.spec.ts index c28778468dc7..8859edfaf182 100644 --- a/sdk/core/core-client-rest/test/internal/getClient.spec.ts +++ b/sdk/core/core-client-rest/test/internal/getClient.spec.ts @@ -161,7 +161,7 @@ describe("getClient", () => { ], }); client.pipeline.addPolicy(retryPolicy, { phase: "Retry" }); - assert(client); + assert.isDefined(client); const policies = client.pipeline.getOrderedPolicies(); const policy2Index = policies.indexOf(policy2); const retryPolicyIndex = policies.indexOf(retryPolicy); diff --git a/sdk/core/core-client-rest/test/internal/operationOptionHelpers.spec.ts b/sdk/core/core-client-rest/test/internal/operationOptionHelpers.spec.ts index 5b5787d0a99a..d22a2edd7a51 100644 --- a/sdk/core/core-client-rest/test/internal/operationOptionHelpers.spec.ts +++ b/sdk/core/core-client-rest/test/internal/operationOptionHelpers.spec.ts @@ -18,9 +18,9 @@ describe("operationOptionsToRequestParameters", () => { headers: { "x-custom": "value" }, }, }); - assert.equal(result.allowInsecureConnection, true); + assert.isTrue(result.allowInsecureConnection); assert.equal(result.timeout, 5000); - assert.equal(result.skipUrlEncoding, true); + assert.isTrue(result.skipUrlEncoding); assert.equal(result.onUploadProgress, onUpload); assert.equal(result.onDownloadProgress, onDownload); assert.deepEqual(result.headers, { "x-custom": "value" }); From 4219a8f524e90c4bc7c9a55ec06fd84a2edbb70e Mon Sep 17 00:00:00 2001 From: Copilot <223556219+Copilot@users.noreply.github.com> Date: Sun, 19 Apr 2026 18:06:52 +0000 Subject: [PATCH 10/17] refactor: modernize error assertions and spy patterns in core-client-rest tests Replace try/catch + assert.fail() with rejects.toThrow(), hand-rolled boolean flags with vi.fn(), and assert.equal(x, undefined) with assert.isUndefined(x). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../test/internal/browser/streams.spec.ts | 12 +--- .../test/internal/clientHelpers.spec.ts | 11 ++-- .../test/internal/createRestError.spec.ts | 4 +- .../test/internal/getClient.spec.ts | 59 ++++++------------- .../test/internal/node/streams.spec.ts | 14 ++--- 5 files changed, 32 insertions(+), 68 deletions(-) diff --git a/sdk/core/core-client-rest/test/internal/browser/streams.spec.ts b/sdk/core/core-client-rest/test/internal/browser/streams.spec.ts index 034b98b4c975..3f976eb8db94 100644 --- a/sdk/core/core-client-rest/test/internal/browser/streams.spec.ts +++ b/sdk/core/core-client-rest/test/internal/browser/streams.spec.ts @@ -101,14 +101,8 @@ describe("[Browser] Streams", () => { const client = getClient(mockBaseUrl); - try { - await client.pathUnchecked("/foo").get().asNodeStream(); - assert.fail("Expected error was not thrown"); - } catch (e: any) { - assert.equal( - e.message, - "`isNodeStream` is not supported in the browser environment. Use `asBrowserStream` to obtain the response body stream.", - ); - } + await expect(client.pathUnchecked("/foo").get().asNodeStream()).rejects.toThrow( + "`isNodeStream` is not supported in the browser environment. Use `asBrowserStream` to obtain the response body stream.", + ); }); }); diff --git a/sdk/core/core-client-rest/test/internal/clientHelpers.spec.ts b/sdk/core/core-client-rest/test/internal/clientHelpers.spec.ts index 3e220500cfc6..1a73d49bbf98 100644 --- a/sdk/core/core-client-rest/test/internal/clientHelpers.spec.ts +++ b/sdk/core/core-client-rest/test/internal/clientHelpers.spec.ts @@ -1,7 +1,7 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. -import { describe, it, assert } from "vitest"; +import { describe, it, assert, expect } from "vitest"; import { createDefaultPipeline } from "../../src/clientHelpers.js"; import { bearerTokenAuthenticationPolicyName } from "@azure/core-rest-pipeline"; import { keyCredentialAuthenticationPolicyName } from "../../src/keyCredentialAuthenticationPolicy.js"; @@ -41,12 +41,9 @@ describe("clientHelpers", () => { }); it("should throw if key credentials but no Api Header Name", () => { - try { - createDefaultPipeline(mockBaseUrl, { key: "mockKey" }); - assert.fail("Expected to throw an error"); - } catch (error: any) { - assert.equal((error as Error).message, "Missing API Key Header Name"); - } + expect(() => createDefaultPipeline(mockBaseUrl, { key: "mockKey" })).toThrow( + "Missing API Key Header Name", + ); }); it("should create a default pipeline with key credentials", () => { diff --git a/sdk/core/core-client-rest/test/internal/createRestError.spec.ts b/sdk/core/core-client-rest/test/internal/createRestError.spec.ts index 3bea6fd3ee86..a12a3064afaf 100644 --- a/sdk/core/core-client-rest/test/internal/createRestError.spec.ts +++ b/sdk/core/core-client-rest/test/internal/createRestError.spec.ts @@ -83,7 +83,7 @@ describe("createRestError", () => { }; const error = createRestError("error message", response); assert.equal(error.statusCode, 400); - assert.equal(error.code, undefined); + assert.isUndefined(error.code); assert.equal(error.message, "error message"); }); @@ -96,7 +96,7 @@ describe("createRestError", () => { }; const error = createRestError(response); assert.equal(error.statusCode, 400); - assert.equal(error.code, undefined); + assert.isUndefined(error.code); assert.equal(error.message, "Unexpected status code: 400"); }); }); diff --git a/sdk/core/core-client-rest/test/internal/getClient.spec.ts b/sdk/core/core-client-rest/test/internal/getClient.spec.ts index 8859edfaf182..7466541b2fdf 100644 --- a/sdk/core/core-client-rest/test/internal/getClient.spec.ts +++ b/sdk/core/core-client-rest/test/internal/getClient.spec.ts @@ -1,7 +1,7 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. -import { describe, it, assert } from "vitest"; +import { describe, it, assert, vi, expect } from "vitest"; import { getClient } from "../../src/getClient.js"; import { isNodeLike } from "@typespec/ts-http-runtime/internal/util"; import type { @@ -207,20 +207,18 @@ describe("getClient", () => { }); it("stream methods should call onResponse", async () => { - let called = false; const fakeHttpClient: HttpClient = { sendRequest: async (request) => { return { headers: createHttpHeaders(), status: 200, request }; }, }; + const onResponseFn = vi.fn(); const client = getClient("https://example.org", { httpClient: fakeHttpClient, }); const res = client.pathUnchecked("/foo").get({ - onResponse: () => { - called = true; - }, + onResponse: onResponseFn, }); if (isNodeLike) { @@ -228,11 +226,10 @@ describe("getClient", () => { } else { await res.asBrowserStream(); } - assert.isTrue(called); + expect(onResponseFn).toHaveBeenCalled(); }); it("onResponse legacyError is passed in", async () => { - let called = false; const fakeHttpClient: HttpClient = { sendRequest: async () => { throw new RestError("error", { @@ -241,21 +238,21 @@ describe("getClient", () => { }, }; + const onResponseFn = vi.fn((_: any, err: any, legacyError: any) => { + assert.isDefined(err); + assert.equal(err, legacyError); + }); const client = getClient("https://example.org", { httpClient: fakeHttpClient, }); try { await client.pathUnchecked("/foo").get({ - onResponse: (_, err, legacyError) => { - assert.isDefined(err); - assert.equal(err, legacyError); - called = true; - }, + onResponse: onResponseFn, }); assert.fail("Expected error to be thrown"); } catch (e: unknown) { - assert.isTrue(called); + expect(onResponseFn).toHaveBeenCalled(); } }); @@ -412,14 +409,11 @@ describe("getClient", () => { describe("when pipeline is passed via options", () => { it("should use the provided pipeline when passed via second parameter (options only)", async () => { - let customPolicyInvoked = false; + const sendRequestFn = vi.fn((req: PipelineRequest, next: SendRequest) => next(req)); const customPipeline = createEmptyPipeline(); const customPolicy: PipelinePolicy = { name: "customTrackingPolicy", - sendRequest: (req, next) => { - customPolicyInvoked = true; - return next(req); - }, + sendRequest: sendRequestFn, }; customPipeline.addPolicy(customPolicy); @@ -429,21 +423,15 @@ describe("getClient", () => { }); await client.pathUnchecked("/foo").get(); - assert.isTrue( - customPolicyInvoked, - "Custom pipeline policy should have been invoked when pipeline passed via second parameter", - ); + expect(sendRequestFn).toHaveBeenCalled(); }); it("should use the provided pipeline when passed via third parameter (with TokenCredential)", async () => { - let customPolicyInvoked = false; + const sendRequestFn = vi.fn((req: PipelineRequest, next: SendRequest) => next(req)); const customPipeline = createEmptyPipeline(); const customPolicy: PipelinePolicy = { name: "customTrackingPolicy", - sendRequest: (req, next) => { - customPolicyInvoked = true; - return next(req); - }, + sendRequest: sendRequestFn, }; customPipeline.addPolicy(customPolicy); @@ -457,21 +445,15 @@ describe("getClient", () => { }); await client.pathUnchecked("/foo").get(); - assert.isTrue( - customPolicyInvoked, - "Custom pipeline policy should have been invoked when pipeline passed via third parameter with TokenCredential", - ); + expect(sendRequestFn).toHaveBeenCalled(); }); it("should use the provided pipeline when passed via third parameter (with KeyCredential)", async () => { - let customPolicyInvoked = false; + const sendRequestFn = vi.fn((req: PipelineRequest, next: SendRequest) => next(req)); const customPipeline = createEmptyPipeline(); const customPolicy: PipelinePolicy = { name: "customTrackingPolicy", - sendRequest: (req, next) => { - customPolicyInvoked = true; - return next(req); - }, + sendRequest: sendRequestFn, }; customPipeline.addPolicy(customPolicy); @@ -485,10 +467,7 @@ describe("getClient", () => { }); await client.pathUnchecked("/foo").get(); - assert.isTrue( - customPolicyInvoked, - "Custom pipeline policy should have been invoked when pipeline passed via third parameter with KeyCredential", - ); + expect(sendRequestFn).toHaveBeenCalled(); }); it("should preserve custom pipeline policies order", async () => { diff --git a/sdk/core/core-client-rest/test/internal/node/streams.spec.ts b/sdk/core/core-client-rest/test/internal/node/streams.spec.ts index b44305458be3..a7fcd282e2fe 100644 --- a/sdk/core/core-client-rest/test/internal/node/streams.spec.ts +++ b/sdk/core/core-client-rest/test/internal/node/streams.spec.ts @@ -1,7 +1,7 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. -import { describe, it, assert, afterEach, vi } from "vitest"; +import { describe, it, assert, expect, afterEach, vi } from "vitest"; import type { ClientRequest, IncomingMessage } from "node:http"; import { type IncomingHttpHeaders } from "node:http"; import { PassThrough } from "node:stream"; @@ -108,15 +108,9 @@ describe("[Node] Streams", () => { const { getClient } = await import("../../../src/getClient.js"); const client = getClient(mockBaseUrl); - try { - await client.pathUnchecked("/foo").get().asBrowserStream(); - assert.fail("Expected error was not thrown"); - } catch (e: any) { - assert.equal( - e.message, - "`asBrowserStream` is supported only in the browser environment. Use `asNodeStream` instead to obtain the response body stream. If you require a Web stream of the response in Node, consider using `Readable.toWeb` on the result of `asNodeStream`.", - ); - } + await expect(client.pathUnchecked("/foo").get().asBrowserStream()).rejects.toThrow( + "`asBrowserStream` is supported only in the browser environment. Use `asNodeStream` instead to obtain the response body stream. If you require a Web stream of the response in Node, consider using `Readable.toWeb` on the result of `asNodeStream`.", + ); }); }); From 3715d87ba887af67d9226e06ac3b3bfd5476790f Mon Sep 17 00:00:00 2001 From: Deyaaeldeen Almahallawi Date: Sun, 19 Apr 2026 20:18:14 +0000 Subject: [PATCH 11/17] refactor: second pass modernization in core-client-rest tests Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../test/internal/browser/streams.spec.ts | 18 +-- .../test/internal/getClient.spec.ts | 141 +++++++++--------- .../test/internal/node/streams.spec.ts | 18 +-- 3 files changed, 84 insertions(+), 93 deletions(-) diff --git a/sdk/core/core-client-rest/test/internal/browser/streams.spec.ts b/sdk/core/core-client-rest/test/internal/browser/streams.spec.ts index 3f976eb8db94..3db150e80826 100644 --- a/sdk/core/core-client-rest/test/internal/browser/streams.spec.ts +++ b/sdk/core/core-client-rest/test/internal/browser/streams.spec.ts @@ -63,7 +63,7 @@ describe("[Browser] Streams", () => { const result = await client.pathUnchecked("/foo").get(); - assert.deepEqual(result.body, responseText); + assert.strictEqual(result.body, responseText); expect(fetchMock).toHaveBeenCalledOnce(); }); @@ -73,12 +73,7 @@ describe("[Browser] Streams", () => { const fetchMock = vi.mocked(fetch); fetchMock.mockRejectedValue(new Error("ExpectedException")); - try { - await client.pathUnchecked("/foo").get(); - assert.fail("Expected an error to be thrown"); - } catch (e: any) { - assert.match(e.message, /ExpectedException/); - } + await expect(client.pathUnchecked("/foo").get()).rejects.toThrow(/ExpectedException/); }); it("should be able to handle errors on streamed response", async () => { @@ -87,12 +82,9 @@ describe("[Browser] Streams", () => { const fetchMock = vi.mocked(fetch); fetchMock.mockRejectedValue(new Error("ExpectedException")); - try { - await client.pathUnchecked("/foo").get().asBrowserStream(); - assert.fail("Expected an error to be thrown"); - } catch (e: any) { - assert.match(e.message, /ExpectedException/); - } + await expect(client.pathUnchecked("/foo").get().asBrowserStream()).rejects.toThrow( + /ExpectedException/, + ); }); it("should throw when attempting to use node streams", async () => { diff --git a/sdk/core/core-client-rest/test/internal/getClient.spec.ts b/sdk/core/core-client-rest/test/internal/getClient.spec.ts index 7466541b2fdf..ba689a5adf2e 100644 --- a/sdk/core/core-client-rest/test/internal/getClient.spec.ts +++ b/sdk/core/core-client-rest/test/internal/getClient.spec.ts @@ -239,21 +239,18 @@ describe("getClient", () => { }; const onResponseFn = vi.fn((_: any, err: any, legacyError: any) => { - assert.isDefined(err); assert.equal(err, legacyError); }); const client = getClient("https://example.org", { httpClient: fakeHttpClient, }); - try { - await client.pathUnchecked("/foo").get({ + await expect( + client.pathUnchecked("/foo").get({ onResponse: onResponseFn, - }); - assert.fail("Expected error to be thrown"); - } catch (e: unknown) { - expect(onResponseFn).toHaveBeenCalled(); - } + }), + ).rejects.toThrow(); + expect(onResponseFn).toHaveBeenCalled(); }); it("should support query parameter with explode set to true", async () => { @@ -295,115 +292,122 @@ describe("getClient", () => { describe("HTTP methods", () => { it("should support post method", async () => { - let policyExecuted = false; + const sendRequestSpy = vi.fn< + (req: PipelineRequest, next: SendRequest) => Promise + >((req, next) => { + assert.equal(req.method, "POST"); + return next(req); + }); const client = getClient("https://example.org", { httpClient }); const validationPolicy: PipelinePolicy = { name: "validationPolicy", - sendRequest: (req, next) => { - policyExecuted = true; - assert.equal(req.method, "POST"); - return next(req); - }, + sendRequest: sendRequestSpy, }; client.pipeline.addPolicy(validationPolicy, { afterPhase: "Serialize" }); await client.pathUnchecked("/foo").post(); - assert.isTrue(policyExecuted, "Validation policy should have executed"); + expect(sendRequestSpy).toHaveBeenCalled(); }); it("should support put method", async () => { - let policyExecuted = false; + const sendRequestSpy = vi.fn< + (req: PipelineRequest, next: SendRequest) => Promise + >((req, next) => { + assert.equal(req.method, "PUT"); + return next(req); + }); const client = getClient("https://example.org", { httpClient }); const validationPolicy: PipelinePolicy = { name: "validationPolicy", - sendRequest: (req, next) => { - policyExecuted = true; - assert.equal(req.method, "PUT"); - return next(req); - }, + sendRequest: sendRequestSpy, }; client.pipeline.addPolicy(validationPolicy, { afterPhase: "Serialize" }); await client.pathUnchecked("/foo").put(); - assert.isTrue(policyExecuted, "Validation policy should have executed"); + expect(sendRequestSpy).toHaveBeenCalled(); }); it("should support patch method", async () => { - let policyExecuted = false; + const sendRequestSpy = vi.fn< + (req: PipelineRequest, next: SendRequest) => Promise + >((req, next) => { + assert.equal(req.method, "PATCH"); + return next(req); + }); const client = getClient("https://example.org", { httpClient }); const validationPolicy: PipelinePolicy = { name: "validationPolicy", - sendRequest: (req, next) => { - policyExecuted = true; - assert.equal(req.method, "PATCH"); - return next(req); - }, + sendRequest: sendRequestSpy, }; client.pipeline.addPolicy(validationPolicy, { afterPhase: "Serialize" }); await client.pathUnchecked("/foo").patch(); - assert.isTrue(policyExecuted, "Validation policy should have executed"); + expect(sendRequestSpy).toHaveBeenCalled(); }); it("should support delete method", async () => { - let policyExecuted = false; + const sendRequestSpy = vi.fn< + (req: PipelineRequest, next: SendRequest) => Promise + >((req, next) => { + assert.equal(req.method, "DELETE"); + return next(req); + }); const client = getClient("https://example.org", { httpClient }); const validationPolicy: PipelinePolicy = { name: "validationPolicy", - sendRequest: (req, next) => { - policyExecuted = true; - assert.equal(req.method, "DELETE"); - return next(req); - }, + sendRequest: sendRequestSpy, }; client.pipeline.addPolicy(validationPolicy, { afterPhase: "Serialize" }); await client.pathUnchecked("/foo").delete(); - assert.isTrue(policyExecuted, "Validation policy should have executed"); + expect(sendRequestSpy).toHaveBeenCalled(); }); it("should support head method", async () => { - let policyExecuted = false; + const sendRequestSpy = vi.fn< + (req: PipelineRequest, next: SendRequest) => Promise + >((req, next) => { + assert.equal(req.method, "HEAD"); + return next(req); + }); const client = getClient("https://example.org", { httpClient }); const validationPolicy: PipelinePolicy = { name: "validationPolicy", - sendRequest: (req, next) => { - policyExecuted = true; - assert.equal(req.method, "HEAD"); - return next(req); - }, + sendRequest: sendRequestSpy, }; client.pipeline.addPolicy(validationPolicy, { afterPhase: "Serialize" }); await client.pathUnchecked("/foo").head(); - assert.isTrue(policyExecuted, "Validation policy should have executed"); + expect(sendRequestSpy).toHaveBeenCalled(); }); it("should support options method", async () => { - let policyExecuted = false; + const sendRequestSpy = vi.fn< + (req: PipelineRequest, next: SendRequest) => Promise + >((req, next) => { + assert.equal(req.method, "OPTIONS"); + return next(req); + }); const client = getClient("https://example.org", { httpClient }); const validationPolicy: PipelinePolicy = { name: "validationPolicy", - sendRequest: (req, next) => { - policyExecuted = true; - assert.equal(req.method, "OPTIONS"); - return next(req); - }, + sendRequest: sendRequestSpy, }; client.pipeline.addPolicy(validationPolicy, { afterPhase: "Serialize" }); await client.pathUnchecked("/foo").options(); - assert.isTrue(policyExecuted, "Validation policy should have executed"); + expect(sendRequestSpy).toHaveBeenCalled(); }); it("should support trace method", async () => { - let policyExecuted = false; + const sendRequestSpy = vi.fn< + (req: PipelineRequest, next: SendRequest) => Promise + >((req, next) => { + assert.equal(req.method, "TRACE"); + return next(req); + }); const client = getClient("https://example.org", { httpClient }); const validationPolicy: PipelinePolicy = { name: "validationPolicy", - sendRequest: (req, next) => { - policyExecuted = true; - assert.equal(req.method, "TRACE"); - return next(req); - }, + sendRequest: sendRequestSpy, }; client.pipeline.addPolicy(validationPolicy, { afterPhase: "Serialize" }); await client.pathUnchecked("/foo").trace(); - assert.isTrue(policyExecuted, "Validation policy should have executed"); + expect(sendRequestSpy).toHaveBeenCalled(); }); }); @@ -471,22 +475,21 @@ describe("getClient", () => { }); it("should preserve custom pipeline policies order", async () => { - const policiesInvoked: string[] = []; + const firstSpy = vi.fn< + (req: PipelineRequest, next: SendRequest) => Promise + >((req, next) => next(req)); + const secondSpy = vi.fn< + (req: PipelineRequest, next: SendRequest) => Promise + >((req, next) => next(req)); const customPipeline = createEmptyPipeline(); const firstPolicy: PipelinePolicy = { name: "firstPolicy", - sendRequest: (req, next) => { - policiesInvoked.push("first"); - return next(req); - }, + sendRequest: firstSpy, }; const secondPolicy: PipelinePolicy = { name: "secondPolicy", - sendRequest: (req, next) => { - policiesInvoked.push("second"); - return next(req); - }, + sendRequest: secondSpy, }; customPipeline.addPolicy(firstPolicy); @@ -498,7 +501,11 @@ describe("getClient", () => { }); await client.pathUnchecked("/foo").get(); - assert.deepEqual(policiesInvoked, ["first", "second"], "Policies should be invoked in order"); + expect(firstSpy).toHaveBeenCalled(); + expect(secondSpy).toHaveBeenCalled(); + expect(firstSpy.mock.invocationCallOrder[0]).toBeLessThan( + secondSpy.mock.invocationCallOrder[0], + ); }); it("should not add default policies when custom pipeline is provided", async () => { diff --git a/sdk/core/core-client-rest/test/internal/node/streams.spec.ts b/sdk/core/core-client-rest/test/internal/node/streams.spec.ts index a7fcd282e2fe..44e9b66d43f6 100644 --- a/sdk/core/core-client-rest/test/internal/node/streams.spec.ts +++ b/sdk/core/core-client-rest/test/internal/node/streams.spec.ts @@ -43,7 +43,7 @@ describe("[Node] Streams", () => { const response = await promise; const stringBody = await readStreamToBuffer(response.body!); - assert.deepEqual(stringBody.toString(), JSON.stringify(expectedBody)); + assert.strictEqual(stringBody.toString(), JSON.stringify(expectedBody)); }); it("should get a JSON body response", async () => { @@ -73,12 +73,7 @@ describe("[Node] Streams", () => { const { getClient } = await import("../../../src/getClient.js"); const client = getClient(mockBaseUrl); - try { - await client.pathUnchecked("/foo").get(); - assert.fail("Expected an error to be thrown"); - } catch (e: any) { - assert.equal(e.message, "ExpectedException"); - } + await expect(client.pathUnchecked("/foo").get()).rejects.toThrow("ExpectedException"); }); it("should be able to handle errors on streamed response", async () => { @@ -89,12 +84,9 @@ describe("[Node] Streams", () => { const { getClient } = await import("../../../src/getClient.js"); const client = getClient(mockBaseUrl); - try { - await client.pathUnchecked("/foo").get().asNodeStream(); - assert.fail("Expected an error to be thrown"); - } catch (e: any) { - assert.equal(e.message, "ExpectedException"); - } + await expect(client.pathUnchecked("/foo").get().asNodeStream()).rejects.toThrow( + "ExpectedException", + ); }); it("should throw when attempting to use browser streams", async () => { From d0f67de23d5ac15edcc1f404970e76904c745a82 Mon Sep 17 00:00:00 2001 From: Deyaaeldeen Almahallawi Date: Mon, 20 Apr 2026 16:07:18 +0000 Subject: [PATCH 12/17] refactor: replace PipelineRequest casts with createPipelineRequest in tests Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../test/internal/createRestError.spec.ts | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/sdk/core/core-client-rest/test/internal/createRestError.spec.ts b/sdk/core/core-client-rest/test/internal/createRestError.spec.ts index a12a3064afaf..3f3cf00175e4 100644 --- a/sdk/core/core-client-rest/test/internal/createRestError.spec.ts +++ b/sdk/core/core-client-rest/test/internal/createRestError.spec.ts @@ -2,7 +2,7 @@ // Licensed under the MIT License. import { createRestError } from "../../src/restError.js"; -import type { PipelineRequest } from "@azure/core-rest-pipeline"; +import { createPipelineRequest } from "@azure/core-rest-pipeline"; import { describe, it, assert } from "vitest"; describe("createRestError", () => { @@ -10,7 +10,7 @@ describe("createRestError", () => { const response = { status: "400", headers: {}, - request: {} as PipelineRequest, + request: createPipelineRequest({ url: "https://example.com" }), body: { error: { code: "code", @@ -28,7 +28,7 @@ describe("createRestError", () => { const response = { status: "400", headers: {}, - request: {} as PipelineRequest, + request: createPipelineRequest({ url: "https://example.com" }), body: { error: { code: "code", @@ -46,7 +46,7 @@ describe("createRestError", () => { const response = { status: "400", headers: {}, - request: {} as PipelineRequest, + request: createPipelineRequest({ url: "https://example.com" }), body: { code: "code", message: "message", @@ -62,7 +62,7 @@ describe("createRestError", () => { const response = { status: "400", headers: {}, - request: {} as PipelineRequest, + request: createPipelineRequest({ url: "https://example.com" }), body: { code: "code", message: "message", @@ -78,7 +78,7 @@ describe("createRestError", () => { const response = { status: "400", headers: {}, - request: {} as PipelineRequest, + request: createPipelineRequest({ url: "https://example.com" }), body: undefined, }; const error = createRestError("error message", response); @@ -91,7 +91,7 @@ describe("createRestError", () => { const response = { status: "400", headers: {}, - request: {} as PipelineRequest, + request: createPipelineRequest({ url: "https://example.com" }), body: undefined, }; const error = createRestError(response); From e9f044ece2beb157e954dc5cd06a35a04e8c03bd Mon Sep 17 00:00:00 2001 From: Deyaaeldeen Almahallawi Date: Mon, 20 Apr 2026 16:34:59 +0000 Subject: [PATCH 13/17] Eliminate unnecessary type casts in core-client-rest tests Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../test/internal/getClient.spec.ts | 12 +++++--- .../test/internal/node/streams.spec.ts | 28 ++++++++----------- 2 files changed, 20 insertions(+), 20 deletions(-) diff --git a/sdk/core/core-client-rest/test/internal/getClient.spec.ts b/sdk/core/core-client-rest/test/internal/getClient.spec.ts index ba689a5adf2e..d21a1fab54cd 100644 --- a/sdk/core/core-client-rest/test/internal/getClient.spec.ts +++ b/sdk/core/core-client-rest/test/internal/getClient.spec.ts @@ -11,17 +11,17 @@ import type { PipelineResponse, SendRequest, } from "@azure/core-rest-pipeline"; -import { createEmptyPipeline, createHttpHeaders, RestError } from "@azure/core-rest-pipeline"; +import { createEmptyPipeline, createHttpHeaders, createPipelineRequest, RestError } from "@azure/core-rest-pipeline"; import type { KeyCredential, TokenCredential } from "@azure/core-auth"; describe("getClient", () => { - const httpClient = { + const httpClient: HttpClient = { sendRequest: (req: PipelineRequest) => { return Promise.resolve({ headers: createHttpHeaders(), status: 200, request: req, - }) as Promise; + }); }, }; @@ -233,7 +233,11 @@ describe("getClient", () => { const fakeHttpClient: HttpClient = { sendRequest: async () => { throw new RestError("error", { - response: { status: 404, headers: createHttpHeaders({}) } as PipelineResponse, + response: { + status: 404, + headers: createHttpHeaders({}), + request: createPipelineRequest({ url: "https://example.org/foo" }), + }, }); }, }; diff --git a/sdk/core/core-client-rest/test/internal/node/streams.spec.ts b/sdk/core/core-client-rest/test/internal/node/streams.spec.ts index 44e9b66d43f6..c2808f0bc74e 100644 --- a/sdk/core/core-client-rest/test/internal/node/streams.spec.ts +++ b/sdk/core/core-client-rest/test/internal/node/streams.spec.ts @@ -4,13 +4,14 @@ import { describe, it, assert, expect, afterEach, vi } from "vitest"; import type { ClientRequest, IncomingMessage } from "node:http"; import { type IncomingHttpHeaders } from "node:http"; +import { EventEmitter } from "node:events"; import { PassThrough } from "node:stream"; vi.mock("node:https", async () => { - const actual = await vi.importActual("node:https"); + const actual = await vi.importActual("node:https"); return { default: { - ...(actual as any).default, + ...actual.default, request: vi.fn(), }, }; @@ -26,10 +27,9 @@ describe("[Node] Streams", () => { }); it("should get a JSON body response as a stream", async () => { - vi.mocked(https.request).mockImplementation((_url, cb) => { + vi.mocked(https.request).mockImplementation((_url, cb?: (res: IncomingMessage) => void) => { const response = createResponse(200, JSON.stringify({ foo: "foo" })); - const callback = cb as (res: IncomingMessage) => void; - callback(response); + cb?.(response); return createRequest(); }); @@ -47,10 +47,9 @@ describe("[Node] Streams", () => { }); it("should get a JSON body response", async () => { - vi.mocked(https.request).mockImplementation((_url, cb) => { + vi.mocked(https.request).mockImplementation((_url, cb?: (res: IncomingMessage) => void) => { const response = createResponse(200, JSON.stringify({ foo: "foo" })); - const callback = cb as (res: IncomingMessage) => void; - callback(response); + cb?.(response); return createRequest(); }); @@ -90,10 +89,9 @@ describe("[Node] Streams", () => { }); it("should throw when attempting to use browser streams", async () => { - vi.mocked(https.request).mockImplementation((_url, cb) => { + vi.mocked(https.request).mockImplementation((_url, cb?: (res: IncomingMessage) => void) => { const response = createResponse(200, JSON.stringify({ foo: "foo" })); - const callback = cb as (res: IncomingMessage) => void; - callback(response); + cb?.(response); return createRequest(); }); @@ -106,9 +104,9 @@ describe("[Node] Streams", () => { }); }); -function createRequest(): ClientRequest { - const request = new FakeRequest(); - return request as unknown as ClientRequest; +function createRequest(overrides?: Partial): ClientRequest { + const emitter = new EventEmitter(); + return Object.assign(emitter, { end: vi.fn(), ...overrides }) as ClientRequest; } class FakeResponse extends PassThrough { @@ -136,5 +134,3 @@ function readStreamToBuffer(stream: NodeJS.ReadableStream): Promise { }); }); } - -class FakeRequest extends PassThrough {} From beeac1cac87eb92ff63ebee15eb13610fbc0aa1a Mon Sep 17 00:00:00 2001 From: Deyaaeldeen Almahallawi Date: Mon, 20 Apr 2026 17:51:18 +0000 Subject: [PATCH 14/17] fix: resolve TS compilation errors in core-client-rest tests Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../test/internal/node/streams.spec.ts | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/sdk/core/core-client-rest/test/internal/node/streams.spec.ts b/sdk/core/core-client-rest/test/internal/node/streams.spec.ts index c2808f0bc74e..2b643e02599b 100644 --- a/sdk/core/core-client-rest/test/internal/node/streams.spec.ts +++ b/sdk/core/core-client-rest/test/internal/node/streams.spec.ts @@ -8,10 +8,10 @@ import { EventEmitter } from "node:events"; import { PassThrough } from "node:stream"; vi.mock("node:https", async () => { - const actual = await vi.importActual("node:https"); + const actual = await vi.importActual("node:https"); return { default: { - ...actual.default, + ...((actual as Record).default as Record), request: vi.fn(), }, }; @@ -27,9 +27,10 @@ describe("[Node] Streams", () => { }); it("should get a JSON body response as a stream", async () => { - vi.mocked(https.request).mockImplementation((_url, cb?: (res: IncomingMessage) => void) => { + vi.mocked(https.request).mockImplementation((_url, cb) => { const response = createResponse(200, JSON.stringify({ foo: "foo" })); - cb?.(response); + const callback = cb as unknown as (res: IncomingMessage) => void; + callback(response); return createRequest(); }); @@ -47,9 +48,10 @@ describe("[Node] Streams", () => { }); it("should get a JSON body response", async () => { - vi.mocked(https.request).mockImplementation((_url, cb?: (res: IncomingMessage) => void) => { + vi.mocked(https.request).mockImplementation((_url, cb) => { const response = createResponse(200, JSON.stringify({ foo: "foo" })); - cb?.(response); + const callback = cb as unknown as (res: IncomingMessage) => void; + callback(response); return createRequest(); }); @@ -89,9 +91,10 @@ describe("[Node] Streams", () => { }); it("should throw when attempting to use browser streams", async () => { - vi.mocked(https.request).mockImplementation((_url, cb?: (res: IncomingMessage) => void) => { + vi.mocked(https.request).mockImplementation((_url, cb) => { const response = createResponse(200, JSON.stringify({ foo: "foo" })); - cb?.(response); + const callback = cb as unknown as (res: IncomingMessage) => void; + callback(response); return createRequest(); }); From 18ad75e8a2e3dbd0490b7e594fd11ddd0c39792f Mon Sep 17 00:00:00 2001 From: Deyaaeldeen Almahallawi Date: Mon, 20 Apr 2026 18:37:37 +0000 Subject: [PATCH 15/17] fix: format getClient.spec.ts Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- sdk/core/core-client-rest/test/internal/getClient.spec.ts | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/sdk/core/core-client-rest/test/internal/getClient.spec.ts b/sdk/core/core-client-rest/test/internal/getClient.spec.ts index d21a1fab54cd..e88875f6d9f6 100644 --- a/sdk/core/core-client-rest/test/internal/getClient.spec.ts +++ b/sdk/core/core-client-rest/test/internal/getClient.spec.ts @@ -11,7 +11,12 @@ import type { PipelineResponse, SendRequest, } from "@azure/core-rest-pipeline"; -import { createEmptyPipeline, createHttpHeaders, createPipelineRequest, RestError } from "@azure/core-rest-pipeline"; +import { + createEmptyPipeline, + createHttpHeaders, + createPipelineRequest, + RestError, +} from "@azure/core-rest-pipeline"; import type { KeyCredential, TokenCredential } from "@azure/core-auth"; describe("getClient", () => { From d0db843cefc43311c33380fd6acf9e6e736aea37 Mon Sep 17 00:00:00 2001 From: Deyaaeldeen Almahallawi Date: Tue, 21 Apr 2026 18:46:27 +0000 Subject: [PATCH 16/17] Strengthen assertions in getClient onResponse test Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- sdk/core/core-client-rest/test/internal/getClient.spec.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/sdk/core/core-client-rest/test/internal/getClient.spec.ts b/sdk/core/core-client-rest/test/internal/getClient.spec.ts index e88875f6d9f6..27484826dba8 100644 --- a/sdk/core/core-client-rest/test/internal/getClient.spec.ts +++ b/sdk/core/core-client-rest/test/internal/getClient.spec.ts @@ -248,6 +248,7 @@ describe("getClient", () => { }; const onResponseFn = vi.fn((_: any, err: any, legacyError: any) => { + assert.isDefined(err, "err should be defined"); assert.equal(err, legacyError); }); const client = getClient("https://example.org", { @@ -258,7 +259,7 @@ describe("getClient", () => { client.pathUnchecked("/foo").get({ onResponse: onResponseFn, }), - ).rejects.toThrow(); + ).rejects.toThrow(/error/); expect(onResponseFn).toHaveBeenCalled(); }); From 03810f92199a2979ad31113f1379ab459f2ddf17 Mon Sep 17 00:00:00 2001 From: Deyaaeldeen Almahallawi Date: Tue, 21 Apr 2026 18:50:54 +0000 Subject: [PATCH 17/17] Use assert.isNotEmpty for consistency Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- sdk/core/core-client-rest/test/internal/clientHelpers.spec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sdk/core/core-client-rest/test/internal/clientHelpers.spec.ts b/sdk/core/core-client-rest/test/internal/clientHelpers.spec.ts index 1a73d49bbf98..d8343ae6cdac 100644 --- a/sdk/core/core-client-rest/test/internal/clientHelpers.spec.ts +++ b/sdk/core/core-client-rest/test/internal/clientHelpers.spec.ts @@ -14,7 +14,7 @@ describe("clientHelpers", () => { const pipeline = createDefaultPipeline(mockBaseUrl); const policies = pipeline.getOrderedPolicies(); - assert.isAbove(policies.length, 0, "default pipeline should contain policies"); + assert.isNotEmpty(policies, "default pipeline should contain policies"); assert.isUndefined( policies.find((p) => p.name === bearerTokenAuthenticationPolicyName),