smithy-lang / smithy-rs

Code generation for the AWS SDK for Rust, as well as server and generic smithy client generation.
Apache License 2.0
486 stars 185 forks source link

Server protocol tests extras #1164

Closed david-perez closed 2 months ago

david-perez commented 2 years ago

The client has suites like rest-json-extras.smithy that contains extra protocol tests, some of which are upstreamed in the awslabs/smithy repo and will be hosted there in the next Smithy release.

Ideally the server should also pass these suites, and we should put there additional tests we come up with.

Fixed tests should also go there, instead of hot-patching them at runtime like we currently do. Advantages:

david-perez commented 2 years ago

A first test we should add to not regress on #1163 (but to the extras suite, not to simple.smithy like the diff below shows):

diff --git a/codegen-server-test/model/simple.smithy b/codegen-server-test/model/simple.smithy
index 70dee327..2feae1d2 100644
--- a/codegen-server-test/model/simple.smithy
+++ b/codegen-server-test/model/simple.smithy
@@ -63,7 +63,10 @@ resource Service {
         id: "RegisterServiceResponseTest",
         protocol: "aws.protocols#restJson1",
         params: { id: "1", name: "TestService" },
-        body: "{\"id\":\"1\",\"name\":\"TestService\"}",
+        headers: {
+            "Content-Type": "TestService",
+        },
+        body: "{\"id\":\"1\"}",
         code: 200,
     }
 ])
@@ -85,6 +88,9 @@ structure RegisterServiceInputRequest {
 structure RegisterServiceOutputResponse {
     @required
     id: ServiceId,
+
+    @required
+    @httpHeader("Content-Type")
     name: ServiceName,
 }
david-perez commented 2 years ago

Another one: httpQueryParams duplicate query keys behavior: https://github.com/awslabs/smithy/issues/1071

drganjoo commented 3 months ago

We can now remove the only remaining broken test as it has been fixed upstream

david-perez commented 2 months ago

I'm going to close this since I no longer stand by this:

Fixed tests should also go there, instead of hot-patching them at runtime like we currently do

Now that we have https://github.com/smithy-lang/smithy-rs/pull/3726/, there are no advantages to putting broken protocol tests in the <protocol>-extras.smithy models.