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

additional service specific protocol tests #3682

Merged
merged 2 commits into from
Jun 6, 2024
Merged

Conversation

aajtodd
Copy link
Contributor

@aajtodd aajtodd commented Jun 5, 2024

Motivation and Context

There are several service specific http request/response tests from Smithy. These are similar to the protocol tests but apply only to a specific service. We were tasked with ensuring some of the S3 URI related tests were captured.

Description

We currently were running the API gateway related tests as part of the aws/sdk-adhoc-test package. There are also tests for machinelearning, glacier, and s3. I attempted to add all of these but hit issues.

  1. machinelearning can be enabled but we have yet to actual implement the customization that would allow this test to pass.
  2. s3 pulls in aws-config as a dev dependency for reasons and this causes conflicts between relocated and non-relocated runtime crates. The sdk-adhoc-test package does not implement any of the runtime relocation and Cargo.toml processing that the sdk build does (nor do I want to pursue that).

For now I've enabled the glacier tests and copied the S3 specific tests into s3-tests.smithy so that they are built and tested with the actual S3 model. In the future we can either (1) fix the build issues allowing us to remove these tests in favor of the ones defined in Smithy or (2) ideally these tests get implemented in the real upstream service models and we don't have to do anything special to get them.

Testing

Tested locally and CI


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

@aajtodd aajtodd requested a review from a team as a code owner June 5, 2024 18:57
Copy link

github-actions bot commented Jun 5, 2024

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.

Copy link
Contributor

@ysaito1001 ysaito1001 left a comment

Choose a reason for hiding this comment

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

This doesn't need to be copied to s3-tests.smithy either because it's already existing or irrelevant (just to better understand)?

@aajtodd aajtodd added this pull request to the merge queue Jun 6, 2024
@aajtodd aajtodd removed this pull request from the merge queue due to a manual request Jun 6, 2024
@aajtodd
Copy link
Contributor Author

aajtodd commented Jun 6, 2024

This doesn't need to be copied to s3-tests.smithy either because it's already existing or irrelevant (just to better understand)?

These are all endpoint related tests that predate EP2.0. There should be EP2.0 tests in S3's model that cover these and more at this point.

@aajtodd aajtodd added this pull request to the merge queue Jun 6, 2024
Merged via the queue into main with commit 1310a3c Jun 6, 2024
44 checks passed
@aajtodd aajtodd deleted the atodd/protocol-tests branch June 6, 2024 13:23
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