Skip to content
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.

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

Restore ENABLE_SMOKETESTS for running smoketests #3799

Merged
merged 1 commit into from
Aug 23, 2024

Conversation

ysaito1001
Copy link
Contributor

Motivation and Context

This PR restores an environment variable we attempted to add in #3758.

Description

We removed that environment variable since we ran into an error ENABLE_SMOKETESTS: unbound variable in our release pipeline. This PR will have a default value false when ENABLE_SMOKETESTS is not present.

We expect the value to be false in smithy-rs CI and true in our release pipeline (can set it to true in our release pipeline asynchronously).

Testing

  • Existing tests in CI

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

Copy link

A new generated diff is ready to view.

  • AWS SDK (ignoring whitespace)
  • No codegen difference in the Client Test
  • No codegen difference in the Server Test
  • No codegen difference in the Server Test Python
  • No codegen difference in the Server Test Typescript

A new doc preview is ready to view.

@ysaito1001 ysaito1001 marked this pull request as ready for review August 22, 2024 22:41
@ysaito1001 ysaito1001 requested review from a team as code owners August 22, 2024 22:41
@ysaito1001 ysaito1001 added this pull request to the merge queue Aug 23, 2024
Merged via the queue into main with commit d558415 Aug 23, 2024
44 checks passed
@ysaito1001 ysaito1001 deleted the ysaito/integrate-smoketest-in-release-pipeline branch August 23, 2024 17:27
github-merge-queue bot pushed a commit that referenced this pull request Aug 28, 2024
#3808)

## Motivation and Context
Follow-up on #3799

## Description
This PR updates the smoke test rendering process for services whose
models specify the `smokeTests` trait
([example](https://github.com/awslabs/aws-sdk-rust/blob/c349073b345a576d18ab7729afc7b75ae273d2ac/aws-models/sns.json#L3392-L3404)).
Previously, `SmokeTestsDecorator` skipped rendering when
`useAccountIdRouting` was set to true in the vendor parameters, which is
the default for `AwsVendorParams`. Even though the Rust SDK does not
currently support the account ID routing, it is safe to render these
tests. This is because existing smoke tests do not rely on this feature,
and when account ID routing is used, the Rust SDK will fall back to
other means to sourcing the identity.

This PR includes a minor fix: it uses `aws_config` to load default
configurations. `config::Builder::new()` would have no credentials
provider chain available, which would then result in test failures at
runtime.
```
---- test_list_certificates_success stdout ----
thread 'test_list_certificates_success' panicked at sdk/acm/tests/smoketests.rs:22:9:
request should succeed: DispatchFailure(DispatchFailure { source: ConnectorError { kind: Other(None), source: NoMatchingAuthSchemeError(ExploredList { items: [ExploredAuthOption { scheme_id: AuthSchemeId { scheme_id: "sigv4" }, result: NoIdentityResolver }], truncated: false }), connection: Unknown } })
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
```

By bringing `aws-config`, the `SmokeTestsDecoratorTest` no longer works,
since the AWS runtime crates required by `aws-config` conflict with
those generated by `awsSdkIntegrationTest` (a known limitation). For
example:
```
error: package collision in the lockfile: packages aws-credential-types v1.2.0 (smithy-rs/aws/rust-runtime/aws-credential-types) and aws-credential-types v1.2.0 (smithy-rs/aws/sdk/build/aws-sdk/sdk/aws-credential-types) are different, but only one can be written to lockfile unambiguously
```
Therefore, we removed `SmokeTestsDecoratorTest.kt` _only_ in this PR to
ship the feature, but are planning to restore it in the next PR by
directly testing `SmokeTestsInstantiator` without using
`awsSdkIntegrationTest`.

## Testing
- [x] Existing tests in CI
- [x] Executed and passed smoke tests for services that support them in
our release pipeline

----

_By submitting this pull request, I confirm that you can use, modify,
copy, and redistribute this contribution, under the terms of your
choice._
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants