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

misc: smoke tests fixes #1160

Merged
merged 17 commits into from
Nov 7, 2024
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changes/c376a9ee-568d-4638-8f4f-2e9a54f2869c.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
{
"id": "c376a9ee-568d-4638-8f4f-2e9a54f2869c",
"type": "feature",
"description": "Failed smoke tests now print exception stack trace"
Copy link
Contributor

Choose a reason for hiding this comment

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

nit/opinion: This does not need a changelog

}
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,7 @@ data class KotlinDependency(
val IDENTITY_API = KotlinDependency(GradleConfiguration.Implementation, "$RUNTIME_ROOT_NS", RUNTIME_GROUP, "identity-api", RUNTIME_VERSION)
val SMITHY_RPCV2_PROTOCOLS = KotlinDependency(GradleConfiguration.Implementation, "$RUNTIME_ROOT_NS.awsprotocol.rpcv2", RUNTIME_GROUP, "smithy-rpcv2-protocols", RUNTIME_VERSION)
val SMITHY_RPCV2_PROTOCOLS_CBOR = KotlinDependency(GradleConfiguration.Implementation, "$RUNTIME_ROOT_NS.awsprotocol.rpcv2.cbor", RUNTIME_GROUP, "smithy-rpcv2-protocols", RUNTIME_VERSION)
val AWS_SIGNING_CRT = KotlinDependency(GradleConfiguration.Implementation, "$RUNTIME_ROOT_NS.auth.awssigning.crt", RUNTIME_GROUP, "aws-signing-crt", RUNTIME_VERSION)

// External third-party dependencies
val KOTLIN_STDLIB = KotlinDependency(GradleConfiguration.Implementation, "kotlin", "org.jetbrains.kotlin", "kotlin-stdlib", KOTLIN_COMPILER_VERSION)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,7 @@ object RuntimeTypes {

object SmokeTests : RuntimeTypePackage(KotlinDependency.CORE, "smoketests") {
val exitProcess = symbol("exitProcess")
val printExceptionStackTrace = symbol("printExceptionStackTrace")
}

object Collections : RuntimeTypePackage(KotlinDependency.CORE, "collections") {
Expand Down Expand Up @@ -378,6 +379,10 @@ object RuntimeTypes {
val sigV4 = symbol("sigV4")
val sigV4A = symbol("sigV4A")
}

object AwsSigningCrt : RuntimeTypePackage(KotlinDependency.AWS_SIGNING_CRT) {
val CrtAwsSigner = symbol("CrtAwsSigner")
}
}

object Observability {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,13 @@ package software.amazon.smithy.kotlin.codegen.rendering.smoketests
import software.amazon.smithy.codegen.core.Symbol
import software.amazon.smithy.kotlin.codegen.core.*
import software.amazon.smithy.kotlin.codegen.integration.SectionId
import software.amazon.smithy.kotlin.codegen.integration.SectionKey
import software.amazon.smithy.kotlin.codegen.model.getTrait
import software.amazon.smithy.kotlin.codegen.model.hasTrait
import software.amazon.smithy.kotlin.codegen.rendering.endpoints.EndpointParametersGenerator
import software.amazon.smithy.kotlin.codegen.rendering.endpoints.EndpointProviderGenerator
import software.amazon.smithy.kotlin.codegen.rendering.smoketests.SmokeTestUriValue.EndpointParameters
import software.amazon.smithy.kotlin.codegen.rendering.smoketests.SmokeTestUriValue.EndpointProvider
import software.amazon.smithy.kotlin.codegen.rendering.util.format
import software.amazon.smithy.kotlin.codegen.utils.dq
import software.amazon.smithy.kotlin.codegen.utils.toCamelCase
Expand All @@ -18,7 +23,21 @@ object SmokeTestsRunner : SectionId
object SmokeTestAdditionalEnvVars : SectionId
object SmokeTestDefaultConfig : SectionId
object SmokeTestRegionDefault : SectionId
object SmokeTestUseDualStackKey : SectionId
object SmokeTestSigv4aRegionSetKey : SectionId
object SmokeTestSigv4aRegionSetValue : SectionId
object SmokeTestAccountIdBasedRoutingKey : SectionId
object SmokeTestAccountIdBasedRoutingValue : SectionId
object SmokeTestHttpEngineOverride : SectionId
object SmokeTestUseAccelerateKey : SectionId
object SmokeTestUseMultiRegionAccessPointsKey : SectionId
object SmokeTestUseMultiRegionAccessPointsValue : SectionId
object SmokeTestUseGlobalEndpoint : SectionId
object SmokeTestUriKey : SectionId
object SmokeTestUriValue : SectionId {
val EndpointProvider: SectionKey<Symbol> = SectionKey("EndpointProvider")
val EndpointParameters: SectionKey<Symbol> = SectionKey("EndpointParameters")
}
Copy link
Contributor

Choose a reason for hiding this comment

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

organization/style: Namespace these sections under another object SmokeTestSectionId

Copy link
Contributor

Choose a reason for hiding this comment

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

nit: My comment here is still valid

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This isn't part of the diff anymore. But do you mean the SectionKeys?


const val SKIP_TAGS = "AWS_SMOKE_TEST_SKIP_TAGS"
const val SERVICE_FILTER = "AWS_SMOKE_TEST_SERVICE_IDS"
Expand All @@ -31,10 +50,11 @@ class SmokeTestsRunnerGenerator(
ctx: CodegenContext,
) {
private val model = ctx.model
private val sdkId = ctx.settings.sdkId
private val settings = ctx.settings
private val sdkId = settings.sdkId
private val symbolProvider = ctx.symbolProvider
private val service = symbolProvider.toSymbol(model.expectShape(ctx.settings.service))
private val operations = ctx.model.topDownOperations(ctx.settings.service).filter { it.hasTrait<SmokeTestsTrait>() }
private val service = symbolProvider.toSymbol(model.expectShape(settings.service))
private val operations = model.topDownOperations(settings.service).filter { it.hasTrait<SmokeTestsTrait>() }

internal fun render() {
writer.declareSection(SmokeTestsRunner) {
Expand Down Expand Up @@ -108,12 +128,72 @@ class SmokeTestsRunnerGenerator(
writer.withInlineBlock("#L {", "}", service) {
if (testCase.vendorParams.isPresent) {
testCase.vendorParams.get().members.forEach { vendorParam ->
if (vendorParam.key.value == "region") {
writeInline("#L = ", vendorParam.key.value.toCamelCase())
declareSection(SmokeTestRegionDefault)
write("#L", vendorParam.value.format())
} else {
write("#L = #L", vendorParam.key.value.toCamelCase(), vendorParam.value.format())
when (vendorParam.key.value) {
"region" -> {
writeInline("#L = ", vendorParam.key.value.toCamelCase())
Copy link
Contributor

Choose a reason for hiding this comment

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

style: repeated vendorParam.key.value.toCamelCase() can be stored in a local variable

declareSection(SmokeTestRegionDefault)
write("#L", vendorParam.value.format())
}
"sigv4aRegionSet" -> {
Copy link
Contributor

Choose a reason for hiding this comment

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

correctness: smithy-kotlin should not know anything about AWS-specific config options. region was fine because we've treated it like a generic client config option, but is there any way to lift the handling of these AWS-specific vendor params out of smithy-kotlin?

I'm not sure there is, since this implementation heavily relies on sections, but something to consider. Can a single "vendor params" section be declared and the name of the vendor param (i.e. sigv4aRegionSet) be passed in context to aws-sdk-kotlin?

declareSection(SmokeTestSigv4aRegionSetKey) {
writeInline("#L", vendorParam.key.value.toCamelCase())
}
writeInline(" = ")
declareSection(SmokeTestSigv4aRegionSetValue) {
write("#L", vendorParam.value.format())
}
}
"uri" -> {
declareSection(SmokeTestUriKey) {
writeInline("#L", vendorParam.key.value.toCamelCase())
}
writeInline(" = ")
declareSection(
SmokeTestUriValue,
mapOf(
EndpointProvider to EndpointProviderGenerator.getSymbol(settings),
EndpointParameters to EndpointParametersGenerator.getSymbol(settings),
),
) {
write("#L", vendorParam.value.format())
}
}
"useDualstack" -> {
declareSection(SmokeTestUseDualStackKey) {
writeInline("#L", vendorParam.key.value.toCamelCase())
}
write(" = #L", vendorParam.value.format())
}
"useAccountIdRouting" -> {
declareSection(SmokeTestAccountIdBasedRoutingKey) {
writeInline("#L", vendorParam.key.value.toCamelCase())
}
writeInline(" = ")
declareSection(SmokeTestAccountIdBasedRoutingValue) {
write("#L", vendorParam.value.format())
}
}
"useAccelerate" -> {
declareSection(SmokeTestUseAccelerateKey) {
writeInline("#L", vendorParam.key.value.toCamelCase())
}
write(" = #L", vendorParam.value.format())
}
"useMultiRegionAccessPoints" -> {
declareSection(SmokeTestUseMultiRegionAccessPointsKey) {
writeInline("#L", vendorParam.key.value.toCamelCase())
}
writeInline(" = ")
declareSection(SmokeTestUseMultiRegionAccessPointsValue) {
write("#L", vendorParam.value.format())
}
}
"useGlobalEndpoint" -> {
declareSection(SmokeTestUseGlobalEndpoint) {
write("#L = #L", vendorParam.key.value.toCamelCase(), vendorParam.value.format())
}
}
else -> write("#L = #L", vendorParam.key.value.toCamelCase(), vendorParam.value.format())
}
}
} else {
Expand All @@ -133,8 +213,10 @@ class SmokeTestsRunnerGenerator(
writer.withBlock(".#T { client ->", "}", RuntimeTypes.Core.IO.use) {
withBlock("client.#L(", ")", operation.defaultName()) {
withBlock("#L {", "}", operationSymbol) {
testCase.params.get().members.forEach { member ->
write("#L = #L", member.key.value.toCamelCase(), member.value.format())
if (testCase.params.isPresent) {
testCase.params.get().members.forEach { member ->
write("#L = #L", member.key.value.toCamelCase(), member.value.format())
}
}
}
}
Expand All @@ -156,6 +238,7 @@ class SmokeTestsRunnerGenerator(
testCase.expectation.isFailure,
writer,
)
writer.write("if (!success) #T(e)", RuntimeTypes.Core.SmokeTests.printExceptionStackTrace)
writer.write("if (!success) exitCode = 1")
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,7 @@ class SmokeTestsRunnerGeneratorTest {
val success = e is SmokeTestsSuccessException
val status = if (success) "ok" else "not ok"
println("${'$'}status Test SuccessTest - no error expected from service ")
if (!success) printExceptionStackTrace(e)
if (!success) exitCode = 1
}
}
Expand Down Expand Up @@ -155,6 +156,7 @@ class SmokeTestsRunnerGeneratorTest {
val success = e is InvalidMessageError
val status = if (success) "ok" else "not ok"
println("${'$'}status Test InvalidMessageErrorTest - error expected from service ")
if (!success) printExceptionStackTrace(e)
if (!success) exitCode = 1
}
}
Expand Down Expand Up @@ -189,6 +191,7 @@ class SmokeTestsRunnerGeneratorTest {
val success = e is SmokeTestsFailureException
val status = if (success) "ok" else "not ok"
println("${'$'}status Test FailureTest - error expected from service ")
if (!success) printExceptionStackTrace(e)
if (!success) exitCode = 1
}
}
Expand Down
2 changes: 2 additions & 0 deletions runtime/protocol/http-client/api/http-client.api
Original file line number Diff line number Diff line change
Expand Up @@ -426,6 +426,7 @@ public final class aws/smithy/kotlin/runtime/http/interceptors/ResponseLengthVal

public final class aws/smithy/kotlin/runtime/http/interceptors/SmokeTestsFailureException : java/lang/Exception {
public fun <init> ()V
public fun <init> (Ljava/lang/String;)V
}

public final class aws/smithy/kotlin/runtime/http/interceptors/SmokeTestsInterceptor : aws/smithy/kotlin/runtime/client/Interceptor {
Expand Down Expand Up @@ -453,6 +454,7 @@ public final class aws/smithy/kotlin/runtime/http/interceptors/SmokeTestsInterce

public final class aws/smithy/kotlin/runtime/http/interceptors/SmokeTestsSuccessException : java/lang/Exception {
public fun <init> ()V
public fun <init> (Ljava/lang/String;)V
}

public final class aws/smithy/kotlin/runtime/http/middleware/DefaultValidateResponse : aws/smithy/kotlin/runtime/http/operation/ReceiveMiddleware {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,14 +16,19 @@ public class SmokeTestsInterceptor : HttpInterceptor {
override fun readBeforeDeserialization(context: ProtocolResponseInterceptorContext<Any, HttpRequest, HttpResponse>) {
val status = context.protocolResponse.status.value
when (status) {
in 400..599 -> throw SmokeTestsFailureException()
in 200..299 -> throw SmokeTestsSuccessException()
else -> throw SmokeTestsUnexpectedException()
in 400..599 -> throw SmokeTestsFailureException("Smoke test failed with HTTP status code: $status")
in 200..299 -> throw SmokeTestsSuccessException("Smoke test succeeded with HTTP status code: $status")
else -> throw SmokeTestsUnexpectedException("Smoke test returned HTTP status code: $status")
}
}
}

@InternalApi public class SmokeTestsFailureException : Exception()
@InternalApi public class SmokeTestsFailureException(message: String) : Exception(message) {
public constructor() : this("Smoke test failed with HTTP status code in the inclusive range: 400-599")
}

@InternalApi public class SmokeTestsSuccessException(message: String) : Exception(message) {
public constructor() : this("Smoke test succeeded with HTTP status code in the inclusive range: 200-599")
}

@InternalApi public class SmokeTestsSuccessException : Exception()
private class SmokeTestsUnexpectedException : Exception()
private class SmokeTestsUnexpectedException(message: String) : Exception(message)
4 changes: 4 additions & 0 deletions runtime/runtime-core/api/runtime-core.api
Original file line number Diff line number Diff line change
Expand Up @@ -2048,6 +2048,10 @@ public final class aws/smithy/kotlin/runtime/smoketests/SmokeTestsFunctionsJVMKt
public static final fun exitProcess (I)Ljava/lang/Void;
}

public final class aws/smithy/kotlin/runtime/smoketests/SmokeTestsFunctionsKt {
public static final fun printExceptionStackTrace (Ljava/lang/Exception;)V
}

public final class aws/smithy/kotlin/runtime/text/Scanner {
public fun <init> (Ljava/lang/String;)V
public final fun getText ()Ljava/lang/String;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,19 @@
package aws.smithy.kotlin.runtime.smoketests

public expect fun exitProcess(status: Int): Nothing

/**
* Prints an exceptions stack trace using test anything protocol (TAP) format e.g.
*
* #java.lang.ArithmeticException: / by zero
* # at FileKt.main(File.kt:3)
* # at FileKt.main(File.kt)
* # at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
* # at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(Unknown Source)
* # at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(Unknown Source)
* # at java.base/java.lang.reflect.Method.invoke(Unknown Source)
* # at executors.JavaRunnerExecutor$Companion.main(JavaRunnerExecutor.kt:27)
* # at executors.JavaRunnerExecutor.main(JavaRunnerExecutor.kt)
*/
public fun printExceptionStackTrace(exception: Exception): Unit =
println(exception.stackTraceToString().split("\n").joinToString("\n") { "#$it" })
Loading