Skip to content

chore: Opt-in service client tests #1000

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.

Sign up for GitHub

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

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open

chore: Opt-in service client tests #1000

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
2b786ae
chore: Opt-in service client tests
jbelkins Nov 17, 2025
bab3f79
Fix ktlint
jbelkins Nov 17, 2025
3fd2e3e
Fix codegen tests
jbelkins Nov 17, 2025
3aa053f
Move manifest generator out of protocol generation, move dependency J…
jbelkins Nov 17, 2025
fc22c30
Fix codegen tests
jbelkins Nov 17, 2025
4083247
Merge branch 'main' into jbe/remove_service_client_tests
jbelkins Nov 25, 2025
0969649
Merge branch 'main' into jbe/remove_service_client_tests
jbelkins Dec 2, 2025
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
30 changes: 0 additions & 30 deletions ...t-codegen/src/main/kotlin/software/amazon/smithy/swift/codegen/DependencyJSONGenerator.kt
View file
Open in desktop

This file was deleted.

22 changes: 7 additions & 15 deletions ...wift-codegen/src/main/kotlin/software/amazon/smithy/swift/codegen/DirectedSwiftCodegen.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,6 @@ class DirectedSwiftCodegen(

LOGGER.info("Generating Swift client for service ${directive.settings().service}")

var shouldGenerateTestTarget = false
context.protocolGenerator?.apply {
val ctx = ProtocolGenerator.GenerationContext(settings, model, service, symbolProvider, integrations, this.protocol, writers)
LOGGER.info("[${service.id}] Generating serde for protocol ${this.protocol}")
Expand All @@ -86,31 +85,24 @@ class DirectedSwiftCodegen(

LOGGER.info("[${service.id}] Generating unit tests for protocol ${this.protocol}")
val numProtocolUnitTestsGenerated = generateProtocolUnitTests(ctx)
shouldGenerateTestTarget = (numProtocolUnitTestsGenerated > 0)
Copy link
Contributor Author

@jbelkins jbelkins Nov 18, 2025

Choose a reason for hiding this comment

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

  • Deleted the unused variable above
  • Moved manifest generation & buffer flush out of the protocol generator
  • Moved "writeAddtionalFiles" to be the very last step in the protocol generator


LOGGER.info("[${service.id}] Generated $numProtocolUnitTestsGenerated tests for protocol ${this.protocol}")

LOGGER.info("[${service.id}] Generating service client for protocol ${this.protocol}")
generateProtocolClient(ctx)

integrations.forEach { it.writeAdditionalFiles(context, ctx, writers) }

LOGGER.info("[${service.id}] Generating smoke tests for service")
generateSmokeTests(ctx)

if (settings.generatePackageManifest) {
LOGGER.info("Generating package manifest file")
PackageManifestGenerator(ctx).writePackageManifest(writers.dependencies)
}
LOGGER.info("[${service.id}] Generating additional files")
integrations.forEach { it.writeAdditionalFiles(context, ctx, writers) }
}

if (settings.generateDependencyJSON) {
LOGGER.info("Generating dependency JSON file")
DependencyJSONGenerator(ctx).writePackageJSON(writers.dependencies)
}
LOGGER.info("[${service.id}] Generating package manifest file")
PackageManifestGenerator(context).writePackageManifest(writers.dependencies)

LOGGER.info("Flushing swift writers")
writers.flushWriters()
}
LOGGER.info("[${service.id}] Flushing swift writers")
writers.flushWriters()
}

override fun generateStructure(directive: GenerateStructureDirective<GenerationContext, SwiftSettings>) {
Expand Down
6 changes: 3 additions & 3 deletions ...-codegen/src/main/kotlin/software/amazon/smithy/swift/codegen/PackageManifestGenerator.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,13 @@
package software.amazon.smithy.swift.codegen

import software.amazon.smithy.codegen.core.SymbolDependency
import software.amazon.smithy.swift.codegen.integration.ProtocolGenerator
import software.amazon.smithy.swift.codegen.core.GenerationContext

class PackageManifestGenerator(
val ctx: ProtocolGenerator.GenerationContext,
val ctx: GenerationContext,
) {
fun writePackageManifest(dependencies: List<SymbolDependency>) {
ctx.delegator.useFileWriter("Package.swift") { writer ->
ctx.writerDelegator().useFileWriter("Package.swift") { writer ->
Copy link
Contributor Author

@jbelkins jbelkins Nov 17, 2025

Choose a reason for hiding this comment

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

The PackageManifestGenerator now uses the non-protocol generation context, since it is not part of any protocol.

writer.write("// swift-tools-version: \$L", ctx.settings.swiftVersion)
writer.write("")
writer.write("import PackageDescription")
Expand Down
10 changes: 0 additions & 10 deletions smithy-swift-codegen/src/main/kotlin/software/amazon/smithy/swift/codegen/SwiftSettings.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,6 @@ private const val MERGE_MODELS = "mergeModels"
private const val COPYRIGHT_NOTICE = "copyrightNotice"
private const val VISIBILITY = "visibility"
private const val INTERNAL_CLIENT = "internalClient"
private const val GENERATE_PACKAGE_MANIFEST = "generatePackageManifest"
private const val GENERATE_DEPENDENCY_JSON = "generateDependencyJSON"

// Prioritized list of protocols supported for code generation
private val DEFAULT_PROTOCOL_RESOLUTION_PRIORITY =
Expand Down Expand Up @@ -67,8 +65,6 @@ class SwiftSettings(
val copyrightNotice: String,
val visibility: String,
val internalClient: Boolean,
val generatePackageManifest: Boolean,
val generateDependencyJSON: Boolean,
Copy link
Contributor Author

@jbelkins jbelkins Nov 17, 2025

Choose a reason for hiding this comment

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

These two SwiftSettings fields are no longer used since the package manifest is now always generated, and the Dependencies.json file generation has been moved to SDK.

) {
companion object {
private val LOGGER: Logger = Logger.getLogger(SwiftSettings::class.java.name)
Expand Down Expand Up @@ -100,8 +96,6 @@ class SwiftSettings(
COPYRIGHT_NOTICE,
VISIBILITY,
INTERNAL_CLIENT,
GENERATE_PACKAGE_MANIFEST,
GENERATE_DEPENDENCY_JSON,
),
)

Expand All @@ -127,8 +121,6 @@ class SwiftSettings(
)
val visibility = config.getStringMemberOrDefault(VISIBILITY, "public")
val internalClient = config.getBooleanMemberOrDefault(INTERNAL_CLIENT, false)
val generatePackageManifest = config.getBooleanMemberOrDefault(GENERATE_PACKAGE_MANIFEST, true)
val generateDependencyJSON = config.getBooleanMemberOrDefault(GENERATE_DEPENDENCY_JSON, false)

return SwiftSettings(
serviceId,
Expand All @@ -144,8 +136,6 @@ class SwiftSettings(
copyrightNotice,
visibility,
internalClient,
generatePackageManifest,
generateDependencyJSON,
)
}

Expand Down
15 changes: 14 additions & 1 deletion smithy-swift-codegen/src/test/kotlin/software/amazon/smithy/swift/codegen/TestUtils.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import software.amazon.smithy.model.traits.DocumentationTrait
import software.amazon.smithy.model.traits.ErrorTrait
import software.amazon.smithy.model.traits.HttpErrorTrait
import software.amazon.smithy.model.traits.RetryableTrait
import software.amazon.smithy.swift.codegen.core.GenerationContext
import software.amazon.smithy.swift.codegen.customtraits.SwiftBoxTrait
import software.amazon.smithy.swift.codegen.integration.HTTPBindingProtocolGenerator
import software.amazon.smithy.swift.codegen.integration.ProtocolGenerator
Expand Down Expand Up @@ -328,6 +329,7 @@ fun createStructureWithOptionalErrorMessage(): StructureShape {
}

class TestContext(
val context: GenerationContext,
val generationCtx: ProtocolGenerator.GenerationContext,
val manifest: MockManifest,
val generator: ProtocolGenerator,
Expand Down Expand Up @@ -398,6 +400,17 @@ fun Model.newTestContext(
.get()
val delegator = SwiftDelegator(settings, this, manifest, provider)

val context =
GenerationContext(
this,
provider,
settings,
manifest,
generator,
emptyList(),
delegator,
)

Copy link
Contributor Author

@jbelkins jbelkins Nov 17, 2025

Choose a reason for hiding this comment

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

TestContext now provides a GenerationContext as well as a ProtocolGenerator.GenerationContext (there are too many things named "context" here.)

This allows for testing codegen outside of a specific protocol.

val ctx =
ProtocolGenerator.GenerationContext(
settings,
Expand All @@ -408,7 +421,7 @@ fun Model.newTestContext(
generator.protocol,
delegator,
)
return TestContext(ctx, manifest, generator)
return TestContext(context, ctx, manifest, generator)
}

fun getSettingsNode(
Expand Down
2 changes: 0 additions & 2 deletions ...c/test/kotlin/software/amazon/smithy/swift/codegen/codegencomponents/SwiftSettingsTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -194,8 +194,6 @@ class SwiftSettingsTest {
copyrightNotice = "// Test copyright",
visibility = "public",
internalClient = false,
generatePackageManifest = false,
generateDependencyJSON = true,
)

private fun createServiceWithProtocols(protocols: Set<ShapeId>): ServiceShape {
Expand Down
42 changes: 0 additions & 42 deletions ...tlin/software/amazon/smithy/swift/codegen/manifestanddocs/DependencyJSONGeneratorTests.kt
View file
Open in desktop

This file was deleted.

10 changes: 5 additions & 5 deletions ...lin/software/amazon/smithy/swift/codegen/manifestanddocs/PackageManifestGeneratorTests.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,15 +98,15 @@ class PackageManifestGeneratorTests {
smithyFile: String,
serviceShapeId: String,
): TestContext {
val context =
val testContext =
TestContext.initContextFrom(smithyFile, serviceShapeId, MockHTTPAWSJson11ProtocolGenerator()) { model ->
model.defaultSettings(serviceShapeId, "MockSDK", "2019-12-16", "MockSDKID")
}
context.generationCtx.delegator.useFileWriter("xyz.swift") { writer ->
testContext.generationCtx.delegator.useFileWriter("xyz.swift") { writer ->
writer.addDependency(SwiftDependency.CLIENT_RUNTIME)
}
PackageManifestGenerator(context.generationCtx).writePackageManifest(context.generationCtx.delegator.dependencies)
context.generationCtx.delegator.flushWriters()
return context
PackageManifestGenerator(testContext.context).writePackageManifest(testContext.generationCtx.delegator.dependencies)
testContext.generationCtx.delegator.flushWriters()
return testContext
Copy link
Contributor Author

@jbelkins jbelkins Nov 17, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Test setup modified to use the GenerationContext instead of a ProtocolGenerator.GenerationContext to match the changes in PackageManifestGenerator.

See just below for where the context: GenerationContext property is added to TestContext.

}
}
Loading