Repository navigation
Replace live-AWS sibling tests in multi_service_engine_versions_test.go (followup to #1225) #1328
Description
Background
PR #1225 (TEST-02 fix) replaces TestQueryMajorEngineVersions_Success and _ProfileHandling in cmd/multi_service_coverage_test.go with hermetic stubbed-client tests and a deterministic profile-precedence test. The PR body explicitly leaves two sibling tests untouched because they were not cited in the TEST-02 review finding:
TestQueryMajorEngineVersions_ErrorHandlingincmd/multi_service_engine_versions_test.go:635-682TestQueryRunningInstanceEngineVersions_ErrorHandlingin same file:684-...
Both have the same defect shape PR #1225 fixed:
- They call
queryMajorEngineVersions(ctx, tt.cfg)/queryRunningInstanceEngineVersions(ctx, tt.cfg)-- which load real AWS config and issue real API calls on credentialed machines. - They guard the assertion behind
if err != nil-- so the test passes whether AWS creds exist or not. Reading the comment in_ErrorHandling: "We expect an error in test environment (no real AWS creds). The important thing is that the function doesn't panic." That is exactly theassert.NotPanicsanti-pattern TEST-02 flagged. - No
testing.Short()guard, so they run under CI'sgo test -short ./...and would hit AWS on a credentialed runner.
Fix
Mirror the PR #1225 approach:
-
Profile-precedence test: convert to a deterministic config-load-fails test using
t.Setenv("AWS_CONFIG_FILE", os.DevNull)+ a profile name guaranteed not to exist, and assert the error names the selected profile. TheisolateAWSEnvhelper added in PR test(cmd): make CSV purchase-path tests assert real behavior #1225 is reusable. -
Engine-versions error-handling: drop the live-config test in favour of stubbed-client tests of
queryMajorEngineVersionsWithClient(already exists in this PR) -- the_Successand_EngineErrorContinuestests from PR test(cmd): make CSV purchase-path tests assert real behavior #1225 already cover most of the surface; add a missing-permissions case (stub returns AccessDenied) to confirm the warn-and-continue path. -
Running-instance error-handling:
queryRunningInstanceEngineVersionsdoes not yet have a*WithClienttestable core. Extract one along the lines of:type RDSInstancesClient interface { DescribeDBInstances(ctx context.Context, ...) (*awsrds.DescribeDBInstancesOutput, error) } func queryRunningInstanceEngineVersionsWithClient(ctx context.Context, c RDSInstancesClient, regions []string) (map[string][]InstanceEngineVersion, error)
Then a stubbed test asserts the per-region fan-out, the warn-and-continue contract for transient errors, and (paired with the followup on
feedback_ctx_cancel_terminal.md) ctx-cancel termination.
Why now
Closing this gap removes the last two live-AWS tests in cmd/multi_service_engine_versions_test.go and gives the engine-versions code path the same test isolation guarantees the rest of the engine-versions surface received in PR #1225. Eliminates the false-green failure mode on credentialed dev laptops.
Triage
P2 / sev medium / impact internal / effort s -- ~120 LOC test refactor, no production code change beyond extracting a one-method *WithClient core.
Activity