Summary
Follow-up from PR #48914 (review feedback from @kushagraThapar).
The 20-arg positional new StoreResult(...) constructor is repeated across many test methods (in ConsistencyWriterTest.java at lines ~667, 725, 757, 779, 834, 894, 903, 919, 947, and several more in QuorumReaderTest.java). Each invocation follows the same pattern: most arguments are constants/null/false, with only 2–3 meaningful per test (replica role, isThrottledException, isValid).
This positional-constructor accretion has been growing across multiple PRs and makes tests verbose and fragile.
Proposed change
Extract a StoreResultTestFactory (or builder) with named, intent-revealing methods, e.g.:
throttled429()
valid(int globalCommittedLsn)
unhealthy(replicaRole)
Benefits:
- New tests become 1-line setup instead of ~20-line setup.
- When the
StoreResult constructor changes (e.g. adding a 21st arg), only the factory updates instead of N test sites.
- Test intent becomes self-documenting (
throttled429() reads as a behavior, not a 20-arg ctor invocation).
Scope
Test-only refactor; no production behavior change. Update existing StoreResult-based tests in:
sdk/cosmos/azure-cosmos-tests/.../directconnectivity/ConsistencyWriterTest.java
sdk/cosmos/azure-cosmos-tests/.../directconnectivity/QuorumReaderTest.java
Context
Reviewer noted this is "not a blocker for [PR #48914] but worth scheduling as a follow-up before the next test wave lands on top."
Summary
Follow-up from PR #48914 (review feedback from @kushagraThapar).
The 20-arg positional
new StoreResult(...)constructor is repeated across many test methods (inConsistencyWriterTest.javaat lines ~667, 725, 757, 779, 834, 894, 903, 919, 947, and several more inQuorumReaderTest.java). Each invocation follows the same pattern: most arguments are constants/null/false, with only 2–3 meaningful per test (replica role,isThrottledException,isValid).This positional-constructor accretion has been growing across multiple PRs and makes tests verbose and fragile.
Proposed change
Extract a
StoreResultTestFactory(or builder) with named, intent-revealing methods, e.g.:throttled429()valid(int globalCommittedLsn)unhealthy(replicaRole)Benefits:
StoreResultconstructor changes (e.g. adding a 21st arg), only the factory updates instead of N test sites.throttled429()reads as a behavior, not a 20-arg ctor invocation).Scope
Test-only refactor; no production behavior change. Update existing
StoreResult-based tests in:sdk/cosmos/azure-cosmos-tests/.../directconnectivity/ConsistencyWriterTest.javasdk/cosmos/azure-cosmos-tests/.../directconnectivity/QuorumReaderTest.javaContext
Reviewer noted this is "not a blocker for [PR #48914] but worth scheduling as a follow-up before the next test wave lands on top."