Skip to content

Improve test coverage to ~100% for @azure/core-rest-pipeline#38180

Merged
deyaaeldeen merged 40 commits intomainfrom
deyaaeldeen/core-rest-pipeline-test-coverage
Apr 24, 2026
Merged

Improve test coverage to ~100% for @azure/core-rest-pipeline#38180
deyaaeldeen merged 40 commits intomainfrom
deyaaeldeen/core-rest-pipeline-test-coverage

Conversation

@deyaaeldeen
Copy link
Copy Markdown
Member

@deyaaeldeen deyaaeldeen commented Apr 16, 2026

Changes

New test files

  • createPipelineFromOptions.spec.ts (browser + node)
  • file.spec.ts (browser + node)
  • policyFactories.spec.ts (node)
  • tokenCycler.spec.ts
  • userAgent.spec.ts
  • userAgentPolicy.spec.ts

Expanded existing tests

  • bearerTokenAuthenticationPolicy.spec.ts: CAE challenges, token refresh, error recovery
  • auxiliaryAuthenticationHeaderPolicy.spec.ts: multi-tenant auth flows
  • fetchHttpClient.spec.ts: abort, streaming, progress reporting
  • nodeHttpClient.spec.ts: abort, streaming, upload/download progress
  • pipeline.spec.ts: policy ordering, request overrides
  • tracingPolicy.spec.ts: span attributes, error recording
  • ndJsonPolicy.spec.ts: edge cases
  • setClientRequestIdPolicy.spec.ts: header setting

Test quality improvements

  • Replaced downloadCalled/uploadCalled/bodySent/listenerRemoved flags with vi.fn()
  • Replaced isCallbackCalled/getTokenRequests counters with vi.fn()
  • Removed inverted try/catch (success assertions) in favor of direct await
  • {} as HttpHeaders changed to createHttpHeaders()
  • Removed redundant 0 as number cast
  • deepEqual on primitives changed to strictEqual

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Deyaaeldeen Almahallawi and others added 12 commits April 17, 2026 00:09
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
SharedArrayBuffer is available in Node >=8.10 and all modern browsers.
The minimum supported Node version is 20.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…en tests

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@deyaaeldeen deyaaeldeen marked this pull request as ready for review April 17, 2026 16:46
@deyaaeldeen deyaaeldeen requested a review from a team as a code owner April 17, 2026 16:46
Copilot AI review requested due to automatic review settings April 17, 2026 16:46
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR adds and extends Vitest coverage for @azure/core-rest-pipeline with the goal of reaching ~100% coverage, primarily by exercising previously untested branches across pipeline policies and internal utilities.

Changes:

  • Adds new internal tests for tokenCycler, file, createPipelineFromOptions, Node policy factories, and user-agent helpers (Node + browser where applicable).
  • Extends existing tests to cover additional branches in multiple pipeline policies (auth, tracing, request-id, NDJSON, user-agent platform).
  • Introduces additional mocking to exercise error/edge paths (e.g., tracing client creation failure, non-recording spans, CAE challenge branches).

Reviewed changes

Copilot reviewed 14 out of 14 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
sdk/core/core-rest-pipeline/test/public/tracingPolicy.spec.ts Adds additional tracingPolicy branch tests (client creation failure, non-recording span, response header attribute, error processing paths, user-agent branch).
sdk/core/core-rest-pipeline/test/public/setClientRequestIdPolicy.spec.ts Adds coverage for not overwriting an existing x-ms-client-request-id.
sdk/core/core-rest-pipeline/test/public/ndJsonPolicy.spec.ts Adds pass-through coverage for non-array / non-string-body cases.
sdk/core/core-rest-pipeline/test/internal/tokenCycler.spec.ts Adds new tests covering token refresh failure and retry loop behavior with fake timers.
sdk/core/core-rest-pipeline/test/internal/node/userAgentPolicy.spec.ts Adds Node-only tests for User-Agent header overwrite behavior.
sdk/core/core-rest-pipeline/test/internal/node/userAgentPlatform.spec.ts Improves mocking and expands runtime-detection branch coverage (Bun/Deno/Node/unknown).
sdk/core/core-rest-pipeline/test/internal/node/userAgent.spec.ts Adds internal tests for user-agent string formatting/prefix handling and platform-data formatting edge cases.
sdk/core/core-rest-pipeline/test/internal/node/policyFactories.spec.ts Adds tests for Node policy factory functions and proxy env parsing behavior.
sdk/core/core-rest-pipeline/test/internal/node/file.spec.ts Adds Node tests for file utilities including stream behavior and raw content helpers.
sdk/core/core-rest-pipeline/test/internal/node/createPipelineFromOptions.spec.ts Adds Node tests ensuring agent/tls policies are wired in via options.
sdk/core/core-rest-pipeline/test/internal/browser/file.spec.ts Adds browser tests for file utilities including File constructor and stream factory behavior.
sdk/core/core-rest-pipeline/test/internal/browser/createPipelineFromOptions.spec.ts Adds browser tests ensuring Node-only policies are excluded/ignored.
sdk/core/core-rest-pipeline/test/internal/bearerTokenAuthenticationPolicy.spec.ts Refactors request creation helper and adds many new CAE/custom-challenge edge-path tests.
sdk/core/core-rest-pipeline/test/internal/auxiliaryAuthenticationHeaderPolicy.spec.ts Refactors request creation helper and adds tests for empty/undefined credentials behavior.

