Move tests to aws-smithy-mocks

[simonmarty]

Issue #, if available:

Description of changes: Accidentally closed #112 on a force push.

Move unit tests to aws-smithy-mocks for easier test coverage.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

[codecov]

Codecov Report

❌ Patch coverage is 97.55556% with 11 lines in your changes missing coverage. Please review.
✅ Project coverage is 86.35%. Comparing base (2d744d7) to head (6278eab).

Files with missing lines	Patch %	Lines
aws_secretsmanager_caching/src/lib.rs	97.55%	9 Missing and 2 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #119      +/-   ##
==========================================
+ Coverage   86.32%   86.35%   +0.02%     
==========================================
  Files          34       34              
  Lines        9369     9469     +100     
  Branches     9369     9469     +100     
==========================================
+ Hits         8088     8177      +89     
- Misses       1148     1156       +8     
- Partials      133      136       +3     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[secrets-manager-ai-code-review]

Claude finished @simonmarty's task in 3m 13s —— View job

---

PR Review: Move tests to aws-smithy-mocks

• Gather context (diff, repo layout, CI)
• Analyze the test migration changes
• Review for bugs, quality, and prior-review follow-ups
• Post review feedback

This is a clean, well-executed migration from the hand-rolled infallible_client_fn / WireMockServer mocks to aws-smithy-mocks. Tests are more declarative, request matchers make intent explicit, and num_calls() assertions add real value (they verify caching/refresh behavior that the old string-matching mock couldn't). The earlier review feedback (KMS error content assertions, HTTP-response-based errors, and the assert_is_current_fast_refreshes helper) all look properly addressed. CI (Rust, Docker, Benchmarks) is green.

A few things worth resolving before merge:

🔴 batch_get_secret_value lost all test coverage

The four batch tests were removed and not ported to the new mock harness:

• test_batch_get_secret_value_success
• test_batch_get_secret_value_partial_failure
• test_batch_get_secret_value_api_error
• test_batch_get_secret_value_transient_suppressed

The production method batch_get_secret_value (aws_secretsmanager_caching/src/lib.rs:303) still exists, so this is a straight coverage regression — including the notable partial-failure path (some secrets succeed, some error) and the transient-error-suppression path, which are exactly the behaviors most worth guarding. This lines up with the Codecov report flagging 9 missing lines. Since aws-smithy-mocks can return a BatchGetSecretValueOutput with populated errors(), these are portable. If dropping them is intentional (e.g. deferred to a follow-up), it'd be good to say so explicitly. Fix this →

🟡 ignore_transient_errors semantics changed for several ported tests

The old fake_client(None, false, …) passed ignore_transient_errors = false; most of the migrated tests now hardcode true (e.g. test_get_secret_value, the test_get_secret_value_refresh_now_* tests). For the happy-path GSV tests this is harmless since no transient error is injected, but the flag flip is silent. Worth a quick confirmation that no test intended to exercise false behavior, and consider keeping false where transient handling isn't under test so the setting reflects intent.

🟢 Minor / nits

• Partial migration is reasonable. test_is_current_gsv_timeout_error_succeeds and test_is_current_describe_timeout_error_succeeds remain on WireMockServer under the new wire_tests submodule — sensible, since smithy-mocks can't easily simulate a connection timeout. Keeping GSV_BODY/DESC_BODY/def_fake_client scoped to that module is clean.
• Weaker matcher in test_get_secret_value_version_id_and_stage_refresh_now: the gsv matcher only checks secret_id, unlike the sibling non-refresh test which also matches version_id/version_stage. Not wrong, just slightly less strict than its neighbors — tightening it would keep the pattern consistent.
• test_is_current_describe_service_error_succeeds doesn't assert num_calls() like the other is_current tests do; adding it would confirm the cached value (not a re-fetch) is being served on the service-error path.

Overall this is a solid improvement in test readability and rigor. The batch coverage gap is the one item I'd treat as blocking.