-
Notifications
You must be signed in to change notification settings - Fork 31
Service sdk testing #1330
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
Service sdk testing #1330
Conversation
…hy-kotlin into server-sdk-main # Please enter a commit message to explain why this merge is necessary, # especially if it merges an updated upstream into a topic branch. # # Lines starting with '#' will be ignored, and an empty message aborts # the commit.
…n into service-sdk-serde # Conflicts: # codegen/smithy-kotlin-codegen/src/main/kotlin/software/amazon/smithy/kotlin/codegen/core/KotlinDependency.kt # codegen/smithy-kotlin-codegen/src/main/kotlin/software/amazon/smithy/kotlin/codegen/core/RuntimeTypes.kt # codegen/smithy-kotlin-codegen/src/main/kotlin/software/amazon/smithy/kotlin/codegen/service/ServiceStubGenerator.kt
…ntime rather than compile
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
…ice-sdk-testing # Conflicts: # codegen/smithy-kotlin-codegen/src/main/kotlin/software/amazon/smithy/kotlin/codegen/CodegenVisitor.kt # codegen/smithy-kotlin-codegen/src/main/kotlin/software/amazon/smithy/kotlin/codegen/core/KotlinDependency.kt
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
1 similar comment
This comment has been minimized.
This comment has been minimized.
...otlin-codegen/src/main/kotlin/software/amazon/smithy/kotlin/codegen/core/KotlinDependency.kt
Show resolved
Hide resolved
...hy-kotlin-codegen/src/main/kotlin/software/amazon/smithy/kotlin/codegen/core/RuntimeTypes.kt
Outdated
Show resolved
Hide resolved
...main/kotlin/software/amazon/smithy/kotlin/codegen/rendering/serde/CborSerializerGenerator.kt
Outdated
Show resolved
Hide resolved
...n-codegen/src/main/kotlin/software/amazon/smithy/kotlin/codegen/service/KtorStubGenerator.kt
Outdated
Show resolved
Hide resolved
| import java.nio.file.Path | ||
| import java.nio.file.Paths | ||
|
|
||
| fun main() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Question: Our test projects don't normally need main() methods. Why couldn't this work be done in a @Before method or even manually written and checked into the repository?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This function is used to generate the service. In gradle.build.kts, you can see that the test depends on runServiceGenerator which will call this funciton. The reason why not having this in @Before method is that I want to separate the file for generating service and testing it. I don't understand what you meant by "manually written and checked into the repository".
tests/codegen/service-codegen-tests/src/test/kotlin/com/test/ServiceGeneratorTest.kt
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall looks good! Just had some small suggestions, nit's and questions but the Java 17 changes would cause us to regress on the behavior introduced by #1297. That's the only blocking thing for me.
Issue #
Description of changes
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.