Skip to content

apply us-east-1 as default region if not set by user in with_test_defaults() #4312

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

apply us-east-1 as default region if not set by user in with_test_defaults() #4312

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
eb60980
apply `us-east-1` as default region if not set by user in `with_test_…
aajtodd Sep 22, 2025
94f0eaf
fix codegen
aajtodd Sep 22, 2025
3fed2bf
remove unused region from test fake
aajtodd Sep 22, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions .changelog/1758554358.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
---
applies_to:
- client
- aws-sdk-rust
authors:
- aajtodd
references:
- smithy-rs#4265
- smithy-rs#4189
breaking: false
new_feature: false
bug_fix: true
---
Apply `us-east-1` as default region if not set by user in `with_test_defaults()` for all clients supporting region allowing `aws-smithy-mocks` to work for non AWS SDK generated clients. Also clarify `test-util` feature requirement when using `aws-smithy-mocks`.
11 changes: 11 additions & 0 deletions aws/codegen-aws-sdk/src/main/kotlin/software/amazon/smithy/rustsdk/RegionDecorator.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -205,6 +205,17 @@ class RegionProviderConfig(codegenContext: ClientCodegenContext) : ConfigCustomi
*codegenScope,
)
}
is ServiceConfig.DefaultForTests -> {
// this was added later, for backwards compat we only set a default if a region hasn't already been set by user
rustTemplate(
"""
if ${section.configBuilderRef}.config.load::<#{Region}>().is_none() {
self.set_region(#{Some}(#{Region}::new("us-east-1")));
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is it worth a BMV for this or other opt-out? I feel like there is a very high probability that someones unit test will be broken by this and although the fix won't be hard they might appreciate a way to hold back to keep their tests working

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Our own unit tests are broken by this and I'm not sure whether we want to ship this right now as we have introduced a precedence problem for defaults between with_test_defaults and endpoint builtins.

Generated test code:

// Code generated by software.amazon.smithy.rust.codegen.smithy-rs. DO NOT EDIT.
#![cfg(feature = "test-util")]
#[::tokio::test]
async fn operation_input_test_test_operation_1() {
    /* documentation: region should fallback to the default */
    /* builtIns: {} */
    /* clientParams: {} */
    let (http_client, rcvr) = ::aws_smithy_http_client::test_util::capture_request(None);
    let conf = {
        #[allow(unused_mut)]
        let mut builder = endpoint_test_service::Config::builder().with_test_defaults().http_client(http_client);
        builder.build()
    };
    let client = endpoint_test_service::Client::from_conf(conf);
    let _result = dbg!(
        client
            .test_operation()
            .set_bar(::std::option::Option::Some(
                endpoint_test_service::types::Bar::builder()
                    .set_f(::std::option::Option::Some("blah".to_owned()))
                    .build()
            ))
            .send()
            .await
    );
    let req = rcvr.expect_request();
    let uri = req.uri().to_string();
    assert!(
        uri.starts_with("https://prod.us-east-2.api.myservice.aws.dev/"),
        "expected URI to start with `https://prod.us-east-2.api.myservice.aws.dev/` but it was `{}`",
        uri
    );
}

failure:

running 1 test
test operation_input_test_test_operation_1 ... FAILED

failures:

---- operation_input_test_test_operation_1 stdout ----
[endpoint-test-service/rust-client-codegen/tests/endpoint_tests.rs:15:19] client.test_operation().set_bar(::std::option::Option::Some(endpoint_test_service::types::Bar::builder().set_f(::std::option::Option::Some("blah".to_owned())).build())).send().await = Ok(
    TestOperationOutput {
        _request_id: None,
    },
)

thread 'operation_input_test_test_operation_1' panicked at endpoint-test-service/rust-client-codegen/tests/endpoint_tests.rs:28:5:
expected URI to start with `https://prod.us-east-2.api.myservice.aws.dev/` but it was `https://prod.us-east-1.api.myservice.aws.dev/foo`
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace


failures:
    operation_input_test_test_operation_1

test result: FAILED. 0 passed; 1 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s

this test is generated by: https://github.com/smithy-lang/smithy-rs/blob/2016a67af3beeb4c976da9fb3d867f15822e[…]/amazon/smithy/rustsdk/endpoints/OperationInputTestGenerator.kt
Interestingly it shows nothing for builtins

but effectively it's relying on the default builtin value of us-east-2which it would not be now because with_test_defaults sets a different default when unset

}
""",
*codegenScope,
)
}

is ServiceConfig.BuilderFromConfigBag -> {
rustTemplate("${section.builder}.set_region(${section.configBag}.load::<#{Region}>().cloned());", *codegenScope)
Expand Down
8 changes: 4 additions & 4 deletions aws/rust-runtime/aws-config/Cargo.lock
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion rust-runtime/Cargo.lock
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion rust-runtime/aws-smithy-mocks/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[package]
name = "aws-smithy-mocks"
version = "0.1.2"
version = "0.2.0"
authors = ["AWS Rust SDK Team <[email protected]>"]
description = "Testing utilities for smithy-rs generated clients"
edition = "2021"
Expand Down
24 changes: 24 additions & 0 deletions rust-runtime/aws-smithy-mocks/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,30 @@ without mocking the entire client or using traits.
- **Response Sequencing**: Define sequences of responses for testing retry behavior
- **Rule Modes**: Control how rules are matched and applied

## Prerequisites

<div class="warning">
You must enable the `test-util` feature of the service client crate in order to use the `mock_client` macro.
Copy link
Contributor

Choose a reason for hiding this comment

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

I know its kind of implicit in the example below that test-util should only be enabled for [dev-dependencies], but should we make that explicit and warn users only to use those features for testing?

Copy link
Collaborator

Choose a reason for hiding this comment

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

This happens so frequently that I wonder if we should design a really nice error where the mock_client invokes a macro we define in the crates and if test-util isn't enabled it uses compile_fail! To give a precise error.

Can't do it here obviously since we'd need to add that macro first in the generated crates

</div>

If the feature is not enabled a compilation error similar to the following will occur:

```ignore
no method named with_test_defaults found for struct <service-client-crate>::config::Builder in the current scope
method not found in Builder
```

Example `Cargo.toml` using the `aws-sdk-s3` crate as the service client crate under test:

```toml
[dependencies]
aws-sdk-s3 = "1"

[test-dependencies]
Copy link
Contributor

Choose a reason for hiding this comment

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

[dev-dependencies]?

aws-smithy-mocks = "0.2"
aws-sdk-s3 = { version = "1", features = ["test-util"] }
```

## Basic Usage

```rust,ignore
Expand Down
4 changes: 3 additions & 1 deletion rust-runtime/aws-smithy-mocks/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,9 @@ macro_rules! mock {

/// `mock_client!` macro produces a Client configured with a number of Rules and appropriate test default configuration.
///
/// ## Prerequisites
/// You must enable the `test-util` feature of the service client crate in order to use the `mock_client` macro.
///
/// # Examples
///
/// **Create a client that uses a mock failure and then a success**:
Expand Down Expand Up @@ -143,7 +146,6 @@ macro_rules! mock_client {
coerce($additional_configuration)(
$aws_crate::config::Config::builder()
.with_test_defaults()
.region($aws_crate::config::Region::from_static("us-east-1"))
.http_client(mock_http_client)
.interceptor(mock_response_interceptor),
)
Expand Down
12 changes: 1 addition & 11 deletions rust-runtime/aws-smithy-mocks/tests/macros.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
* SPDX-License-Identifier: Apache-2.0
*/

/// Basic test of using the mock_client macro from an "external" crate
//! Basic test of using the mock_client macro from an "external" crate

mod fake_crate {
pub(crate) mod client {
Expand Down Expand Up @@ -32,9 +32,6 @@ mod fake_crate {
pub fn build(self) -> Config {
Config {}
}
pub fn region(self, _region: crate::fake_crate::config::Region) -> Self {
Self {}
}
pub fn with_test_defaults(self) -> Self {
Self {}
}
Expand All @@ -46,13 +43,6 @@ mod fake_crate {
self
}
}

pub(crate) struct Region {}
impl Region {
pub fn from_static(_region: &'static str) -> Self {
Self {}
}
}
}
}
#[test]
Expand Down
4 changes: 2 additions & 2 deletions tools/ci-build/publisher/Cargo.lock
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading