Improve test coverage to ~100% for @azure/core-amqp by deyaaeldeen · Pull Request #38174 · Azure/azure-sdk-for-js

deyaaeldeen · 2026-04-16T23:49:56Z

Changes

New test files

checkNetworkConnection.spec.ts (browser)
checkNetworkMocked.spec.ts (node, mocked network)
errors.spec.ts (browser)
hmacSha256.spec.ts (browser)
retryNetworkDown.spec.ts (node)
runtimeInfo.spec.ts (browser)
typeGuards.spec.ts
utils.spec.ts

Expanded existing tests

cbs.spec.ts: connection error paths, session close errors
context.spec.ts: extracted properties/webSocketOptions into typed consts
errors.spec.ts: translate edge cases, mapping coverage
lock.spec.ts: timeout and release paths
message.spec.ts: header/properties encoding edge cases
pipeline.spec.ts: policy ordering
requestResponse.spec.ts: retry, timeout, cleanup callbacks
retry.spec.ts: all retry modes and error paths
tokenProvider.spec.ts: token refresh edge cases

Test quality improvements

Replaced hand-rolled counters with vi.fn() + toHaveBeenCalledTimes()
Replaced boolean tracking flags with vi.fn() + toHaveBeenCalled()
Replaced try/catch + assert.fail with rejects.toThrow()
Removed unnecessary as any casts
Extracted non-null assertions into typed consts

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

- Use plain type instead of Class & {...} intersection for private access (private fields cause 'never' type in intersection) - Remove unused imports (expect, MessagingError) and unused variables - Add body to mock RheaMessage, fix statusCode type assertion Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

randomNumberFromInterval, executePromisesSequentially, isIotHubConnectionString, and Timeout were removed in #38149. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

- Remove redundant dynamic imports where top-level import suffices - Rename dynamic imports to avoid shadowing (check → alias) - Rename local connectionString to emulatorConnectionString Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

- tokenProvider: isTrue(x < 2) → isBelow(x, 2) - context: isTrue(calls.length > 0) → isAbove(calls.length, 0) - retryNetworkDown: isTrue(calls.length > 0) → isAbove(calls.length, 0) - lock, requestResponse: remove line number references from test names Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

- requestResponse: isTrue(instanceof) → instanceOf, equal(x,true) → isTrue - errors: equal(retryable, false) → isFalse - message: remove redundant isDefined before equal Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

…ontext and message tests - Extract properties/webSocketOptions into local consts after asserting defined - Extract date values into consts to avoid non-null assertions on optional fields 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>

Replace hand-rolled callCount with vi.fn() and try/catch + assert.fail() with rejects.toThrow() for cleaner test code. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

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

Copilot

Pull request overview

This PR focuses on bringing @azure/core-amqp’s unit test coverage close to 100% by adding new tests and expanding existing suites across Node and browser runtimes, plus a small typing improvement in a related core package test.

Changes:

Added multiple new Node- and browser-specific test suites to cover previously untested branches (network checks, crypto signing, runtime info, type guards, utilities).
Expanded and refactored existing tests to cover additional error paths and edge cases (CBS, retry, message encoding, locks, request/response behavior).
Minor test typing cleanup in @azure/core-rest-pipeline by using createHttpHeaders() rather than casting.

Reviewed changes

