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

Add rest-json protocol test updates #924

Merged

Conversation

nateprewitt
Copy link
Contributor

These changes are to help codify current Smithy behavior and ensure all generated clients are conformant with expectations for the rest-json protocol. Each test should have an explanation of what it's trying to achieve and why that's relevant to clients generated to Smithy.

Added checks in http-content-type.smithy:

  • Validating what an empty body looks like when handling structured and unstructured input.
  • Validating correct Content-Type headers are sent with payloads and can be overwritten with members targeting an HTTP header.
  • Validating methods that should not serialize a body are correctly omitting relevant headers.

From initial testing, Smithy clients appear to already follow these requirements but they aren't formally specified in any single test suite. Some of these checks are redundant with tests from http-payload.smithy and http-headers.smithy, but have been added to this file to reduce cognitive load when trying to understand how a client behaves in these cases.

@jasdel
Copy link
Contributor

jasdel commented Oct 5, 2021

I think the main.smithy file also needs to be updated to include the new TestBodyStructure operation in the service's set of operations.

https://github.com/awslabs/smithy/blob/f60c17e9b85181d868a55cba8536ea3ac240ac13/smithy-aws-protocol-tests/model/restJson1/main.smithy#L19

In addition, I think that some of these tests duplicate, or are very similar to, some of the existing HttpPayload* operation tests. If these are all new maybe it would be better to be named similar to HttpPayloadContent, and grouped with the other @httpPayload tests.

@nateprewitt nateprewitt force-pushed the rest-json-content-types branch from 69db653 to b31691f Compare October 5, 2021 21:46
@nateprewitt nateprewitt marked this pull request as ready for review October 5, 2021 22:59
@nateprewitt nateprewitt requested a review from a team as a code owner October 5, 2021 22:59
These changes are to help codify current Smithy behavior and
ensure all generated clients are conformant with expectations
for the rest-json protocol.

This includes:
* Validating what an empty body looks like when
  handling structured and unstructured input.
* Validating correct Content-Type headers are sent
  with payloads and can be overwritten with members
  targeting an HTTP header.
* Validating methods that should not serialize a body are
  correctly omitting relevant headers.
@nateprewitt nateprewitt force-pushed the rest-json-content-types branch from b31691f to 368f817 Compare October 5, 2021 23:05
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.

5 participants