-
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
chore: Run CLI unit tests in CI #1799
Conversation
@@ -6,7 +6,6 @@ | |||
// | |||
|
|||
import Foundation | |||
import PackageDescription |
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.
This is an old remnant of the AWSSDKSwiftCLI dependency on the swift-package-manager
package.
@@ -9,7 +9,7 @@ let package = Package( | |||
], | |||
dependencies: [ | |||
.package(url: "https://github.com/apple/swift-argument-parser", from: "1.2.0"), | |||
.package(url: "https://github.com/awslabs/aws-sdk-swift", from: "0.46.0"), | |||
.package(url: "https://github.com/awslabs/aws-sdk-swift", from: "1.0.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.
GA AWS SDK now used by the SPRCLI tools.
.appending(component: version) | ||
.appendingPathComponent(scope) | ||
.appendingPathComponent(name) | ||
.appendingPathComponent(version) |
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.
You will see several places where the method appending(component:)
is replaced by the older & now deprecated .appendingPathComponent(_:)
so that these tools will compile on Swift 5.9.
@@ -47,7 +47,8 @@ extension SPRPublisher { | |||
} | |||
|
|||
private func createMetadata() -> PackageInfo { | |||
let now = Date().ISO8601Format() | |||
let formatter = ISO8601DateFormatter() | |||
let now = formatter.string(from: Date()) |
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.
Another new method replaced by an older one for compatibility with older Swift.
@@ -8,25 +8,26 @@ | |||
import Foundation | |||
import AWSCLIUtils | |||
|
|||
struct FeaturesReader: Decodable { | |||
private let requestFilePath: String | |||
private let mappingFilePath: String |
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.
Instead of taking individual paths to files, this type assumes the individual files are in the parent of the current working directory (which will be the repo's root).
mappingFilePath: "feature-service-id.json" | ||
) | ||
features: FeaturesReader().getFeaturesFromFile(), | ||
featuresIDToServiceName: FeaturesReader().getFeaturesIDToServiceNameDictFromFile() |
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.
During tests, the FeaturesReader just reads the feature & map files out of the temp project root created for testing.
@@ -15,7 +15,8 @@ struct ReleaseNotesBuilder { | |||
let repoOrg: PrepareRelease.Org | |||
let repoType: PrepareRelease.Repo | |||
let commits: [String] | |||
var featuresReader: FeaturesReader = FeaturesReader() | |||
let features: Features | |||
let featuresIDToServiceName: [String: String] |
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.
ReleaseNotesBuilders takes the feature list & mapping as inputs rather than reading them itself.
@@ -15,7 +15,7 @@ jobs: | |||
matrix: | |||
# This matrix runs tests on iOS sim & Mac, on oldest & newest supported Xcodes | |||
runner: | |||
- macos-13 | |||
- macos-14 |
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.
macos-14 is used for running min Xcode instead of macos-13. This matches the macOS used in Catapult.
swift test | ||
cd ../SPRCLI | ||
unset AWS_SWIFT_SDK_USE_LOCAL_DEPS | ||
swift build |
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.
Macs test & build CLI tools only directly on the Mac, not on simulators.
swift test | ||
cd ../SPRCLI | ||
unset AWS_SWIFT_SDK_USE_LOCAL_DEPS | ||
swift build |
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.
All Linux jobs build & test the CLI tools.
@@ -235,13 +235,16 @@ struct PrepareRelease { | |||
previousVersion: Version | |||
) throws { | |||
let commits = try Process.git.listOfCommitsBetween("HEAD", "\(previousVersion)") | |||
|
|||
let featuresReader = FeaturesReader(repoPath: repoPath) |
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.
PrepareRelease configures & runs the FeatureReader with its own repoPath
, then the features & mapping are injected into the ReleaseNotesBuilder.
let features = try featuresReader.getFeaturesFromFile() | ||
let mapping = try featuresReader.getFeaturesIDToServiceNameDictFromFile() | ||
return buildServiceFeatureSection(features, mapping) + buildServiceDocSection(features, mapping) | ||
return buildServiceFeatureSection(features, featuresIDToServiceName) + buildServiceDocSection(features, featuresIDToServiceName) |
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.
Here, the injected features & mapping are used directly to generate the release notes content.
@@ -65,7 +65,7 @@ public struct SPRPublisher { | |||
} | |||
|
|||
private mutating func setOptions() async { | |||
await SDKLoggingSystem.initialize(logLevel: .error) | |||
await SDKLoggingSystem().initialize(logLevel: .error) |
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.
We changed this interface before GA.
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.
Will approve when E2E tests pass
This reverts commit cdd65ac.
@@ -235,13 +235,16 @@ struct PrepareRelease { | |||
previousVersion: Version | |||
) throws { | |||
let commits = try Process.git.listOfCommitsBetween("HEAD", "\(previousVersion)") | |||
|
|||
let featuresReader = FeaturesReader() | |||
|
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.
Features reader is used to read the build message & mapping, then the read data is passed into the release notes builder.
@@ -12,7 +12,7 @@ import XCTest | |||
/* | |||
* Regression tests for protection against change in generated release notes markdown content. | |||
*/ | |||
class ReleaseNotesBuilderTests: XCTestCase { | |||
class ReleaseNotesBuilderTests: CLITestCase { |
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.
This test now inherits from CLITestCase, which sets up a temporary directory structure that allows the Trebuchet files to be written to the workspace directory.
@@ -11,8 +11,14 @@ import AWSCLIUtils | |||
|
|||
class CLITestCase: XCTestCase { | |||
let tmpPath = "tmp" | |||
let projectPath = "aws-sdk-swift-or-smithy-swift" |
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 directory at tmpPath
now serves as the "workspace" dir.
A "project directory" named projectPath
is created within.
This structure allows Trebuchet build files to be written to the same relative location in tests as in actual use.
Issue #
#1732
Description of changes
I discovered that the CLI unit tests were failing due to an inability to read the build message & mapping files during tests. The test is fixed, and CLI unit tests are added to CI for visibility going forward.
Specific changes:
AWSSDKSwiftCLI
to continuous integration. Tests are run on Mac and Linux, oldest & newest OS. (CLI unit tests are not run on any Apple platform simulators.)SPRCLI
project does not contain unit tests, but it is built to ensure it builds without error.FeaturesReader
type is modified to ensure that it accesses the right location during testing.New/existing dependencies impact assessment, if applicable
No new dependencies were added to this change.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.