Copilot reviewed 18 out of 18 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
sdk/core/core-rest-pipeline/test/public/pipeline.spec.ts	Uses `createHttpHeaders()` in a response-shape test to avoid manual header casting.
sdk/core/core-amqp/test/utils/createConnectionStub.ts	Adds a “full” connection/session/sender/receiver stub to support more link lifecycle tests.
sdk/core/core-amqp/test/public/tokenProvider.spec.ts	Adds additional SasTokenProvider behavior tests and tightens assertions.
sdk/core/core-amqp/test/public/retry.spec.ts	Refactors retry tests to use `vi.fn`/`expect` assertions and adds more default/edge-path tests.
sdk/core/core-amqp/test/public/message.spec.ts	Improves message TTL/expiry test clarity and removes non-null assertions via typed locals.
sdk/core/core-amqp/test/public/context.spec.ts	Refactors setup constants and adds coverage for maxListeners behavior on the connection.
sdk/core/core-amqp/test/public/cbs.spec.ts	Adds CBS close/remove/isOpen and error-handler coverage using the new full stub.
sdk/core/core-amqp/test/internal/utils.spec.ts	Adds unit tests for internal utils helpers (`isString`, `isNumber`, `delay`, `getGlobalProperty`).
sdk/core/core-amqp/test/internal/typeGuards.spec.ts	Adds tests for `isSasTokenProvider` including real provider instances.
sdk/core/core-amqp/test/internal/requestResponse.spec.ts	Adds additional RequestResponseLink lifecycle/error-path tests beyond the existing skipped browser suite.
sdk/core/core-amqp/test/internal/node/retryNetworkDown.spec.ts	Adds Node-specific test that mocks network check to validate retry behavior when network is down.
sdk/core/core-amqp/test/internal/node/checkNetworkMocked.spec.ts	Adds Node DNS-mocking tests for `checkNetworkConnection`.
sdk/core/core-amqp/test/internal/lock.spec.ts	Expands lock tests for timeouts/empty-queue/private-path coverage.
sdk/core/core-amqp/test/internal/errors.spec.ts	Adds more translate/mapping edge-case tests and removes unnecessary casts.
sdk/core/core-amqp/test/internal/browser/runtimeInfo.spec.ts	Adds browser runtime info tests (`getPlatformInfo`, `getFrameworkInfo`).
sdk/core/core-amqp/test/internal/browser/hmacSha256.spec.ts	Adds browser Web Crypto signing tests for `signString`.
sdk/core/core-amqp/test/internal/browser/errors.spec.ts	Adds browser websocket error translation tests.
sdk/core/core-amqp/test/internal/browser/checkNetworkConnection.spec.ts	Adds browser test validating navigator-based network check behavior.

- Create typed mock helpers (createMockSession, createMockSender, createMockReceiver, mockCreateSession) in createConnectionStub.ts to replace inline `as any` session mocks - Update createFullConnectionStub to use the new mock helpers - Create accessor helpers (getCbsLink, getResponsesMap, lockPrivate) to consolidate private member access casts into single locations - Replace `isError(err) + (err as Error)` patterns with `assert.instanceOf(err, Error)` which narrows types automatically via vitest's assertion type guards - Replace `translate(x) as MessagingError` with `translate(x) + assert.instanceOf` - Replace `as Date` casts with `assert.instanceOf(x, Date)` in message.spec.ts - Replace `let req: any = {}` with `let req: RheaMessage = { body: undefined }` - Remove unused `isError` imports from @azure/core-util Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

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

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

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

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

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

jeremymeng · 2026-04-23T19:47:15Z

Looks good overall! I added some comments. There are some more un-commented

callCount verification could be replaced with spy
try-catch verification of error could be replaced with expect().rejects

They can be addressed in follow-up if we want to maintain consistency

- Verify exact HMAC-SHA256 signature value instead of isOk/isString - Remove duplicate hmacSha256 describe block - Add abort listener removal verification via spy - Replace manual callCount with vi.fn() spy in retry test - Convert 3 try/catch + assert.fail patterns to expect().rejects Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

- Convert all try/catch + TEST_FAILURE sentinel patterns to expect().rejects in requestResponse.spec.ts (6 instances), cbs.spec.ts (7 instances), lock.spec.ts (1 instance) - Remove unused TEST_FAILURE constants - Add expect import where needed Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Verify removeEventListener was called specifically with 'abort' event type and a function, not just that it was called at all. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

github-actions Bot added the Azure.Core label Apr 16, 2026

deyaaeldeen force-pushed the deyaaeldeen/core-amqp-test-coverage branch from 0c75c05 to 50411da Compare April 18, 2026 03:55

Deyaaeldeen Almahallawi and others added 10 commits April 18, 2026 05:41

Improve test coverage to ~100% for @azure/core-amqp

b71929d

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

