Skip to content

Improving Test robustness after changing configured Regions for live tests#49639

Merged
FabianMeiswinkel merged 83 commits into
Azure:mainfrom
FabianMeiswinkel:users/fabianm/Testfix
Jul 2, 2026
Merged

Improving Test robustness after changing configured Regions for live tests#49639
FabianMeiswinkel merged 83 commits into
Azure:mainfrom
FabianMeiswinkel:users/fabianm/Testfix

Conversation

@FabianMeiswinkel

Copy link
Copy Markdown
Member

No description provided.

Copilot AI review requested due to automatic review settings June 25, 2026 23:43
@FabianMeiswinkel FabianMeiswinkel requested review from a team and kirankumarkolli as code owners June 25, 2026 23:43
@FabianMeiswinkel

Copy link
Copy Markdown
Member Author

/azp run java - cosmos - tests

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

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 improves Cosmos E2E test diagnosability by replacing terse region-contact assertions with helper methods that generate richer failure messages including preferred/service-ordered regions and diagnostics context.

Changes:

  • Added reusable assertion helpers for validating contacted region sets/counts with detailed failure output.
  • Updated multiple retry/failover E2E tests to use the new helpers for clearer intent and debugging.
Show a summary per file
File Description
sdk/cosmos/azure-cosmos-tests/src/test/java/com/azure/cosmos/rx/ClientRetryPolicyE2ETests.java Introduces contacted-region assertion helpers and applies them across client retry policy E2E scenarios.
sdk/cosmos/azure-cosmos-tests/src/test/java/com/azure/cosmos/PerPartitionCircuitBreakerE2ETests.java Refactors diagnostics-context contacted-region asserts into helper methods with more informative failure messages.

Review details

  • Files reviewed: 2/2 changed files
  • Comments generated: 2
  • Review effort level: Low

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Review details

  • Files reviewed: 2/2 changed files
  • Comments generated: 1
  • Review effort level: Low

FabianMeiswinkel and others added 2 commits June 26, 2026 12:54
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
@FabianMeiswinkel

Copy link
Copy Markdown
Member Author

/azp run java - cosmos - tests

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

@FabianMeiswinkel

Copy link
Copy Markdown
Member Author

/azp run java - cosmos - tests

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

@FabianMeiswinkel

Copy link
Copy Markdown
Member Author

Resolution — findings triaged ✅

Worked through all 9 findings one-by-one. Dispositions:

Addressed in code (this branch):

  • 🔴 P1 — Multiplicative retry latency: RxDocumentClientImpl outer routing-map retry reduced 10 → 1 (worst-case ~60–90s → ~15s) so it no longer multiplies with InCompleteRoutingMapRetryPolicy's backoff; kept the collection-cache refresh. Added a bounded-attempts unit test that also asserts the 404/1024 terminal.
  • 🟡 P2 — 404/1024 terminal error: documented in azure-cosmos/CHANGELOG.md.
  • 🟡 P2 — Cross-region-on-404 rationale: added an explanatory comment in ClientRetryPolicy.
  • 🟡 P2 — Missing unit tests: added a ClientRetryPolicy new-branch test (sub-status 0/1003/1013 + negative gating), expanded InCompleteRoutingMapRetryPolicyTest (1003/1013 + exhaustion, renamed the misleading test), and added a BulkOperationRetryPolicy 2-attempt-bound test. All pass.

Intentional / no change (won't fix):

  • ServerConnectionTimeout and warmup assertion loosenings — deliberate CI-stability changes for the new higher-latency regions.
  • PerPartitionCircuitBreaker 5-min transient-404 retry and satellite-region 404/1002 batch tolerance — deliberate, scoped tolerances for the post-create metadata race (the circuit-breaker retry skips fault-injected failures; the batch tolerance is region-guarded).
  • ClientMetricsTest region-metric — the loosening is a correctness fix (read/query routing can legitimately contact a non-first region).

Resolving this thread.


⚠️ AI-generated review — resolution recorded by the PR author.

@FabianMeiswinkel

Copy link
Copy Markdown
Member Author

/azp run java - cosmos - tests

@FabianMeiswinkel

Copy link
Copy Markdown
Member Author

/azp run java - cosmos - kafka

@FabianMeiswinkel

Copy link
Copy Markdown
Member Author

/azp run java - cosmos - spark

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

2 similar comments
@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

@FabianMeiswinkel

Copy link
Copy Markdown
Member Author

/azp run java - cosmos - tests

@FabianMeiswinkel

Copy link
Copy Markdown
Member Author

/azp run java - cosmos - kafka

@FabianMeiswinkel

Copy link
Copy Markdown
Member Author

/azp run java - cosmos - spark

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

2 similar comments
@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

@xinlian12 xinlian12 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM, thanks

@FabianMeiswinkel FabianMeiswinkel merged commit a79d582 into Azure:main Jul 2, 2026
135 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants