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

feat: Smoke Tests V2 #1791

Merged
merged 16 commits into from
Oct 29, 2024
Merged
Show file tree
Hide file tree
Changes from all 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
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ Package.resolved
build/
codegen/protocol-test-codegen-local/smithy-build.json
codegen/protocol-test-codegen/smithy-build.json
SmokeTests/

# vim temporary files
*.swp
Expand Down
1 change: 1 addition & 0 deletions AWSSDKSwiftCLI/Package.swift
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ let package = Package(
resources: [
.process("Resources/Package.Prefix.txt"),
.process("Resources/Package.Base.txt"),
.process("Resources/SmokeTestsPackage.Base.txt"),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The package manifest for SmokeTests module will also be generated along with the Package.swift for aws-sdk-swift project. The static file SmokeTestsPackage.Base.txt is appended after generated content during manifest generation, similar to how it's done for root project. Both manifests will use same file for prefix (Package.Prefix.txt).

.process("Resources/DocIndex.Base.md")
]
),
Expand Down
13 changes: 13 additions & 0 deletions AWSSDKSwiftCLI/Sources/AWSCLIUtils/FileManager+Utils.swift
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,19 @@ public extension FileManager {
.filter { !$0.hasPrefix(".") }
}

/// Returns the list of services that have smoke tests and test runner generated for them.
/// Service names are extracted from service name prefix of directory names under `SmokeTests/`.
/// E.g., extract `AWSS3` from `SmokeTests/AWSS3SmokeTestRunner/`.
///
/// - Returns: The list of services with generated smoke tests.
func servicesWithSmokeTests() throws -> [String] {
try FileManager.default
.contentsOfDirectory(atPath: "SmokeTests")
.sorted()
.filter { !$0.hasPrefix(".") && $0.hasSuffix("SmokeTestRunner") }
.map { $0.replacingOccurrences(of: "SmokeTestRunner", with: "") }
}

Comment on lines +39 to +51
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Similar logic to getting enabled services - by looking at generated directories.

/// Returns the list of Smithy runtime modules within `../smithy-swift/Sources/Core`
///
/// - Returns: The list of Smithy runtime modules.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,8 @@ struct AWSSDKSwiftCLI: ParsableCommand {
PrepareReleaseCommand.self,
SyncClientRuntimeVersionCommand.self,
TestAWSSDKCommand.self,
GenerateDocIndexCommand.self
GenerateDocIndexCommand.self,
GenerateSmokeTestsPackageManifestCommand.self
]
)
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
//
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Separate subcommand for generating package manifest for SmokeTests module.

// Copyright Amazon.com Inc. or its affiliates.
// All Rights Reserved.
//
// SPDX-License-Identifier: Apache-2.0
//

import ArgumentParser
import Foundation
import AWSCLIUtils

struct GenerateSmokeTestsPackageManifestCommand: ParsableCommand {
static var configuration = CommandConfiguration(
commandName: "generate-smoke-tests-package-manifest",
abstract: "Generates the Package.swift manifest for the aws-sdk-swift/SmokeTests package."
)

@Argument(help: "The path to the aws-sdk-swift repository")
var repoPath: String

func run() throws {
let generateSmokeTestsPackageManifest = GenerateSmokeTestsPackageManifest(
repoPath: repoPath
)
try generateSmokeTestsPackageManifest.run()
}
}

