-
Notifications
You must be signed in to change notification settings - Fork 35
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.
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
base: main
Are you sure you want to change the base?
Conversation
| ) { | ||
| fun writePackageManifest(dependencies: List<SymbolDependency>) { | ||
| ctx.delegator.useFileWriter("Package.swift") { writer -> | ||
| ctx.writerDelegator().useFileWriter("Package.swift") { writer -> |
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 PackageManifestGenerator now uses the non-protocol generation context, since it is not part of any protocol.
| val visibility: String, | ||
| val internalClient: Boolean, | ||
| val generatePackageManifest: Boolean, | ||
| val generateDependencyJSON: Boolean, |
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.
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.
| emptyList(), | ||
| delegator, | ||
| ) | ||
|
|
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.
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.
| return context | ||
| PackageManifestGenerator(testContext.context).writePackageManifest(testContext.generationCtx.delegator.dependencies) | ||
| testContext.generationCtx.delegator.flushWriters() | ||
| return testContext |
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.
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.
|
|
||
| LOGGER.info("[${service.id}] Generating unit tests for protocol ${this.protocol}") | ||
| val numProtocolUnitTestsGenerated = generateProtocolUnitTests(ctx) | ||
| shouldGenerateTestTarget = (numProtocolUnitTestsGenerated > 0) |
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.
- 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
Description of changes
These changes support moving service clients from being committed to the project to being an opt-in feature to be utilized only during development & distribution.
Dependencies.jsonto aws-sdk-swift since this file is only used by that projectSwiftSettingsfields associated withPackage.swiftandDependencies.jsonsince they are no longer needed.Scope
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.