Comment thread sdk/core/core-rest-pipeline/test/internal/browser/file.spec.ts Outdated
Comment thread sdk/core/core-rest-pipeline/test/internal/bearerTokenAuthenticationPolicy.spec.ts Outdated
Comment thread sdk/core/core-rest-pipeline/test/public/tracingPolicy.spec.ts Outdated
Comment thread sdk/core/core-rest-pipeline/test/public/setClientRequestIdPolicy.spec.ts Outdated
Comment thread sdk/core/core-rest-pipeline/test/public/ndJsonPolicy.spec.ts Outdated
Deyaaeldeen Almahallawi and others added 11 commits April 17, 2026 17:07
- createPipelineFromOptions.spec.ts: use proper Agent/TlsSettings types
- policyFactories.spec.ts: remove unused PipelineResponse, fix Agent/TlsSettings
- userAgentPlatform.spec.ts: add default property to mock type parameters
- Format all test files

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Guard SharedArrayBuffer test with it.runIf for browser environments
- Fix typo 'retriving' -> 'retrieving' in bearerTokenAuth test comment
- Merge duplicate describe blocks (tracingPolicy, setClientRequestIdPolicy, ndJsonPolicy)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- agentPolicy: verify request.agent is set and not overridden
- proxyPolicy: verify proxy agent is set on request, not overridden
- tlsPolicy: verify request.tlsSettings is set and not overridden
- Keep retry policy smoke tests as-is (well-tested elsewhere)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Fix weak assertions: replace .resolves; with .resolves.toBeDefined()
- Fix reversed assert.equal arguments in bearerTokenAuthenticationPolicy test

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…ient test

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Replace try/catch + assert.fail() with rejects.toThrow/toMatchObject
and assert.throws() for cleaner, more idiomatic test code.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…peline tests

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Deyaaeldeen Almahallawi and others added 14 commits April 19, 2026 20:23
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Replace manual boolean tracking with vi.fn() mock functions and
expect().toHaveBeenCalled() assertions for cleaner test patterns.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Replace inline 'as' type casts with:
- Typed vi.importActual<typeof import(...)> for mock factories
- Helper functions (toIncomingMessage, createMockWritableRequest, getMultipartParts, readBodyAsArrayBuffer) to isolate necessary casts
- Proper typed objects for Agent and TlsSettings instead of empty object casts
- assert.instanceOf for runtime type narrowing (ReadableStream)
- Typed vi.fn<TokenCredential["getToken"]>() instead of 'as any'
- Runtime type guards instead of inline casts
- satisfies operator for browser test stubs
- createHttpHeaders() instead of {} as HttpHeaders

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Replace BodyInit with ConstructorParameters<typeof Response>[0]
- Fix signalLike type annotation to be compatible with AbortSignalLike
- Spread actual module instead of actual.default for node:https/http mocks

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Replace if/assert.fail type guard with assert.instanceOf for Uint8Array
- Replace if/assert.fail type guard with assert.isString for http.url attribute

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Convert try/catch + error capture to expect().rejects.toThrow() in
  auxiliaryAuthenticationHeaderPolicy and bearerTokenAuthenticationPolicyChallenge
- Replace any-typed vi.fn params with Buffer/BufferEncoding types in nodeHttpClient

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copy link
Copy Markdown
Member

@jeremymeng jeremymeng left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good overall! I added some comments

Comment thread sdk/core/core-rest-pipeline/test/public/node/nodeHttpClient.spec.ts Outdated
Deyaaeldeen Almahallawi and others added 2 commits April 23, 2026 23:34
- Remove unnecessary IIFE for httpRequestChecker (revert to plain object)
- Rename duplicate test suite to 'with fake timers' for clarity
- Strengthen onDownloadProgress assertions to verify ev.loadedBytes

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
BufferEncoding is narrower than what Writable.write passes, causing
TS2345 in CI. Also removes unnecessary IIFE for httpRequestChecker.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@deyaaeldeen deyaaeldeen merged commit ef31af4 into main Apr 24, 2026
32 checks passed
@deyaaeldeen deyaaeldeen deleted the deyaaeldeen/core-rest-pipeline-test-coverage branch April 24, 2026 00:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants