-
Notifications
You must be signed in to change notification settings - Fork 80
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
feat: Smoke Tests V2 #1791
Changes from 13 commits
5d20161
833e7a4
772a902
90fe001
b0b5438
ba0b666
83407be
4ef5af6
4576e3c
68224c5
3d67e22
cc056bc
19914a1
8452690
324ac6f
f85d8f3
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -79,12 +79,38 @@ struct GeneratePackageManifest { | |
/// Generates a package manifest file and saves it to `packageFileName` | ||
func run() throws { | ||
try FileManager.default.changeWorkingDirectory(repoPath) | ||
|
||
// Generate package manifest for aws-sdk-swift and save it as aws-sdk-swift/Package.swift | ||
let contents = try generatePackageManifestContents() | ||
try savePackageManifest(contents) | ||
try savePackageManifest(contents, packageFileName) | ||
|
||
// Generate package manifest for smoke tests and save it as aws-sdk-swift/SmokeTests/Package.swift | ||
let smokeTestsContents = try generateSmokeTestsPackageManifestContents() | ||
try savePackageManifest(smokeTestsContents, "SmokeTests/\(packageFileName)") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Generate manifest for smoke tests module at |
||
} | ||
|
||
// 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) | ||
} | ||
|
||
/// Returns the contents of the generated package manifest. | ||
/// This determines the versions of the dependencies and the list of services to include and then genraetes the package manifest with those values. | ||
/// | ||
|
@@ -102,10 +128,10 @@ struct GeneratePackageManifest { | |
/// If no file exists, then this will create a new file. Otherwise, this will overwrite the existing file. | ||
/// | ||
/// - Parameter contents: The contents of the package manifest. | ||
func savePackageManifest(_ contents: String) throws { | ||
log("Saving package manifest to \(packageFileName)...") | ||
func savePackageManifest(_ contents: String, _ packageFilePath: String) throws { | ||
log("Saving package manifest to \(packageFilePath)...") | ||
try contents.write( | ||
toFile: packageFileName, | ||
toFile: packageFilePath, | ||
atomically: true, | ||
encoding: .utf8 | ||
) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. A small refactor to make this function reusable for different manifest filepaths. |
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,39 @@ | ||
// MARK: - Static Content | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 |
||
|
||
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" | ||
) | ||
} |
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is a place holder file just to add |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -203,13 +203,23 @@ val AwsService.modelExtrasDir: String | |
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 |
||
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) | ||
} | ||
} | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 { | ||
|
@@ -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 | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,77 @@ | ||
package software.amazon.smithy.aws.swift.codegen | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe 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? There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 |
---|---|---|
|
@@ -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( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is there also a list of There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,46 @@ | ||
#!/bin/bash | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Convenience script for developers to run every smoke test generated under |
||
|
||
# This is a convenience script for developers for running every smoke test under SmokeTests/. | ||
# The script must be run from aws-sdk-swift/, the directory containing SmokeTests/. | ||
|
||
# cd into test module dir | ||
cd SmokeTests/ || { echo "ERROR: Failed to change directory to SmokeTests."; exit 1; } | ||
|
||
# Build and discard output for clean log | ||
echo "INFO: Building SmokeTests module..." | ||
swift build > /dev/null 2>&1 | ||
|
||
# Header print helpers | ||
print_header() { | ||
print_spacer | ||
local header=$1 | ||
echo "##### $header #####" | ||
print_spacer | ||
} | ||
|
||
print_spacer() { | ||
echo "" | ||
} | ||
|
||
# Build and run each and every test runner; save result to results array | ||
print_header "TEST RUNS" | ||
results=() | ||
for runnerName in ./*; do | ||
if [ -d "$runnerName" ]; then | ||
swift run "${runnerName#./}" | ||
if [ $? -eq 0 ]; then | ||
# Record success | ||
results+=("SUCCESS: ${runnerName#./}") | ||
else | ||
# record failure | ||
results+=("FAILURE: ${runnerName#./}") | ||
fi | ||
print_spacer | ||
fi | ||
done | ||
|
||
# Print result summary | ||
print_header "TEST RESULT SUMMARY" | ||
for result in "${results[@]}"; do | ||
echo "$result" | ||
done |
There was a problem hiding this comment.
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 thePackage.swift
foraws-sdk-swift
project. The static fileSmokeTestsPackage.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
).