Remove 'coverage' from test describe block names

c38c099

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

Clean up describe block names

e82f9dd

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

Strengthen isDefined-only assertions with value checks

b7c9744

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

Extract repeated test constants into shared variables

fca5a36

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

Move imports to top level (vitest hoists vi.mock automatically)

ccc3967

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

Format context.spec.ts

bbfbe78

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

fix: merge main and remove tests for deleted utils functions

2acac63

randomNumberFromInterval, executePromisesSequentially, isIotHubConnectionString, and Timeout were removed in #38149. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

deyaaeldeen force-pushed the deyaaeldeen/core-amqp-test-coverage branch from 50411da to cfdb9d1 Compare April 18, 2026 05:41

Deyaaeldeen Almahallawi and others added 9 commits April 18, 2026 07:41

refactor: modernize spy and error patterns in core-amqp tests

3147a81

Replace hand-rolled callCount with vi.fn() and try/catch + assert.fail() with rejects.toThrow() for cleaner test code. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

refactor: third pass modernization in core-amqp tests

7445146

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

fix: remove foreign package files from core-amqp branch

d8ad111

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

refactor: remove unnecessary type casts in core-rest-pipeline tests

0944088

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

refactor: remove unnecessary type casts in core-amqp tests

7b9ea2a

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

deyaaeldeen marked this pull request as ready for review April 20, 2026 16:32

Copilot AI review requested due to automatic review settings April 20, 2026 16:32

deyaaeldeen requested review from a team and xirzec as code owners April 20, 2026 16:32

Copilot started reviewing on behalf of deyaaeldeen April 20, 2026 16:34 View session

Copilot AI reviewed Apr 20, 2026

View reviewed changes

Comment thread sdk/core/core-amqp/test/internal/requestResponse.spec.ts Outdated

Comment thread sdk/core/core-amqp/test/public/context.spec.ts Outdated

Comment thread sdk/core/core-amqp/test/internal/utils.spec.ts Outdated

Deyaaeldeen Almahallawi and others added 2 commits April 20, 2026 17:09

fix: restore foreign pipeline.spec.ts to main version

a9bcd33

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

Deyaaeldeen Almahallawi and others added 9 commits April 20, 2026 18:25

fix: resolve lint errors from cast elimination

c5e6ca9

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

address PR feedback: skipIf(isBrowser), restore prototype spies, fix …

c53141b

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

move node-only requestResponse tests to test/internal/node/

9f81b9a

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

rename duplicate test names in cbs.spec.ts for clarity

e1c24bc

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

trigger CI re-run

cf4466e

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

fix: align test title to cover all three methods tested in context.sp…

424dc33

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

fix: remove foreign core-client files from amqp branch

f524a11

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

Fix test titles to match actual assertions in cbs and retry tests

6fc4b1d

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

Replace isAbove(x.length, 0) with isNotEmpty/toHaveBeenCalled

59472f2

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

jeremymeng reviewed Apr 23, 2026

View reviewed changes

Deyaaeldeen Almahallawi and others added 3 commits April 23, 2026 21:36

Strengthen abort listener removal assertion per review feedback

63dadbc

Verify removeEventListener was called specifically with 'abort' event type and a function, not just that it was called at all. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

jeremymeng approved these changes Apr 23, 2026

View reviewed changes

deyaaeldeen merged commit 49bc828 into main Apr 23, 2026
32 checks passed

deyaaeldeen deleted the deyaaeldeen/core-amqp-test-coverage branch April 23, 2026 23:10

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improve test coverage to ~100% for @azure/core-amqp#38174

Improve test coverage to ~100% for @azure/core-amqp#38174
deyaaeldeen merged 33 commits intomainfrom
deyaaeldeen/core-amqp-test-coverage

deyaaeldeen commented Apr 16, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jeremymeng commented Apr 23, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

deyaaeldeen commented Apr 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Changes

New test files

Expanded existing tests

Test quality improvements

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jeremymeng commented Apr 23, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

deyaaeldeen commented Apr 16, 2026 •

edited

Loading