Skip to content

DocumentClient: Adds tests for PartitionKeyRangeLocation disposal#5778

Merged
kirankumarkolli merged 3 commits into
masterfrom
users/kirankk/dispose-partition-endpoint-tests
Apr 13, 2026
Merged

DocumentClient: Adds tests for PartitionKeyRangeLocation disposal#5778
kirankumarkolli merged 3 commits into
masterfrom
users/kirankk/dispose-partition-endpoint-tests

Conversation

@kirankumarkolli
Copy link
Copy Markdown
Member

@kirankumarkolli kirankumarkolli commented Apr 12, 2026

Description

Adds unit tests covering the fix in PR #5777 where DocumentClient.Dispose() was missing disposal of PartitionKeyRangeLocation (GlobalPartitionEndpointManagerCore), causing the circuit breaker failback loop timer to leak.

Tests added to GlobalPartitionEndpointManagerUnitTests:

  • Dispose_CancelsCancellationTokenSource: verifies cancellation token is cancelled after dispose, preventing background loop re-initialization
  • Dispose_IsIdempotent: multiple dispose calls are safe (no exceptions)
  • Dispose_StopsBackgroundFailbackLoop: background callback stops firing after dispose
  • NoOp_DoesNotImplementIDisposable: confirms GlobalPartitionEndpointManagerNoOp is not IDisposable

New DocumentClientDisposeTests class:

  • Dispose_DisposesPartitionKeyRangeLocationWhenIDisposable: verifies property is disposed, nulled, and manager's background loop is stopped
  • Dispose_HandlesNonDisposablePartitionKeyRangeLocation: GlobalPartitionEndpointManagerNoOp doesn't throw
  • Dispose_HandlesNullPartitionKeyRangeLocation: null property (uninitialized client) doesn't throw
  • Dispose_IsIdempotentWithDisposablePartitionKeyRangeLocation: double-dispose is safe end-to-end

Type of change

  • Bug fix (non-breaking change which fixes an issue)

@kirankumarkolli
DocumentClient: Adds tests for PartitionKeyRangeLocation disposal
e560173 
Adds unit tests covering the fix in PR #5777 where DocumentClient.Dispose()
was missing disposal of PartitionKeyRangeLocation (GlobalPartitionEndpointManagerCore),
causing the circuit breaker failback loop timer to leak.

Tests added to GlobalPartitionEndpointManagerUnitTests:
- Dispose_CancelsCancellationTokenSource: verifies cancellation after dispose
- Dispose_IsIdempotent: multiple dispose calls are safe
- Dispose_StopsBackgroundFailbackLoop: background loop stops after dispose
- NoOp_DoesNotImplementIDisposable: confirms NoOp variant is not IDisposable

New DocumentClientDisposeTests class:
- Dispose_DisposesPartitionKeyRangeLocationWhenIDisposable
- Dispose_HandlesNonDisposablePartitionKeyRangeLocation
- Dispose_HandlesNullPartitionKeyRangeLocation
- Dispose_IsIdempotentWithDisposablePartitionKeyRangeLocation

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

azure-pipelines Bot commented Apr 12, 2026

Azure Pipelines:
Successfully started running 1 pipeline(s).

kirankumarkolli
kirankumarkolli commented Apr 12, 2026
View reviewed changes
Comment thread Microsoft.Azure.Cosmos/src/DocumentClient.cs Outdated
@kirankumarkolli
Address review feedback: simplify disposal and consolidate tests
1a26afc 
- DocumentClient.cs: Replace 'is IDisposable' pattern with simple null check
- Move Dispose_DisposesPartitionKeyRangeLocationWhenIDisposable test to
  GlobalPartitionEndpointManagerUnitTests
- Remove Dispose_CancelsCancellationTokenSource, Dispose_IsIdempotent,
  and NoOp_DoesNotImplementIDisposable tests
- Delete DocumentClientDisposeTests.cs (remaining tests undone)

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

azure-pipelines Bot commented Apr 12, 2026

Azure Pipelines:
Successfully started running 1 pipeline(s).

@azure-pipelines
Copy link
Copy Markdown

azure-pipelines Bot commented Apr 12, 2026

Azure Pipelines:
Successfully started running 1 pipeline(s).

kirankumarkolli
kirankumarkolli commented Apr 12, 2026
View reviewed changes
Comment thread Microsoft.Azure.Cosmos/src/DocumentClient.cs
@azure-pipelines
Copy link
Copy Markdown

azure-pipelines Bot commented Apr 12, 2026

Azure Pipelines:
Successfully started running 1 pipeline(s).

@kirankumarkolli kirankumarkolli force-pushed the users/kirankk/dispose-partition-endpoint-tests branch from f363392 to cd8b4cc Compare April 12, 2026 21:51
@azure-pipelines
Copy link
Copy Markdown

azure-pipelines Bot commented Apr 12, 2026

Azure Pipelines:
Successfully started running 1 pipeline(s).

@kirankumarkolli kirankumarkolli changed the title [Internal] DocumentClient: Adds tests for PartitionKeyRangeLocation disposal DocumentClient: Fixes DocumentClient to dispose PartitionKeyRangeLocation Apr 12, 2026
@kirankumarkolli kirankumarkolli changed the title DocumentClient: Fixes DocumentClient to dispose PartitionKeyRangeLocation DocumentClient: Adds tests for PartitionKeyRangeLocation disposal Apr 13, 2026
kirankumarkolli
kirankumarkolli commented Apr 13, 2026
View reviewed changes
Copy link
Copy Markdown
Member Author

@kirankumarkolli kirankumarkolli left a comment

Choose a reason for hiding this comment

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

LGTM, ready for review

kundadebdatta
kundadebdatta approved these changes Apr 13, 2026
View reviewed changes
Copy link
Copy Markdown
Member

@kundadebdatta kundadebdatta left a comment

Choose a reason for hiding this comment

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

LGTM.

@kirankumarkolli kirankumarkolli merged commit 79d18b7 into master Apr 13, 2026
34 checks passed
@kirankumarkolli kirankumarkolli deleted the users/kirankk/dispose-partition-endpoint-tests branch April 13, 2026 17:12
@kirankumarkolli kirankumarkolli mentioned this pull request Apr 13, 2026
Closed
1 task
@kirankumarkolli kirankumarkolli mentioned this pull request May 13, 2026
Closed
This was referenced May 18, 2026
Closed
Open
Open
Merged
Open
Open
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@kundadebdatta kundadebdatta kundadebdatta approved these changes

@khdang khdang Awaiting requested review from khdang

@sboshra sboshra Awaiting requested review from sboshra

@adityasa adityasa Awaiting requested review from adityasa

@neildsh neildsh Awaiting requested review from neildsh

@FabianMeiswinkel FabianMeiswinkel Awaiting requested review from FabianMeiswinkel

@kirillg kirillg Awaiting requested review from kirillg

@Pilchie Pilchie Awaiting requested review from Pilchie

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@kirankumarkolli @kundadebdatta