struct GenerateSmokeTestsPackageManifest {
/// The path to the package repository
let repoPath: String

func run() throws {
try FileManager.default.changeWorkingDirectory(repoPath)
// Generate package manifest for smoke tests and save it as aws-sdk-swift/SmokeTests/Package.swift
let smokeTestsContents = try generateSmokeTestsPackageManifestContents()
try savePackageManifest(smokeTestsContents)
}

// MARK: - Helpers

func generateSmokeTestsPackageManifestContents() throws -> String {
return [
// SmokeTests package manifest uses same prefix as one for aws-sdk-swift.
try PackageManifestBuilder.contentReader(filename: "Package.Prefix")(),
try generateServiceNamesArray(),
try PackageManifestBuilder.contentReader(filename: "SmokeTestsPackage.Base")()
].joined(separator: .newline)
}

func generateServiceNamesArray() throws -> String {
let servicesWithSmokeTests = try FileManager.default.servicesWithSmokeTests()
let formatedServiceList = servicesWithSmokeTests.map { "\t\"\($0)\"," }.joined(separator: .newline)
return [
"// All services that have smoke tests generated for them.",
"let serviceNames: [String] = [",
formatedServiceList,
"]"
].joined(separator: .newline)
}

func savePackageManifest(_ contents: String) throws {
let packageFilePath = "SmokeTests/Package.swift"
log("Saving package manifest to \(packageFilePath)...")
try contents.write(
toFile: packageFilePath,
atomically: true,
encoding: .utf8
)
log("Successfully saved package manifest to \(packageFilePath)")
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
// MARK: - Static Content
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The static content of package manifest. The only generated content, which comes right before this, is serviceNames array which will contain names of all services that have smoke tests. Then, that array is used in this static content for adding all the executable targets and executable products needed to build & run our test runners.


extension Target.Dependency {
// AWS runtime module
static var awsClientRuntime: Self { .product(name: "AWSClientRuntime", package: "aws-sdk-swift") }
// Smithy runtime module
static var clientRuntime: Self { .product(name: "ClientRuntime", package: "smithy-swift") }
}

let package = Package(
name: "SmokeTests",
platforms: [
.macOS(.v10_15)
],
products: serviceNames.map(productForRunner(_:)),
dependencies: [
.package(path: "../../smithy-swift"),
.package(path: "../../aws-sdk-swift")
],
targets: serviceNames.map(targetForRunner(_:))
)

// MARK: - Helper functions

private func productForRunner(_ serviceName: String) -> Product {
.executable(name: "\(serviceName)SmokeTestRunner", targets: ["\(serviceName)SmokeTestRunner"])
}

private func targetForRunner(_ serviceName: String) -> Target {
.executableTarget(
name: "\(serviceName)SmokeTestRunner",
dependencies: [
.clientRuntime,
.awsClientRuntime,
.product(name: "\(serviceName)", package: "aws-sdk-swift")
],
path: "\(serviceName)SmokeTestRunner"
)
}
Empty file added SmokeTests/.gitkeep
Empty file.
16 changes: 13 additions & 3 deletions codegen/sdk-codegen/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -203,13 +203,23 @@ val AwsService.modelExtrasDir: String

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changes in this file adds staging logic for moving generated smoke tests for each service to corresponding directory under SmokeTests/.

task("stageSdks") {
group = "codegen"
description = "relocate generated SDK(s) from build directory to Sources and Tests directories"
description = "relocate generated SDK artifacts from build directory to correct locations"
doLast {
discoveredServices.forEach {
logger.info("copying ${it.outputDir} source to ${it.sourcesDir}")
copy {
from("${it.outputDir}")
into("${it.sourcesDir}")
from(it.outputDir)
into(it.sourcesDir)
exclude { details ->
// Exclude `SmokeTests` directory
details.file.name.endsWith("SmokeTests")
}
}

logger.info("copying ${it.outputDir}/SmokeTests to SmokeTests")
copy {
from("${it.outputDir}/SmokeTests")
into(rootProject.file("SmokeTests").absolutePath)
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,8 @@ abstract class AWSHTTPBindingProtocolGenerator(
val requestTestBuilder = HttpProtocolUnitTestRequestGenerator.Builder()
val responseTestBuilder = HttpProtocolUnitTestResponseGenerator.Builder()
val errorTestBuilder = HttpProtocolUnitTestErrorGenerator.Builder()
open val testsToIgnore: Set<String> = setOf()
open val tagsToIgnore: Set<String> = setOf()
open val protocolTestsToIgnore: Set<String> = setOf()
open val protocolTestTagsToIgnore: Set<String> = setOf()
Comment on lines +39 to +40
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Small rename for specifying type of tests these apply to, just for additional clarity.


override val shouldRenderEncodableConformance = false
override fun generateProtocolUnitTests(ctx: ProtocolGenerator.GenerationContext): Int {
Expand All @@ -48,11 +48,15 @@ abstract class AWSHTTPBindingProtocolGenerator(
errorTestBuilder,
customizations,
getProtocolHttpBindingResolver(ctx, defaultContentType),
testsToIgnore,
tagsToIgnore,
protocolTestsToIgnore,
protocolTestTagsToIgnore,
).generateProtocolTests() + renderEndpointsTests(ctx)
}

override fun generateSmokeTests(ctx: ProtocolGenerator.GenerationContext) {
return AWSSmokeTestGenerator(ctx).generateSmokeTests()
}

fun renderEndpointsTests(ctx: ProtocolGenerator.GenerationContext): Int {
val ruleSetNode = ctx.service.getTrait<EndpointRuleSetTrait>()?.ruleSet
val ruleSet = if (ruleSetNode != null) EndpointRuleSet.fromNode(ruleSetNode) else null
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
package software.amazon.smithy.aws.swift.codegen
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Extends generic smoke test generator in smithy-swift. Overrides methods to provide AWS specific logic.


import software.amazon.smithy.aws.traits.ServiceTrait
import software.amazon.smithy.model.node.ObjectNode
import software.amazon.smithy.swift.codegen.SwiftWriter
import software.amazon.smithy.swift.codegen.integration.ProtocolGenerator
import software.amazon.smithy.swift.codegen.integration.SmokeTestGenerator
import software.amazon.smithy.swift.codegen.utils.toUpperCamelCase

class AWSSmokeTestGenerator(
private val ctx: ProtocolGenerator.GenerationContext
) : SmokeTestGenerator(ctx) {
// Filter out tests by name or tag at codegen time.
// Each element must have the prefix "<service-name>:" before the test name or tag name.
// E.g., "AWSS3:GetObjectTest" or "AWSS3:BucketTests"
override val smokeTestIdsToIgnore = setOf<String>(
// Add smoke test name to ignore here:
// E.g., "AWSACM:GetCertificateFailure",
)
override val smokeTestTagsToIgnore = setOf<String>(
// Add smoke test tag to ignore here:
// E.g., "AWSACM:TagToIgnore",
)

override fun getServiceName(): String {
return "AWS" + ctx.service.getTrait(ServiceTrait::class.java).get().sdkId.toUpperCamelCase()
}

override fun getClientName(): String {
return ctx.service.getTrait(ServiceTrait::class.java).get().sdkId.toUpperCamelCase().removeSuffix("Service") + "Client"
}

override fun renderCustomFilePrivateVariables(writer: SwiftWriter) {
writer.write("fileprivate let regionFromEnv = ProcessInfo.processInfo.environment[\"AWS_SMOKE_TEST_REGION\"]")
writer.write("fileprivate let tagsToSkip = (ProcessInfo.processInfo.environment[\"AWS_SMOKE_TEST_SKIP_TAGS\"] ?? \"\").components(separatedBy: \",\")")
}

override fun handleVendorParams(vendorParams: ObjectNode, writer: SwiftWriter) {
val nameToValueMappings = getFormattedVendorParams(vendorParams)
nameToValueMappings.forEach { mapping ->
writer.write("config.${mapping.key} = ${mapping.value}")
}
}

// Converts trait definition vendor param key:value pairs to Swift SDK config field:value pairs.
private fun getFormattedVendorParams(vendorParams: ObjectNode): Map<String, String> {
val formattedMapping = mutableMapOf<String, String>()
vendorParams.members.forEach { originalMapping ->
when (originalMapping.key.value) {
/* BaseAwsVendorParams members */
"region" -> {
// Take region value retrieved from environment variable if present; otherwise, take from trait definition.
val regionValue = "regionFromEnv ?? \"${originalMapping.value.expectStringNode().value}\""
formattedMapping.put("region", regionValue)
formattedMapping.put("signingRegion", regionValue)
}
"sigv4aRegionSet" -> { /* no-op; setting multiple signing regions in config is unsupported atm. */ }
"uri" -> { formattedMapping.put("endpoint", "\"${originalMapping.value.expectStringNode().value}\"") }
"useFips" -> { formattedMapping.put("useFIPS", originalMapping.value.expectBooleanNode().value.toString()) }
"useDualstack" -> { formattedMapping.put("useDualStack", originalMapping.value.expectBooleanNode().value.toString()) }
"useAccountIdRouting" -> { /* no-op; setting account ID routing in config is unsupported atm. */ }

/* S3VendorParams members */
"useAccelerate" -> { formattedMapping.put("accelerate", originalMapping.value.expectBooleanNode().value.toString()) }
"useMultiRegionAccessPoints" -> {
// Name for corresponding config in Swift SDK is: `disableMultiRegionAccessPoints`; value needs to be flipped.
formattedMapping.put("disableMultiRegionAccessPoints", (!(originalMapping.value.expectBooleanNode().value)).toString())
}
"useGlobalEndpoint", "forcePathStyle", "useArnRegion" -> {
// No change needed for these
formattedMapping.put(originalMapping.key.value, originalMapping.value.expectBooleanNode().value.toString())
}
}
}
return formattedMapping
}
Comment on lines +45 to +76
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 "translates" vendor params officially defined in Smithy vendor param shapes that service teams can use to specify client config values in their smoke tests. There're a couple that we don't have config bindings for yet.

Copy link
Contributor

Choose a reason for hiding this comment

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

What is the behavior if they use a param we haven't supported yet? Fail codegen or generate without it & presumably fail the test later?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If they use a param we haven't supported yet, the test will be generated without it and test will most likely fail later. If that happens we'll need to disable smoke test temporarily.

}
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ class AWSJSON1_0ProtocolGenerator : AWSHTTPBindingProtocolGenerator(AWSJSONCusto
override val defaultContentType = "application/x-amz-json-1.0"
override val protocol: ShapeId = AwsJson1_0Trait.ID
override val shouldRenderEncodableConformance: Boolean = true
override val testsToIgnore = setOf(
override val protocolTestsToIgnore = setOf(
"SDKAppliedContentEncoding_awsJson1_0",
"SDKAppendsGzipAndIgnoresHttpProvidedEncoding_awsJson1_0",
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ class AWSJSON1_1ProtocolGenerator : AWSHTTPBindingProtocolGenerator(AWSJSONCusto
override val defaultContentType = "application/x-amz-json-1.1"
override val protocol: ShapeId = AwsJson1_1Trait.ID
override val shouldRenderEncodableConformance: Boolean = true
override val testsToIgnore = setOf(
override val protocolTestsToIgnore = setOf(
"SDKAppliedContentEncoding_awsJson1_1",
"SDKAppendsGzipAndIgnoresHttpProvidedEncoding_awsJson1_1",
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ open class AWSQueryProtocolGenerator : AWSHTTPBindingProtocolGenerator(AWSQueryC
HttpBindingResolver = FormURLHttpBindingResolver(ctx, defaultContentType)

override val shouldRenderEncodableConformance = true
override val testsToIgnore = setOf(
override val protocolTestsToIgnore = setOf(
"SDKAppliedContentEncoding_awsQuery",
"SDKAppendsGzipAndIgnoresHttpProvidedEncoding_awsQuery",
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ class EC2QueryProtocolGenerator : AWSHTTPBindingProtocolGenerator(EC2QueryCustom
HttpBindingResolver = FormURLHttpBindingResolver(ctx, contentType)

override val shouldRenderEncodableConformance = true
override val testsToIgnore = setOf(
override val protocolTestsToIgnore = setOf(
"SDKAppliedContentEncoding_ec2Query",
"SDKAppendsGzipAndIgnoresHttpProvidedEncoding_ec2Query"
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import software.amazon.smithy.model.shapes.ShapeId
class AWSRestJson1ProtocolGenerator : AWSHTTPBindingProtocolGenerator(RestJSONCustomizations()) {
override val defaultContentType = "application/json"
override val protocol: ShapeId = RestJson1Trait.ID
override val testsToIgnore = setOf(
override val protocolTestsToIgnore = setOf(
"SDKAppliedContentEncoding_restJson1",
"SDKAppendedGzipAfterProvidedEncoding_restJson1",
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import software.amazon.smithy.swift.codegen.integration.ProtocolGenerator
class RestXMLProtocolGenerator : AWSHTTPBindingProtocolGenerator(RestXMLCustomizations()) {
override val defaultContentType: String = "application/xml"
override val protocol: ShapeId = RestXmlTrait.ID
override val testsToIgnore: Set<String> = setOf(
override val protocolTestsToIgnore: Set<String> = setOf(
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there also a list of smokeTestsToIgnore or are you just renaming this for clarity?

Copy link
Contributor Author

@sichanyoo sichanyoo Oct 28, 2024

Choose a reason for hiding this comment

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

The latter, just renaming this to clarify.

"S3DefaultAddressing", // can leave disabled, pre-endpoints 2.0
"S3VirtualHostAddressing", // can leave disabled, pre-endpoints 2.0
"S3VirtualHostDualstackAddressing", // can leave disabled, pre-endpoints 2.0
Expand Down
1 change: 1 addition & 0 deletions scripts/codegen.sh
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ rm -rf codegen/sdk-codegen/build/smithyprojections/sdk-codegen/*
rm -rf ServiceClients/*
rm -rf Sources/Services/*
rm -rf Tests/Services/*
rm -rf SmokeTests/*

# Regenerate code
./gradlew -p codegen/sdk-codegen build
Expand Down
Loading
Loading