Unverified Commit ed7f7e69 authored by ysaito1001's avatar ysaito1001 Committed by GitHub
Browse files

Restore `SmokeTestsDecoratorTest` (#3811)

## Motivation and Context
Restores `SmokeTestsDecoratorTest` that was temporarily removed in
https://github.com/smithy-lang/smithy-rs/pull/3808.

## Description
The said test was temporarily removed because `SmokeTestsDecorator` is
included in the [predefined
decorators](https://github.com/smithy-lang/smithy-rs/blob/b38ccb969e09ae0856c235fcb496b3f3faf41c87/aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/AwsCodegenDecorator.kt#L35)
and pulls-in the `aws-config` crate for code generation. The issue was
conflicts between the runtime crates used by `aws-config` (located in
the `aws/sdk/build` directory) and those brought-in by that predefined
decorators used by `awsSdkIntegrationTest` (located in the
`rust-runtime` and `aws/rust-runtime` directories). This PR addresses
these conflicts and restores the test functionality.

Given the challenges, the code changes are centered around the following
ideas:
- Focus solely on testing the core class, `SmokeTestsInstantiator`. By
doing this, we avoid running `SmokeTestsDecorator`, which would
otherwise pull in the `aws-config` crate.
- Initializing a config builder in smoke tests needs to be parameterized
depending on the environment; in production we use
`aws_config::load_defaults` and in testing we avoid including
`aws-config` by using a default-constructed config builder, which is
sufficient for compile-only tests.
- The generated smoke tests in `SmokeTestsDecoratorTest` require minimal
runtime crates for compilation. We need the `CodegenVisitor` but with a
different set of codegen decorators. Prior to this PR,
`awsSdkIntegrationTest` used
[RustClientCodegenPlugin](https://github.com/smithy-lang/smithy-rs/blob/b38ccb969e09ae0856c235fcb496b3f3faf41c87/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/RustClientCodegenPlugin.kt#L46)
that in turn loaded [predefined
decorators](https://github.com/smithy-lang/smithy-rs/blob/b38ccb969e09ae0856c235fcb496b3f3faf41c87/aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/AwsCodegenDecorator.kt#L35)
on the classpath, which included `SmokeTestsDecorator`. In this PR, we
work around this by defining a minimal set of codegen decorators and
making them pluggable through `awsSdkIntegrationTest`.

## Testing
Tests in CI

----

_By submitting this pull request, I confirm that you can use, modify,
copy, and redistribute this contribution, under the terms of your
choice._
parent 2c3a4c15
Loading
Loading
Loading
Loading
+103 −73
Original line number Diff line number Diff line
@@ -18,10 +18,13 @@ import software.amazon.smithy.rust.codegen.core.rustlang.Attribute
import software.amazon.smithy.rust.codegen.core.rustlang.Attribute.Companion.cfg
import software.amazon.smithy.rust.codegen.core.rustlang.AttributeKind
import software.amazon.smithy.rust.codegen.core.rustlang.RustWriter
import software.amazon.smithy.rust.codegen.core.rustlang.Writable
import software.amazon.smithy.rust.codegen.core.rustlang.containerDocs
import software.amazon.smithy.rust.codegen.core.rustlang.docs
import software.amazon.smithy.rust.codegen.core.rustlang.rust
import software.amazon.smithy.rust.codegen.core.rustlang.rustBlock
import software.amazon.smithy.rust.codegen.core.rustlang.rustTemplate
import software.amazon.smithy.rust.codegen.core.rustlang.writable
import software.amazon.smithy.rust.codegen.core.smithy.CodegenContext
import software.amazon.smithy.rust.codegen.core.smithy.PublicImportSymbolProvider
import software.amazon.smithy.rust.codegen.core.smithy.RustCrate
@@ -67,15 +70,13 @@ class SmokeTestsDecorator : ClientCodegenDecorator {
        rustCrate: RustCrate,
    ) {
        // Get all operations with smoke tests
        val smokeTestedOperations =
            codegenContext.model.getOperationShapesWithTrait(SmokeTestsTrait::class.java).toList()
        val smokeTestedOperations = operationToTestCases(codegenContext.model)
        val supportedTests =
            smokeTestedOperations.map { operationShape ->
            smokeTestedOperations.map { (operationShape, testCases) ->
                // filter out unsupported smoke tests, logging a warning for each one, and sort the remaining tests by
                // case ID. This ensures deterministic rendering, meaning the test methods are always rendered in a
                // consistent order.
                val testCases =
                    operationShape.expectTrait<SmokeTestsTrait>().testCases.filter { smokeTestCase ->
                testCases.filter { smokeTestCase ->
                    isSmokeTestSupported(smokeTestCase)
                }.sortedBy { smokeTestCase -> smokeTestCase.id }

@@ -85,10 +86,45 @@ class SmokeTestsDecorator : ClientCodegenDecorator {
                .filter { (_, testCases) -> testCases.isNotEmpty() }
                // Similar to sorting test cases above, sort operations by name to ensure consistent ordering.
                .sortedBy { (operationShape, _) -> operationShape.id.name }

        // Return if there are no supported smoke tests across all operations
        if (supportedTests.isEmpty()) return

        rustCrate.integrationTest("smoketests") {
            renderPrologue(codegenContext.moduleUseName(), this)

            for ((operationShape, testCases) in supportedTests) {
                docs("Smoke tests for the `${operationShape.id.name.toSnakeCase()}` operation")
                val instantiator =
                    SmokeTestsInstantiator(
                        codegenContext, operationShape,
                        configBuilderInitializer = { ->
                            writable {
                                rustTemplate(
                                    """
                                    let config = #{awsConfig}::load_defaults(config::BehaviorVersion::latest()).await;
                                    let conf = config::Config::from(&config).to_builder()
                                    """,
                                    "awsConfig" to AwsCargoDependency.awsConfig(codegenContext.runtimeConfig).toType(),
                                )
                            }
                        },
                    )
                for (testCase in testCases) {
                    Attribute.TokioTest.render(this)
                    this.rustBlock("async fn test_${testCase.id.toSnakeCase()}()") {
                        instantiator.render(this, testCase)
                    }
                }
            }
        }
    }
}

fun renderPrologue(
    moduleUseName: String,
    writer: RustWriter,
) = writer.apply {
    // Don't run the tests in this module unless `RUSTFLAGS="--cfg smoketests"` is passed.
    Attribute(cfg("smoketests")).render(this, AttributeKind.Inner)

@@ -99,32 +135,16 @@ class SmokeTestsDecorator : ClientCodegenDecorator {

        ```sh
        RUSTFLAGS="--cfg smoketests" cargo test.
                ```""",
        ```
        """,
    )

            val model = codegenContext.model
            val moduleUseName = codegenContext.moduleUseName()
    rust("use $moduleUseName::{Client, config};")

            for ((operationShape, testCases) in supportedTests) {
                val operationName = operationShape.id.name.toSnakeCase()
                val operationInput = operationShape.inputShape(model)

                docs("Smoke tests for the `$operationName` operation")

                for (testCase in testCases) {
                    Attribute.TokioTest.render(this)
                    this.rustBlock("async fn test_${testCase.id.toSnakeCase()}()") {
                        val instantiator = SmokeTestsInstantiator(codegenContext)
                        instantiator.renderConf(this, testCase)
                        rust("let client = Client::from_conf(conf);")
                        instantiator.renderInput(this, operationShape, operationInput, testCase.params)
                        instantiator.renderExpectation(this, model, testCase.expectation)
                    }
                }
            }
        }
}

fun operationToTestCases(model: Model) =
    model.getOperationShapesWithTrait(SmokeTestsTrait::class.java).toList().map { operationShape ->
        operationShape to operationShape.expectTrait<SmokeTestsTrait>().testCases
    }

class SmokeTestsBuilderKindBehavior(val codegenContext: CodegenContext) : Instantiator.BuilderKindBehavior {
@@ -136,22 +156,34 @@ class SmokeTestsBuilderKindBehavior(val codegenContext: CodegenContext) : Instan
    override fun doesSetterTakeInOption(memberShape: MemberShape): Boolean = true
}

class SmokeTestsInstantiator(private val codegenContext: ClientCodegenContext) : Instantiator(
class SmokeTestsInstantiator(
    codegenContext: ClientCodegenContext, private val operationShape: OperationShape,
    private val configBuilderInitializer: () -> Writable,
) : Instantiator(
        PublicImportSymbolProvider(codegenContext.symbolProvider, codegenContext.moduleUseName()),
        codegenContext.model,
        codegenContext.runtimeConfig,
        SmokeTestsBuilderKindBehavior(codegenContext),
    ) {
    fun renderConf(
    private val model = codegenContext.model
    private val symbolProvider = codegenContext.symbolProvider

    fun render(
        writer: RustWriter,
        testCase: SmokeTestCase,
    ) {
        writer.rust(
            "let config = #{T}::load_defaults(config::BehaviorVersion::latest()).await;",
            AwsCargoDependency.awsConfig(codegenContext.runtimeConfig).toType(),
        )
        writer.rust("let conf = config::Config::from(&config).to_builder()")
        writer.indent()
    ) = writer.apply {
        renderConf(this, testCase)
        rust("let client = Client::from_conf(conf);")
        renderInput(this, testCase.params)
        renderExpectation(this, testCase.expectation)
    }

    private fun renderConf(
        writer: RustWriter,
        testCase: SmokeTestCase,
    ) = writer.apply {
        rustTemplate("#{config_builder_initializer}", "config_builder_initializer" to configBuilderInitializer())
        indent()

        // TODO(https://github.com/smithy-lang/smithy-rs/issues/3776) Once Account ID routing is supported,
        //  reflect the config setting here, especially to disable it if needed, as it is enabled by default in
@@ -159,58 +191,56 @@ class SmokeTestsInstantiator(private val codegenContext: ClientCodegenContext) :

        val vendorParams = AwsSmokeTestModel.getAwsVendorParams(testCase)
        vendorParams.orNull()?.let { params ->
            writer.rust(".region(config::Region::new(${params.region.dq()}))")
            writer.rust(".use_dual_stack(${params.useDualstack()})")
            writer.rust(".use_fips(${params.useFips()})")
            params.uri.orNull()?.let { writer.rust(".endpoint_url($it)") }
            rust(".region(config::Region::new(${params.region.dq()}))")
            rust(".use_dual_stack(${params.useDualstack()})")
            rust(".use_fips(${params.useFips()})")
            params.uri.orNull()?.let { rust(".endpoint_url($it)") }
        }

        val s3VendorParams = AwsSmokeTestModel.getS3VendorParams(testCase)
        s3VendorParams.orNull()?.let { params ->
            writer.rust(".accelerate_(${params.useAccelerate()})")
            writer.rust(".force_path_style_(${params.forcePathStyle()})")
            writer.rust(".use_arn_region(${params.useArnRegion()})")
            writer.rust(".disable_multi_region_access_points(${params.useMultiRegionAccessPoints().not()})")
            rust(".accelerate_(${params.useAccelerate()})")
            rust(".force_path_style_(${params.forcePathStyle()})")
            rust(".use_arn_region(${params.useArnRegion()})")
            rust(".disable_multi_region_access_points(${params.useMultiRegionAccessPoints().not()})")
        }

        writer.rust(".build();")
        writer.dedent()
        rust(".build();")
        dedent()
    }

    fun renderInput(
    private fun renderInput(
        writer: RustWriter,
        operationShape: OperationShape,
        inputShape: StructureShape,
        data: Optional<ObjectNode>,
        headers: Map<String, String> = mapOf(),
        ctx: Ctx = Ctx(),
    ) {
    ) = writer.apply {
        val operationBuilderName =
            FluentClientGenerator.clientOperationFnName(operationShape, codegenContext.symbolProvider)
            FluentClientGenerator.clientOperationFnName(operationShape, symbolProvider)
        val inputShape = operationShape.inputShape(model)

        writer.rust("let res = client.$operationBuilderName()")
        writer.indent()
        rust("let res = client.$operationBuilderName()")
        indent()
        data.orNull()?.let {
            renderStructureMembers(writer, inputShape, it, headers, ctx)
        }
        writer.rust(".send().await;")
        writer.dedent()
        rust(".send().await;")
        dedent()
    }

    fun renderExpectation(
    private fun renderExpectation(
        writer: RustWriter,
        model: Model,
        expectation: Expectation,
    ) {
    ) = writer.apply {
        if (expectation.isSuccess) {
            writer.rust("""res.expect("request should succeed");""")
            rust("""res.expect("request should succeed");""")
        } else if (expectation.isFailure) {
            val expectedErrShape = expectation.failure.orNull()?.errorId?.orNull()
            println(expectedErrShape)
            if (expectedErrShape != null) {
                val failureShape = model.expectShape(expectedErrShape)
                val errName = codegenContext.symbolProvider.toSymbol(failureShape).name.toSnakeCase()
                writer.rust(
                val errName = symbolProvider.toSymbol(failureShape).name.toSnakeCase()
                rust(
                    """
                    let err = res.expect_err("request should fail");
                    let err = err.into_service_error();
@@ -218,7 +248,7 @@ class SmokeTestsInstantiator(private val codegenContext: ClientCodegenContext) :
                    """,
                )
            } else {
                writer.rust("""res.expect_err("request should fail");""")
                rust("""res.expect_err("request should fail");""")
            }
        }
    }
+174 −0
Original line number Diff line number Diff line
/*
 * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
 * SPDX-License-Identifier: Apache-2.0
 */

package software.amazon.smithy.rustsdk

import org.junit.jupiter.api.Test
import software.amazon.smithy.build.PluginContext
import software.amazon.smithy.rust.codegen.client.smithy.ClientCodegenVisitor
import software.amazon.smithy.rust.codegen.client.smithy.customizations.NoAuthDecorator
import software.amazon.smithy.rust.codegen.client.smithy.customize.ClientCodegenDecorator
import software.amazon.smithy.rust.codegen.client.smithy.customize.CombinedClientCodegenDecorator
import software.amazon.smithy.rust.codegen.client.smithy.customize.RequiredCustomizations
import software.amazon.smithy.rust.codegen.client.smithy.endpoint.EndpointsDecorator
import software.amazon.smithy.rust.codegen.client.testutil.ClientDecoratableBuildPlugin
import software.amazon.smithy.rust.codegen.client.testutil.testClientCodegenContext
import software.amazon.smithy.rust.codegen.core.rustlang.rust
import software.amazon.smithy.rust.codegen.core.rustlang.writable
import software.amazon.smithy.rust.codegen.core.testutil.asSmithyModel
import software.amazon.smithy.rust.codegen.core.testutil.integrationTest
import software.amazon.smithy.rust.codegen.core.testutil.tokioTest
import software.amazon.smithy.rust.codegen.core.util.toSnakeCase

class SmokeTestsDecoratorTest {
    companion object {
        val model =
            """
            namespace test

            use aws.api#service
            use smithy.test#smokeTests
            use aws.auth#sigv4
            use aws.protocols#restJson1
            use smithy.rules#endpointRuleSet

            @service(sdkId: "dontcare")
            @restJson1
            @sigv4(name: "dontcare")
            @auth([sigv4])
            @endpointRuleSet({
                "version": "1.0",
                "rules": [{ "type": "endpoint", "conditions": [], "endpoint": { "url": "https://example.com" } }],
                "parameters": {
                    "Region": { "required": false, "type": "String", "builtIn": "AWS::Region" },
                }
            })
            service TestService {
                version: "2023-01-01",
                operations: [SomeOperation]
            }

            @smokeTests([
                {
                    id: "SomeOperationSuccess",
                    params: {}
                    vendorParams: {
                        region: "us-west-2"
                    }
                    expect: { success: {} }
                }
                {
                    id: "SomeOperationFailure",
                    params: {}
                    vendorParams: {
                        region: "us-west-2"
                    }
                    expect: { failure: {} }
                }
                {
                    id: "SomeOperationFailureExplicitShape",
                    params: {}
                    vendorParams: {
                        region: "us-west-2"
                    }
                    expect: {
                        failure: { errorId: FooException }
                    }
                }
            ])
            @http(uri: "/SomeOperation", method: "POST")
            @optionalAuth
            operation SomeOperation {
                input: SomeInput,
                output: SomeOutput,
                errors: [FooException]
            }

            @input
            structure SomeInput {}

            @output
            structure SomeOutput {}

            @error("server")
            structure FooException { }
            """.asSmithyModel(smithyVersion = "2")
    }

    @Test
    fun smokeTestSdkCodegen() {
        val codegenContext = testClientCodegenContext(model)
        val smokeTestedOperations = operationToTestCases(model)
        awsSdkIntegrationTest(
            model,
            buildPlugin = SdkSmokeTestsRustClientCodegenPlugin(),
            // `SdkSmokeTestsRustClientCodegenPlugin` only uses the minimal set of codegen decorators, which results
            // in a significant amount of unused code. This can cause `clippy` to fail with the `--deny warnings`
            // setting enabled by default in `.crate/config.toml` in test workspaces.
            // To work around this issue, we unset `RUSTFLAGS` to allow unused and dead code.
            environment = mapOf(Pair("RUSTFLAGS", "")),
            test = { _, crate ->
                // It should compile. We can't run the tests because they don't target a real service.
                // They are skipped because the `smoketests` flag is unset for `rustc` in the `cargo test`
                // invocation specified by `awsIntegrationTestParams`.
                crate.integrationTest("smoketests") {
                    renderPrologue(codegenContext.moduleUseName(), this)
                    for ((shape, testCases) in smokeTestedOperations) {
                        val sut =
                            SmokeTestsInstantiator(
                                codegenContext, shape,
                                // We cannot use `aws_config::load_defaults`, but a default-constructed config builder
                                // will suffice for the test.
                                configBuilderInitializer = { ->
                                    writable {
                                        rust("let conf = config::Builder::new()")
                                    }
                                },
                            )
                        for (testCase in testCases) {
                            tokioTest("test_${testCase.id.toSnakeCase()}") {
                                sut.render(this, testCase)
                            }
                        }
                    }
                }
            },
        )
    }
}

/**
 * A `ClientDecoratableBuildPlugin` that intentionally avoids including `codegenDecorator` on the classpath.
 *
 *  If we used `RustClientCodegenPlugin` on the classpath from this location, it would return decorators including
 *  `SmokeTestsDecorator` that ultimately pulls in the `aws-config` crate. This crate depends on runtime crates located
 *  in the `aws/sdk/build` directory, causing conflicts with runtime crates in `rust-runtime` or in `aws/rust-runtime`.
 *
 *  This class does not look at the classpath to prevent the inclusion of the `aws-config` crate. Instead, it uses the
 *  minimal set of codegen decorators to generate modules sufficient to compile smoke tests in a test
 *  workspace.
 */
class SdkSmokeTestsRustClientCodegenPlugin : ClientDecoratableBuildPlugin() {
    override fun getName(): String = "sdk-smoke-tests-rust-client-codegen"

    override fun executeWithDecorator(
        context: PluginContext,
        vararg decorator: ClientCodegenDecorator,
    ) {
        val codegenDecorator =
            CombinedClientCodegenDecorator(
                listOf(
                    EndpointsDecorator(),
                    AwsFluentClientDecorator(),
                    SdkConfigDecorator(),
                    NoAuthDecorator(),
                    RequiredCustomizations(),
                    *decorator,
                ),
            )

        ClientCodegenVisitor(context, codegenDecorator).execute()
    }
}
+6 −0
Original line number Diff line number Diff line
@@ -9,6 +9,8 @@ import software.amazon.smithy.model.Model
import software.amazon.smithy.model.node.ObjectNode
import software.amazon.smithy.rust.codegen.client.smithy.ClientCodegenContext
import software.amazon.smithy.rust.codegen.client.smithy.ClientRustSettings
import software.amazon.smithy.rust.codegen.client.smithy.RustClientCodegenPlugin
import software.amazon.smithy.rust.codegen.client.testutil.ClientDecoratableBuildPlugin
import software.amazon.smithy.rust.codegen.client.testutil.clientIntegrationTest
import software.amazon.smithy.rust.codegen.client.testutil.testClientCodegenContext
import software.amazon.smithy.rust.codegen.client.testutil.testClientRustSettings
@@ -42,10 +44,14 @@ fun awsTestCodegenContext(
fun awsSdkIntegrationTest(
    model: Model,
    params: IntegrationTestParams = awsIntegrationTestParams(),
    buildPlugin: ClientDecoratableBuildPlugin = RustClientCodegenPlugin(),
    environment: Map<String, String> = mapOf(),
    test: (ClientCodegenContext, RustCrate) -> Unit = { _, _ -> },
) = clientIntegrationTest(
    model,
    params,
    buildPlugin = buildPlugin,
    environment = environment,
    test = test,
)

+4 −2
Original line number Diff line number Diff line
@@ -21,6 +21,8 @@ fun clientIntegrationTest(
    params: IntegrationTestParams =
        IntegrationTestParams(cargoCommand = "cargo test --features behavior-version-latest"),
    additionalDecorators: List<ClientCodegenDecorator> = listOf(),
    buildPlugin: ClientDecoratableBuildPlugin = RustClientCodegenPlugin(),
    environment: Map<String, String> = mapOf(),
    test: (ClientCodegenContext, RustCrate) -> Unit = { _, _ -> },
): Path {
    fun invokeRustCodegenPlugin(ctx: PluginContext) {
@@ -38,9 +40,9 @@ fun clientIntegrationTest(
                    test(codegenContext, rustCrate)
                }
            }
        RustClientCodegenPlugin().executeWithDecorator(ctx, codegenDecorator, *additionalDecorators.toTypedArray())
        buildPlugin.executeWithDecorator(ctx, codegenDecorator, *additionalDecorators.toTypedArray())
    }
    return codegenIntegrationTest(model, params, invokePlugin = ::invokeRustCodegenPlugin)
    return codegenIntegrationTest(model, params, invokePlugin = ::invokeRustCodegenPlugin, environment)
}

/**
+2 −1
Original line number Diff line number Diff line
@@ -139,6 +139,7 @@ fun codegenIntegrationTest(
    model: Model,
    params: IntegrationTestParams,
    invokePlugin: (PluginContext) -> Unit,
    environment: Map<String, String> = mapOf(),
): Path {
    val (ctx, testDir) =
        generatePluginContext(
@@ -156,7 +157,7 @@ fun codegenIntegrationTest(
    invokePlugin(ctx)
    ctx.fileManifest.printGeneratedFiles()
    val logger = Logger.getLogger("CodegenIntegrationTest")
    val out = params.command?.invoke(testDir) ?: (params.cargoCommand ?: "cargo test --lib --tests").runCommand(testDir)
    val out = params.command?.invoke(testDir) ?: (params.cargoCommand ?: "cargo test --lib --tests").runCommand(testDir, environment = environment)
    logger.fine(out.toString())
    return testDir
}