Skip to content

Improve test coverage to ~100% for @azure-rest/core-client#38177

Merged
deyaaeldeen merged 17 commits intomainfrom
deyaaeldeen/core-client-rest-test-coverage
Apr 21, 2026
Merged

Improve test coverage to ~100% for @azure-rest/core-client#38177
deyaaeldeen merged 17 commits intomainfrom
deyaaeldeen/core-client-rest-test-coverage

Conversation

@deyaaeldeen
Copy link
Copy Markdown
Member

@deyaaeldeen deyaaeldeen commented Apr 16, 2026

Changes

New test files

  • keyCredentialAuthenticationPolicy.spec.ts
  • operationOptionHelpers.spec.ts

Expanded existing tests

  • clientHelpers.spec.ts: error handling, pipeline configuration
  • createRestError.spec.ts: error code paths, replaced {} as PipelineRequest with createPipelineRequest()
  • getClient.spec.ts: custom policies, onResponse callbacks, error propagation
  • streams.spec.ts (browser + node): error paths, stream type validation

Test quality improvements

  • Replaced boolean policyExecuted flags with vi.fn() + toHaveBeenCalled()
  • Replaced try/catch + assert.fail with rejects.toThrow()
  • deepEqual on primitives changed to strictEqual
  • Removed redundant isDefined before equal assertions

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@deyaaeldeen deyaaeldeen marked this pull request as ready for review April 17, 2026 00:48
Copilot AI review requested due to automatic review settings April 17, 2026 00:48
@deyaaeldeen deyaaeldeen requested review from a team, joheredi and timovv as code owners April 17, 2026 00:48
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 increases test coverage for @azure-rest/core-client by adding/expanding internal Vitest specs to exercise previously uncovered helpers and request-method wrappers.

Changes:

  • Added new internal tests for operationOptionsToRequestParameters.
  • Added a new internal test for keyCredentialAuthenticationPolicy.
  • Extended getClient tests to validate all supported HTTP verb helpers (post, put, patch, delete, head, options, trace).
  • Extended clientHelpers tests to cover getCachedDefaultHttpsClient caching behavior.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.

File Description
sdk/core/core-client-rest/test/internal/operationOptionHelpers.spec.ts New tests for operation options → request parameters conversion (empty + abortSignal).
sdk/core/core-client-rest/test/internal/keyCredentialAuthenticationPolicy.spec.ts New test validating API key header injection behavior for the policy.
sdk/core/core-client-rest/test/internal/getClient.spec.ts Adds coverage for non-GET HTTP method helpers to ensure correct PipelineRequest.method.
sdk/core/core-client-rest/test/internal/clientHelpers.spec.ts Adds coverage for getCachedDefaultHttpsClient instance caching.

Deyaaeldeen Almahallawi and others added 2 commits April 17, 2026 01:15
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
xirzec
xirzec previously requested changes Apr 17, 2026
Comment thread sdk/core/core-client-rest/test/internal/operationOptionHelpers.spec.ts Outdated
…ptions 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>
@deyaaeldeen deyaaeldeen requested a review from xirzec April 17, 2026 21:37
Deyaaeldeen Almahallawi and others added 7 commits April 17, 2026 23:38
… 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>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- 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>
- 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>
…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>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@deyaaeldeen deyaaeldeen force-pushed the deyaaeldeen/core-client-rest-test-coverage branch from 012f678 to 3715d87 Compare April 19, 2026 20:21
Deyaaeldeen Almahallawi and others added 4 commits April 20, 2026 16:07
… 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>
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!

Comment thread sdk/core/core-client-rest/test/internal/clientHelpers.spec.ts Outdated
Deyaaeldeen Almahallawi and others added 2 commits April 21, 2026 18:46
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@deyaaeldeen deyaaeldeen requested a review from jeremymeng April 21, 2026 18:51
@deyaaeldeen deyaaeldeen merged commit 38058fd into main Apr 21, 2026
14 checks passed
@deyaaeldeen deyaaeldeen deleted the deyaaeldeen/core-client-rest-test-coverage branch April 21, 2026 21: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.

5 participants