Update Smithy version dependency. #1623
Conversation
DavidOgunsAWS
force-pushed
the
smithyidl2
branch
from
4d61b45
to
bca2ef7
Compare
|
A new generated diff is ready to view.
A new doc preview is ready to view. |
...ftware/amazon/smithy/rust/codegen/server/smithy/generators/ServerHttpSensitivityGenerator.kt
Outdated
Show resolved
Hide resolved
|
@DavidOgunsAWS You just have to remove those two tests from the list of tests that we expect to fail, and CI should pass. The tests were fixed upstream by @82marbag in smithy-lang/smithy#1319, so with this PR, they now pass. I noted this in #1590 (review). |
DavidOgunsAWS
force-pushed
the
smithyidl2
branch
from
4e945c5
to
51e4d0c
Compare
...t/kotlin/software/amazon/smithy/rust/codegen/server/smithy/protocols/EventStreamTestTools.kt
Show resolved
Hide resolved
| @@ -61,7 +61,7 @@ structure CreateFooInput {} | |||
| </PrimitiveIntDocument> | |||
| """, | |||
| bodyMediaType: "application/xml", | |||
| params: { value: 1 } | |||
| params: { value: 1, requiredValue: 0 } | |||
There was a problem hiding this comment.
The test description is: "Primitive ints should not be serialized when they are unset", so adding requiredValue to the params seems to break what is being tested. Are these tests still valid with Smithy IDL 2?
There was a problem hiding this comment.
Primitives are now just the type with a default value backing it. And default values should be serialized by the server, not by the client.
Perhaps this test needs refinement or is there an implementation & test change needed here?
There was a problem hiding this comment.
I think the model for this test needs to be revised because it seems like the test isn't testing what it intended to test anymore. It seems like the requiredValue should get a default set on it rather than providing a value for it in the test case.
There was a problem hiding this comment.
The change I made was on the basis of understanding that old behavior around primitives and @required were kind of a wild west -- apparently not just for nullability, but also serialization. @default in 2.0 is where the behavior starts to be defined and predicted so I upgraded the test model to 2.0 and used that trait instead.
I don't believe there is anything further to be tested.
|
A new generated diff is ready to view.
A new doc preview is ready to view. |
… Fix function added.
…r and default value serialization.
Co-authored-by: John DiSanti <[email protected]>
DavidOgunsAWS
force-pushed
the
smithyidl2
branch
from
68de52c
to
945a21a
Compare
Motivation and Context
Description
Testing
1.23.0using./gradlew clean build publishToMavenLocal smithy-cli:runtime./gradlew --refresh-dependencies clean codegen-test:test aws:sdk:assembleTo success.
Checklist
CHANGELOG.next.tomlif I made changes to the smithy-rs codegen or runtime cratesCHANGELOG.next.tomlif I made changes to the AWS SDK, generated SDK code, or SDK runtime cratesBy submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.