Run HttpMalformedRequest tests through router #1904
Conversation
…egen/server/smithy/generators/protocol/ServerProtocolTestGenerator.kt Co-authored-by: david-perez <d@vidp.dev>
This causes HttpMalformedRequest tests to fail.
| @@ -842,7 +846,7 @@ class ServerProtocolTestGenerator( | |||
| private fun assertOk(rustWriter: RustWriter, inner: Writable) { | |||
| rustWriter.rust("#T(", RuntimeType.ProtocolTestHelper(codegenContext.runtimeConfig, "assert_ok")) | |||
| inner(rustWriter) | |||
| rustWriter.rust(");") | |||
| rustWriter.write(");") | |||
There was a problem hiding this comment.
Using rustWriter.rust here caused IntelliJ to report an error because ); is not a valid rust snippet.
|
A new generated diff is ready to view.
A new doc preview is ready to view. |
| .get::<#{SmithyHttpServer}::extension::OperationExtension>() | ||
| .expect("extension `OperationExtension` not found"); | ||
| #{AssertEq}(operation_extension.absolute(), operation_full_name); | ||
| // let operation_extension = http_response.extensions() |
There was a problem hiding this comment.
This check ensures that the request has been successfully been routed by checking for OperationExtension set by the Handler implementation - looks like we're missing this assertion now?
This assertion won't succeed if the request fails to parse, which is why I guess it's commented out here?
There was a problem hiding this comment.
Yes, I was figuring out how that worked. I've re-added it for (non-malformed) request tests here: db95341.
There was a problem hiding this comment.
The equivalent check in checkRequest2 is done by a channel fired in the handler.
There was a problem hiding this comment.
Fixed in 6e1d747. I attempted to unify both approaches (checking operation extension vs using a channel in the handler) but ran into many type errors in the way.
I decided to revisit this later.
|
A new generated diff is ready to view.
A new doc preview is ready to view. |
- Remove `checkRequest`, `checkRequest2` in favour of using `makeRequest`/`makeRequest2` directly - Run service builder tests for MalformedHttpRequest tests
|
A new generated diff is ready to view.
A new doc preview is ready to view. |
This test actually passes fine, but is not relevant for the server.
|
A new generated diff is ready to view.
A new doc preview is ready to view. |
jjant
force-pushed
the
jjant/run-tests-against-services
branch
from
742216a to
fb11a3e
Compare
|
A new generated diff is ready to view.
A new doc preview is ready to view. |
|
A new generated diff is ready to view.
A new doc preview is ready to view. |
|
A new generated diff is ready to view.
A new doc preview is ready to view. |
…y-rs into jjant/run-tests-against-services
…/smithy-rs into jjant/run-tests-against-services
|
A new generated diff is ready to view.
A new doc preview is ready to view. |
|
A new generated diff is ready to view.
A new doc preview is ready to view. |
Marks these two tests as failing: - RestJsonWithPayloadExpectsImpliedContentType - RestJsonBodyMalformedMapNullKey These will be fixed in smithy-lang/smithy#1477.
|
A new generated diff is ready to view.
A new doc preview is ready to view. |
Motivation and Context
Addresses #1212.
Testing
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.