Test-quality nits raised by the 3-axis review of #1451 that were deliberately
NOT fixed there — each is a strengthening of existing green tests, not a
correctness gap, and none was worth restarting that PR's CI + real-AWS integ
cycle for. Filing so they are not silently dropped.
All in tests/unit/provisioning/dynamodb-globaltable-provider-index-autoscaling.test.ts
unless noted.
1. The cross-region create test asserts only the client region
routes a cross-region replica read target through that replica region client
checks autoScalingRegionSpy was called with eu-west-1 and nothing else. It
would pass if the register carried the wrong ResourceId, dimension, or
Min/Max. Assert the RegisterScalableTargetCommand input, the way the
local-region tests do.
2. No ordering assertions
Two load-bearing ordering claims are documented in code comments but pinned by
nothing:
create() registers only after the table AND every replica are ACTIVE;
- step 6b runs after step 6, so a GSI added by the same deploy exists.
The DynamoDB and application-autoscaling mocks are separate vi.fn()s, so
ordering is invisible to per-mock assertions. mock.invocationCallOrder spans
mocks and would pin both.
3. describeOnceThenGone hides the waitForTableGone poll
The delete-path helper serves one DescribeTable then raises RNF, so the
teardown tests never exercise more than one poll iteration. Fail-safe (a
second describe yields an empty index list and the assertions fail loudly), but
it means the poll loop itself is uncovered on this path.
4. verify.sh literal-tab comparisons
tests/integration/dynamodb-globaltable/verify.sh compares
--output text results against literal "2\t20" / "7\t70" strings. Correct
today, but brittle against any AWS CLI output-formatting change. Per-value
queries (one --query per field) would be formatter-proof.
Not included
toSdkReplicaGlobalSecondaryIndexes taking no CapacitySource was also raised
and is deliberately closed as won't-do (recorded in #1451's body): the helper is
not reached on the billing-flip path, so the seed context cannot apply to it.
Test-quality nits raised by the 3-axis review of #1451 that were deliberately
NOT fixed there — each is a strengthening of existing green tests, not a
correctness gap, and none was worth restarting that PR's CI + real-AWS integ
cycle for. Filing so they are not silently dropped.
All in
tests/unit/provisioning/dynamodb-globaltable-provider-index-autoscaling.test.tsunless noted.
1. The cross-region create test asserts only the client region
routes a cross-region replica read target through that replica region clientchecks
autoScalingRegionSpywas called witheu-west-1and nothing else. Itwould pass if the register carried the wrong
ResourceId, dimension, orMin/Max. Assert the
RegisterScalableTargetCommandinput, the way thelocal-region tests do.
2. No ordering assertions
Two load-bearing ordering claims are documented in code comments but pinned by
nothing:
create()registers only after the table AND every replica are ACTIVE;The DynamoDB and application-autoscaling mocks are separate
vi.fn()s, soordering is invisible to per-mock assertions.
mock.invocationCallOrderspansmocks and would pin both.
3.
describeOnceThenGonehides thewaitForTableGonepollThe delete-path helper serves one
DescribeTablethen raises RNF, so theteardown tests never exercise more than one poll iteration. Fail-safe (a
second describe yields an empty index list and the assertions fail loudly), but
it means the poll loop itself is uncovered on this path.
4.
verify.shliteral-tab comparisonstests/integration/dynamodb-globaltable/verify.shcompares--output textresults against literal"2\t20"/"7\t70"strings. Correcttoday, but brittle against any AWS CLI output-formatting change. Per-value
queries (one
--queryper field) would be formatter-proof.Not included
toSdkReplicaGlobalSecondaryIndexestaking noCapacitySourcewas also raisedand is deliberately closed as won't-do (recorded in #1451's body): the helper is
not reached on the billing-flip path, so the seed context cannot apply to it.