feat: add protocol tests runner #1293
Conversation
| { | ||
| "id": "e9fa30cd-1aa7-4c5d-b01a-384078b8863a", | ||
| "type": "feature", | ||
| "description": "Added a protocol tests runner, a new mechanism to run the tests and report the results in JSON format detached from the test phase of a regular build." | ||
| } |
There was a problem hiding this comment.
Nit: This update isn't user-facing so wouldn't typically need a changelog message. You can delete this and add the no-changelog label to this PR.
| val KOTLIN_COMPILER_VERSION: String = System.getProperty("smithy.kotlin.codegen.kotlinCompilerVersion", "2.1.0") | ||
| val SHADOW_JAR_VERSION: String = System.getProperty("com.github.johnrengelman.shadowVersion", "8.1.1") |
There was a problem hiding this comment.
Nit: We need to stop embedding hardcoded versions into our codegen files and should be auto-discovering them instead, similar to how we do with clientRuntimeVersion above. I don't expect you to implement that, though. Please leave a // FIXME on these two lines mentioning that we should detect versions.
| if (enableApplication) { | ||
| indent() | ||
| write("application") | ||
| write("id(\"com.github.johnrengelman.shadow\") version #S", SHADOW_JAR_VERSION) | ||
| dedent() | ||
| } |
There was a problem hiding this comment.
Style: Prefer Smithy placeholders or triple-quotes to avoid escaping strings which may contain quotes.
// either
write("id(#S) version #S", "com.github.johnrengelman.shadow", SHADOW_JAR_VERSION)
// or
write("""id("com.github.johnrengelman.shadow") version #S""", SHADOW_JAR_VERSION)Applies in other spots as well.
| writer.addImport("${ctx.settings.pkg.name}", "*") | ||
|
|
There was a problem hiding this comment.
Nit: We should not be adding more star imports to our codegen. Let's remove this and use #T placeholders to handle automatic importing.
| import org.jetbrains.kotlin.gradle.tasks.KotlinCompile | ||
|
|
||
| plugins { | ||
| alias(libs.plugins.kotlin.jvm) |
There was a problem hiding this comment.
Question: Does this need to be JVM only? Or can these tests be run in Kotlin/Native as well?
| private fun hasStandardProtocolTests(service: ServiceShape, model: Model): Boolean { | ||
| val isAwsServiceTest = service.findTrait(TagsTrait.ID) | ||
| .map { trait -> TagsTrait::class.java.cast(trait) } | ||
| // We add a tag to protocol tests for specific AWS services | ||
| // that require customizations to run properly. Those | ||
| // not standard protocol tests are filtered out here. | ||
| .map { tags -> tags.values.contains("aws-service-test") } | ||
| .orElse(false) | ||
|
|
||
| if (isAwsServiceTest) { | ||
| return false | ||
| } | ||
| // Check that at least one operation in the service has a test trait, either for | ||
| // requests or for responses. | ||
| TopDownIndex.of(model).getContainedOperations(service).forEach { operation -> | ||
| if (operation.hasTrait(HttpResponseTestsTrait.ID) || | ||
| operation.hasTrait(HttpRequestTestsTrait.ID) | ||
| ) { | ||
| return true | ||
| } | ||
| } | ||
| return false | ||
| } |
There was a problem hiding this comment.
Style: Can be simplified:
private fun hasStandardProtocolTests(service: ServiceShape, model: Model): Boolean {
val isAwsServiceTest = service
.getTrait<TagsTrait>()
?.values
// We add a tag to protocol tests for specific AWS services
// that require customizations to run properly. Those
// not standard protocol tests are filtered out here.
?.contains("aws-service-test")
?: false
return
!isAwsServiceTest &&
TopDownIndex
.of(model)
.getContainedOperations(service)
.any {
it.hasTrait<HttpResponseTestsTrait>() ||
it.hasTrait<HttpRequestTestsTrait>()
}
}
}| /** | ||
| * Executes `gradle build -x test` in the given directory to build the generated | ||
| * code that includes the client and the protocol tests. This build is expected | ||
| * to generate an uber JAR that we can the call directly without having to setup | ||
| * all the classpath elements. | ||
| */ | ||
| private fun buildProtocolTests(codegenPath: Path) { | ||
| ProcessBuilder("gradle", "build", "-x", "test") | ||
| .directory(codegenPath.toFile()) | ||
| .inheritIO() | ||
| .start() | ||
| .waitFor() | ||
| } |
There was a problem hiding this comment.
Correctness: This code assumes that Gradle will be installed on the $PATH in the current environment. IDK about other SDK developers but I never install Gradle on my machines, I just use the Gradle wrapper that comes with each repository. On my machine and anyone else's who doesn't install Gradle, this code will fail.
| publishing { | ||
| publications { | ||
| create<MavenPublication>("codegen") { | ||
| from(components["java"]) | ||
| artifact(sourcesJar) | ||
| } | ||
| } | ||
| } | ||
|
|
||
| configurePublishing("smithy-kotlin", "smithy-lang") |
There was a problem hiding this comment.
Question: Why do we need to publish this project? Will we need to fetch/run it from Maven Central in some cases?
Applies in other places too.
| /** | ||
| * A JSON simple, streaming, JSON writer. | ||
| */ | ||
| class JsonWriter(private val writer: Writer) : Closeable { |
There was a problem hiding this comment.
Style: We should replace this custom writer with kotlinx-serialization-json.
You can use buildJsonObject to create a JSON document instance and then either encodeToString or encodeToStream to write out the final JSON body:
val jsonDoc = buildJsonObject {
put("foo", "bar")
putJsonObject("baz") {
put("a", "Apple")
put("b", "Banana")
}
maybeNullQux?.let { put("qux", it) }
}
val jsonText = Json.encodeToString(jsonDoc)
/*
Resulting JSON:
{
"foo": "bar",
"baz": {
"a": "Apple",
"b": "Banana"
},
"qux": "turns out it wasn't null!"
}
*/| enum class Result(val value: String) { | ||
| PASSED("passed"), | ||
| FAILED("failed"), | ||
| ERRORED("errored"), | ||
| SKIPPED("skipped"), | ||
| } | ||
|
|
||
| enum class TestType(val value: String) { | ||
| REQUEST("request"), | ||
| RESPONSE("response"), | ||
| } | ||
|
|
||
| data class TestResult( | ||
| val testId: String, | ||
| val testType: TestType, | ||
| var result: Result = Result.PASSED, | ||
| var log: String? = null, | ||
| ) |
There was a problem hiding this comment.
Style:
This can also be replaced with kotlinx-serialziation by marking the classes as serializable:
@Serializable
enum class Result(val value: String) {
@SerialName("passed") PASSED,
@SerialName("failed") FAILED,
@SerialName("errored") ERRORED,
@SerialName("skipped") SKIPPED,
}
@Serializable
enum class TestType(val value: String) {
@SerialName("request") REQUEST,
@SerialName("response") RESPONSE,
}
@Serializable
data class TestResult(
val testId: String,
val testType: TestType,
var result: Result = Result.PASSED,
var log: String? = null,
)Now the Json serializer knows how to read/write these classes we can mix it up with your hand-written (i.e., unmodeled) results container via encodeToJsonElement:
fun writeResults(serviceId: String, protocolId: String, results: List<TestResult>) {
val jsonDoc = buildJsonObject {
put("service", serviceId)
put("protocol", protocolId)
putJsonArray("results") {
for (result in results) {
add(Json.encodeToJsonElement(result))
}
}
}
println(Json.encodeToString(jsonDoc))
}...but wait, there's more! 😃 By actually modelling your results container as a data class, you can achieve ultimate JSON zen:
@Serializable
data class ProtocolTestResults(service: String, protocol: String, results: List<TestResult>)
fun writeResults(serviceId: String, protocolId: String, results: List<TestResult>) {
val resultsObj = ProtocolTestResults(serviceId, protocolId, results)
println(Json.encodeToString(resultsObj))
}
Description of changes
Adds a protocol tests runner, a new mechanism to run the tests and report the results in JSON format detached from the test phase of a regular build.
This change adds new modules to the project:
codegen/smithy-kotlin-protocol-tests-codegenbuilds on top ofsmithy-kotlin-codegento generate a client and methods for each defined protocol test that generates a JSON result of detached from the test phase of the build.tests/protocol-tests-utilsimplements utility classes that are used to run the protocol tests such a the enums and JSON writer used in the reportstests/protocol-tests-utilsimplements the protocol tests runner, that identifies all the clients with protocol tests and for eachProtocol Tests Runner
This is an example of how to run the protocol tests runner. The runner is a single, fat jar, that can be run directly on the command line. It takes as an argument the JARs containing the protocol tests definitions, for instance:
At the end it creates in the same directory a file
results.jsonthat contains all the results from running the protocol tests.A gist of the resulting JSON file can be found here
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.