Unverified Commit 8a453134 authored by david-perez's avatar david-perez Committed by GitHub
Browse files

Rely on `.cargo/config.toml` instead of `RUSTFLAGS` in tests' generated crates (#3192)

This is PR is similar to #1422. When generating a crate (either via
`Project.compileAndTest` or `{client,server}IntegrationTest`), we're
setting `RUSTFLAGS`. When developing, a common thing to do after
generating a crate is to open its contents in an editor, make
modifications, and run `cargo` again.

- the editor will kick off `rust-analyzer`, which will build the code
  again without the `RUSTFLAGS`; and
- the developer is unlikely to use the same `RUSTFLAGS` when running
  manual `cargo` commands; in fact, they'll most likely use none.

Indeed, using `CARGO_LOG=cargo::core::compiler::fingerprint=trace` on
the generated crate reveals that `cargo` is claiming it has to rebuild
the entire dependency closure because a different set of flags was used.
This causes unnecessary crate rebuilds during which the developer has to
wait.

Instead of relying on `RUSTFLAGS` in the command line invocation, we can
leverage `.cargo/config.toml`'s `[build.rustflags]` to persist the
settings in a file, like we did in #1422. That way, any plain `cargo`
command that runs on the same project will reuse the previous
compilation artifacts.

----

_By submitting this pull request, I confirm that you can use, modify,
copy, and redistribute this contribution, under the terms of your
choice._
parent b3ca5bfd
Loading
Loading
Loading
Loading
+1 −3
Original line number Diff line number Diff line
@@ -11,8 +11,6 @@ import org.junit.jupiter.params.provider.MethodSource
import software.amazon.smithy.model.Model
import software.amazon.smithy.rust.codegen.client.testutil.EndpointTestDiscovery
import software.amazon.smithy.rust.codegen.client.testutil.clientIntegrationTest
import software.amazon.smithy.rust.codegen.core.testutil.IntegrationTestParams
import software.amazon.smithy.rust.codegen.core.util.runCommand

class EndpointResolverGeneratorTest {
    companion object {
@@ -55,7 +53,7 @@ class EndpointResolverGeneratorTest {
        // if (!suite.toString().contains("hostable")) {
        // return
        // }
        clientIntegrationTest(suite, params = IntegrationTestParams(command = { it -> "cargo test".runCommand(it, mapOf("RUSTFLAGS" to "")) }))
        clientIntegrationTest(suite)
    }

    /*
+2 −1
Original line number Diff line number Diff line
@@ -17,6 +17,7 @@ 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.runWithWarnings
import software.amazon.smithy.rust.codegen.core.util.CommandError
import software.amazon.smithy.rust.codegen.core.util.runCommand

/**
 * End-to-end test of endpoint resolvers, attaching a real resolver to a fully generated service
@@ -128,7 +129,7 @@ class EndpointsDecoratorTest {
        val testDir = clientIntegrationTest(
            model,
            // Just run integration tests.
            IntegrationTestParams(command = { "cargo test --all-features --test *".runWithWarnings(it) }),
            IntegrationTestParams(command = { "cargo test --all-features --test *".runCommand(it) }),
        ) { clientCodegenContext, rustCrate ->
            rustCrate.integrationTest("endpoint_params_test") {
                val moduleName = clientCodegenContext.moduleUseName()
+4 −1
Original line number Diff line number Diff line
@@ -40,10 +40,13 @@ fun codegenIntegrationTest(model: Model, params: IntegrationTestParams, invokePl
        params.runtimeConfig,
        params.overrideTestDir,
    )

    testDir.writeDotCargoConfigToml(listOf("--deny", "warnings"))

    invokePlugin(ctx)
    ctx.fileManifest.printGeneratedFiles()
    val logger = Logger.getLogger("CodegenIntegrationTest")
    val out = params.command?.invoke(testDir) ?: (params.cargoCommand ?: "cargo test --lib --tests").runCommand(testDir, environment = mapOf("RUSTFLAGS" to "-D warnings"))
    val out = params.command?.invoke(testDir) ?: (params.cargoCommand ?: "cargo test --lib --tests").runCommand(testDir)
    logger.fine(out.toString())
    return testDir
}
+22 −1
Original line number Diff line number Diff line
@@ -40,9 +40,11 @@ import software.amazon.smithy.rust.codegen.core.util.letIf
import software.amazon.smithy.rust.codegen.core.util.orNullIfEmpty
import software.amazon.smithy.rust.codegen.core.util.runCommand
import java.io.File
import java.nio.file.Files
import java.nio.file.Files.createTempDirectory
import java.nio.file.Path
import kotlin.io.path.absolutePathString
import kotlin.io.path.writeText

val TestModuleDocProvider = object : ModuleDocProvider {
    override fun docsWriter(module: RustModule.LeafModule): Writable = writable {
@@ -340,7 +342,13 @@ fun TestWriterDelegator.compileAndTest(
    } catch (e: Exception) {
        // cargo fmt errors are useless, ignore
    }
    val env = mapOf("RUSTFLAGS" to "-A dead_code")

    // Clean `RUSTFLAGS` because in CI we pass in `--deny warnings` and
    // we still generate test code with warnings.
    // TODO(https://github.com/smithy-lang/smithy-rs/issues/3194)
    val env = mapOf("RUSTFLAGS" to "")
    baseDir.writeDotCargoConfigToml(listOf("--allow", "dead_code"))

    val testOutput = "cargo test".runCommand(baseDir, env)
    if (runClippy) {
        "cargo clippy --all-features".runCommand(baseDir, env)
@@ -348,6 +356,19 @@ fun TestWriterDelegator.compileAndTest(
    return testOutput
}

fun Path.writeDotCargoConfigToml(rustFlags: List<String>) {
    val dotCargoDir = this.resolve(".cargo")
    Files.createDirectory(dotCargoDir)

    dotCargoDir.resolve("config.toml")
        .writeText(
            """
            [build]
            rustflags = [${rustFlags.joinToString(",") { "\"$it\"" }}]
            """.trimIndent(),
        )
}

fun TestWriterDelegator.rustSettings() =
    testRustSettings(
        service = ShapeId.from("fake#Fake"),