chore: Use client to perform HTTP response tests #1018
Conversation
| /// Calls to CRT functions may crash the SDK if `CommonRuntimeKit.initialize()` is not called first. | ||
| /// | ||
| /// This function may safely be called multiple times. | ||
| public func initialize() { CommonRuntimeKit.initialize() } |
There was a problem hiding this comment.
Some protocol tests were crashing because they were calling CRT before it had been initialized. We have seen this problem crop up from time to time when customers swap out our CRT-based implementations of things for mocks or for their own.
The issue is that we typically initialize CRT at the point of use, but not all points of use. So if we remove a config component that is initializing CRT, then the first call might get made from another component that doesn't initialize CRT but was depending on the removed one to do it. That results in a crash.
To fix this problem once & for all, we'll be calling this during initialization of any client, so that we're assured CRT is already initialized before any operation is started.
|
|
||
| import protocol SmithyHTTPAPI.HTTPClient | ||
| import class SmithyHTTPAPI.HTTPRequest | ||
| import class SmithyHTTPAPI.HTTPResponse |
There was a problem hiding this comment.
Below is a simple mock that conforms to the HTTPClient protocol. Rather than making a request it simply returns the response it was created with.
It will be used to initialize the client when performing HTTP response & error protocol tests.
|
|
||
| open fun renderInitFunction() { | ||
| writer.openBlock("public required init(config: \$L) {", "}", serviceConfig.typeName) { | ||
| writer.write("\$N()", ClientRuntimeTypes.Core.initialize) |
There was a problem hiding this comment.
This is the call to ClientRuntime.initialize() that will ensure CRT is initialized before any operation is called.
| builder: Builder, | ||
| ) : HttpProtocolUnitTestResponseGenerator(builder) { | ||
| val error: Shape = builder.error ?: throw CodegenException("builder did not set an error shape") | ||
| override val outputShape: Shape? = error |
There was a problem hiding this comment.
This override is confusing because the error is not an output shape; these terms have specific meaning. Better to just use the error property (just above) directly.
| writer.write("") | ||
| renderActualOutput() | ||
| writer.write("") | ||
| renderCompareActualAndExpectedErrors(test, error) |
There was a problem hiding this comment.
Above, we rendered a superfluous do-catch to wrap the error test & fail on an error, it's useless because XCTest fails on an uncaught error anyway.
| val errorType = symbolProvider.toSymbol(errorShape).name | ||
|
|
||
| writer.openBlock("if let actual = \$L as? \$L {", "} else {", operationErrorVariableName, errorType) { | ||
| writer.openBlock("if let actual = operationError as? \$L {", "} else {", errorType) { |
There was a problem hiding this comment.
Here & elsewhere in this file, operationErrorVariableName is deleted and the name operationError is used in all cases.
Trying to use a different name per-test for the variable holding the error adds complexity for no gain.
|
|
||
| private fun renderActualOutput(outputStruct: Symbol) { | ||
| val responseClosure = ResponseClosureUtils(ctx, writer, operation).render() | ||
| writer.write("let actual: \$N = try await \$L(httpResponse)", outputStruct, responseClosure) |
There was a problem hiding this comment.
The code above accessed client internals to process the HTTP response into an output.
It is replaced with a specially configured client, see below.
| let serviceName = "Rest Json Protocol" | ||
| public required init(config: RestJsonProtocolClient.RestJsonProtocolClientConfiguration) { | ||
| ClientRuntime.initialize() |
There was a problem hiding this comment.
Here & below are codegen test changes.
| "let actual = try await client.\$L(input: input)", | ||
| operation.toLowerCamelCase(), | ||
| ) | ||
| } |
There was a problem hiding this comment.
For an output test, captureResponse() renders a call to the operation and stores the result for verification.
Error tests override this method to catch and store the thrown error for test verification.
2 participants
Description of changes
Makes HTTP response & error protocol tests independent of internal implementation by using a specially configured client to perform the tests, similar to what we already do for request tests.
This change will allow schema-based services and services using current ReadWrite serde to use the same protocol tests, since the test no longer utilizes client internals.
Specific changes:
httpResponsewhen called.Also:
Scope
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.