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.

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

Add server extra tests #1497

Merged
merged 8 commits into from
Aug 24, 2022
Merged

Add server extra tests #1497

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
0e8fa80
Add server extra tests
82marbag Jun 24, 2022
deb7aec
add symlink
82marbag Jun 24, 2022
d4e9c57
address comments
82marbag Jun 27, 2022
6c21fb0
Merge branch 'main' into fix-tests-3
82marbag Aug 23, 2022
f9ff234
Address comments
Aug 23, 2022
dc3dda0
Address comment
82marbag Aug 23, 2022
ea49afc
Merge branch 'main' into fix-tests-3
82marbag Aug 23, 2022
4dd0c39
Merge branch 'main' into fix-tests-3
82marbag Aug 24, 2022
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
1 change: 1 addition & 0 deletions codegen-server-test/build.gradle.kts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ val allCodegenTests = listOf(
CodegenTest("naming_obs_structs#NamingObstacleCourseStructs", "naming_test_structs"),
CodegenTest("com.amazonaws.simple#SimpleService", "simple"),
CodegenTest("aws.protocoltests.restjson#RestJson", "rest_json"),
CodegenTest("aws.protocoltests.restjson#RestJsonExtras", "rest_json_extras"),
CodegenTest("aws.protocoltests.restjson.validation#RestJsonValidation", "rest_json_validation"),
CodegenTest("aws.protocoltests.json10#JsonRpc10", "json_rpc10"),
CodegenTest("aws.protocoltests.json#JsonProtocol", "json_rpc11"),
Expand Down
1 change: 1 addition & 0 deletions codegen-server-test/model/rest-json-extras.smithy
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
../../codegen-test/model/rest-json-extras.smithy
36 changes: 26 additions & 10 deletions codegen-test/model/rest-json-extras.smithy
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,16 +73,26 @@ service RestJsonExtras {
@httpResponseTests([
{
documentation: "Upper case error modeled lower case",
id: "ServiceLevelError",
id: "ServiceLevelErrorClient",
protocol: "aws.protocols#restJson1",
code: 500,
body: "",
headers: { "X-Amzn-Errortype": "ExtraError" },
params: {}
params: {},
appliesTo: "client",
Copy link
Contributor

@david-perez david-perez Aug 23, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

This test fails for the server because servers are expected to serialize {} when the output parameters are empty.

Hence why we have a serverStructureSerializer that behaves like that. We should arguably modify the Kotlin docs for serverStructureSerializer calling this out.

Anyway, this test can remain for the client only, but we should duplicate it in a new test that only applies to the server and that has body: "{}", and copy over the documentation from RestJsonGreetingWithErrors to justify why we're doing so.

},
{
documentation: "Upper case error modeled lower case",
id: "ServiceLevelErrorServer",
protocol: "aws.protocols#restJson1",
code: 500,
body: "{}",
headers: { "X-Amzn-Errortype": "ExtraError" },
params: {},
appliesTo: "server",
}
])
@error("server")
82marbag marked this conversation as resolved.
Show resolved Hide resolved
@error("server")
structure ExtraError {}

@http(uri: "/StringPayload", method: "POST")
Expand Down Expand Up @@ -113,7 +123,8 @@ structure StringPayloadInput {
uri: "/primitive-document",
method: "POST",
body: "{}",
params: {}
headers: { "Content-Type": "application/json" },
params: {},
}])
@http(uri: "/primitive-document", method: "POST")
operation PrimitiveIntOp {
Expand All @@ -131,14 +142,14 @@ structure PrimitiveIntDocument {
protocol: "aws.protocols#restJson1",
code: 200,
headers: { "x-field": "123" },
params: { field: 123 }
params: { field: 123 },
},
{
id: "DeserPrimitiveHeaderMissing",
protocol: "aws.protocols#restJson1",
code: 200,
headers: { },
params: { field: 0 }
params: { field: 0 },
}
])
@http(uri: "/primitive", method: "POST")
Expand Down Expand Up @@ -201,7 +212,8 @@ structure MapWithEnumKeyInputOutput {
method: "POST",
protocol: "aws.protocols#restJson1",
body: "{\"map\":{\"enumvalue\":\"something\"}}",
params: { map: { "enumvalue": "something" } }
headers: { "Content-Type": "application/json" },
params: { map: { "enumvalue": "something" } },
},
])
@httpResponseTests([
Expand Down Expand Up @@ -239,6 +251,7 @@ structure EscapedStringValuesInputOutput {
method: "POST",
protocol: "aws.protocols#restJson1",
body: "{\"enum\":\"has\\\"quotes\",\"also\\\"has\\\"quotes\":\"test\"}",
headers: { "Content-Type": "application/json" },
params: { enum: "has\"quotes", someString: "test" },
}
])
Expand Down Expand Up @@ -283,6 +296,7 @@ structure NullInNonSparseOutput {
code: 200,
body: "{\"list\":[null,\"one\",null,\"two\",null],\"map\":{\"zero\":null,\"one\":\"1\"}}",
params: { list: ["one", "two"], map: { "one": "1" } },
appliesTo: "client",
}
])
operation NullInNonSparse {
Expand All @@ -296,13 +310,14 @@ operation CaseInsensitiveErrorOperation {

@httpResponseTests([
{
documentation: "Upper case error modeled lower case",
documentation: "Upper case error modeled lower case. See: https://github.com/awslabs/smithy-rs/blob/6c21fb0eb377c7120a8179f4537ba99a4b50ba96/rust-runtime/inlineable/src/json_errors.rs#L51-L51",
id: "UpperErrorModeledLower",
protocol: "aws.protocols#restJson1",
code: 500,
body: "{\"Message\": \"hello\"}",
headers: { "X-Amzn-Errortype": "CaseInsensitiveError" },
params: { message: "hello" }
params: { message: "hello" },
appliesTo: "client",
Copy link
Contributor

Choose a reason for hiding this comment

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

I would expand the documentation to justify why this is a client-only test; took me a while to figure out why. It turns out that smithy-rs clients should be able to parse error messages if the JSON keys them as message, Message, or errorMessage. The relevant parsing code is here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added a comment with the link

}
])
@error("server")
Expand All @@ -322,7 +337,8 @@ structure EmptyStructWithContentOnWireOpOutput {
protocol: "aws.protocols#restJson1",
code: 200,
body: "{\"empty\": {\"value\":\"not actually empty\"}}",
params: { empty: {} }
params: { empty: {} },
appliesTo: "client",
}
])
operation EmptyStructWithContentOnWireOp {
Expand Down