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.

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

Swift smithy request unit test generation #50

Merged

Conversation

phani-srikar
Copy link
Contributor

@phani-srikar phani-srikar commented Aug 29, 2020

Description of changes:
This PR follows 48 which covered ClientRuntime changes necessary to support http request unit testing.
This PR defines the base class HttpProtocolTestGenerator which is responsible for generating the request, response and error object unit tests. It also defines a HttpProtocolUnitTestGenerator generic class which is then specialized to handle the request/response test cases. The HttpRequest test cases are handled through HttpProtocolUnitTestRequestGenerator. Similar specialized class for response tests will be added later.
This PR also includes a ShapeValueGenerator which renders instances of any shape. Unit tests are included for the same.
Minor ClientRuntime changes to support this PR are also included.
The tests generator are not yet run (few compile errors in generated service client that need to be fixed), they do compile successfully.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

.indent()
.call { block() }
.dedent()
// TODO:: fix indentation when `writeInline` retains indent
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I believe this PR would solve this? @aajtodd

Copy link
Contributor

Choose a reason for hiding this comment

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

yes presumably it will, last I checked it hadn't made it into a release though.

Copy link
Contributor

@aajtodd aajtodd left a comment

Choose a reason for hiding this comment

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

Looks pretty good overall. Couple small things but otherwise LGTM

.indent()
.call { block() }
.dedent()
// TODO:: fix indentation when `writeInline` retains indent
Copy link
Contributor

Choose a reason for hiding this comment

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

yes presumably it will, last I checked it hadn't made it into a release though.


private fun primitiveDecl(writer: SwiftWriter, shape: Shape, block: () -> Unit) {
var suffix = ""
when (shape.type) {
Copy link
Contributor

Choose a reason for hiding this comment

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

val suffix = when(shape.type) {
    ShapeType.STRING -> { ... }
    else -> ""
}

val memberShape: Shape
when (currShape) {
is StructureShape -> {
val member = if (currShape.asStructureShape().get().getMember(keyNode.value).isPresent) {
Copy link
Contributor

Choose a reason for hiding this comment

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

This is an odd if/else statement (which now that I look may have come from me and/or one of the other codebases).

Would be cleaner to just do something like:

val member = currShape.getMember(keyNode.value).orElseThrow {
    CodegenException("unknown member ${currShape.id}.${keyNode.value}")
}

We already know currShape is a StructureShape so we should just rely on smart casting instead of checking if it's a structure shape again. Also since the else branch throws this is easier to follow intent.

* Render a test class and unit tests for the specified [testCases]
*/
fun renderTestClass(testClassName: String) {
// writer.addImport(KotlinDependency.KOTLIN_TEST.namespace, "Test", "")
Copy link
Contributor

Choose a reason for hiding this comment

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

remove these commented out lines

@phani-srikar phani-srikar merged commit 578e523 into smithy-swift-http-bindings-staging Aug 31, 2020
@phani-srikar phani-srikar deleted the swift-smithy-request-test-gen branch August 31, 2020 23:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants