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

chore: Run CLI unit tests in CI #1799

Merged
merged 23 commits into from
Oct 23, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
231adf4
chore: Run CLI unit tests in CI
jbelkins Oct 23, 2024
f18b7e8
Fix 'Failed to load data for file at path ../build-request.json' error
jbelkins Oct 23, 2024
dd803d8
Fix SPRCLI tests, fix path methods
jbelkins Oct 23, 2024
e26ee0d
Remove reference to PackageDescription from SPRCLI
jbelkins Oct 23, 2024
5c17a4d
Use older form of file URL initializer
jbelkins Oct 23, 2024
e6aa350
Fix SPRCLI build, move it to AWS SDK 1.0+
jbelkins Oct 23, 2024
45a3bbf
Move CI from macos-13 to macos-14
jbelkins Oct 23, 2024
1288bf2
build SPRCLI only, no tests
jbelkins Oct 23, 2024
c5bffb5
Run CLI unit tests on Linux too
jbelkins Oct 23, 2024
17f8c72
Update path references
jbelkins Oct 23, 2024
84d9220
Fix date formatter
jbelkins Oct 23, 2024
4bac8e3
Merge remote-tracking branch 'origin/main' into jbe/cli_unti_tests_in_ci
jbelkins Oct 23, 2024
cdd65ac
Dummy build request & mapping files added
jbelkins Oct 23, 2024
9782279
FeaturesReader now always reads from CWD
jbelkins Oct 23, 2024
3bba2eb
Revert "Dummy build request & mapping files added"
jbelkins Oct 23, 2024
d3cdd34
Code & tests now fixed to read Trebuchet files from parent of project…
jbelkins Oct 23, 2024
465ee53
Revert "Revert "Dummy build request & mapping files added""
jbelkins Oct 23, 2024
5604554
chore: Updates version to 1.0.27
jbelkins Oct 23, 2024
835f097
Remove Trebuchet build files
jbelkins Oct 23, 2024
3ed9e1d
Merge remote-tracking branch 'origin/main' into jbe/cli_unti_tests_in_ci
jbelkins Oct 23, 2024
cc376b2
Fix comment
jbelkins Oct 23, 2024
df03510
Revert version files
jbelkins Oct 23, 2024
681870b
Revert version files
jbelkins Oct 23, 2024
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
19 changes: 17 additions & 2 deletions .github/workflows/continuous-integration.yml
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ jobs:
matrix:
# This matrix runs tests on iOS sim & Mac, on oldest & newest supported Xcodes
runner:
- macos-13
- macos-14
Copy link
Contributor Author

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.

- macos-15
xcode:
- Xcode_15.2
Expand All @@ -30,7 +30,7 @@ jobs:
- 'platform=macOS'
exclude:
# Don't run old macOS with new Xcode
- runner: macos-13
- runner: macos-14
xcode: Xcode_16
# Don't run new macOS with old Xcode
- runner: macos-15
Expand Down Expand Up @@ -101,6 +101,14 @@ jobs:
java-version: 17
- name: Tools Versions
run: ./scripts/ci_steps/log_tool_versions.sh
- name: Run CLI Unit Tests
if: ${{ matrix.destination == 'platform=macOS' }}
run: |
cd AWSSDKSwiftCLI
swift test
cd ../SPRCLI
unset AWS_SWIFT_SDK_USE_LOCAL_DEPS
swift build
Copy link
Contributor Author

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.

- name: Prepare Protocol & Unit Tests
run: |
./scripts/ci_steps/prepare_protocol_and_unit_tests.sh
Expand Down Expand Up @@ -182,6 +190,13 @@ jobs:
run: ./scripts/ci_steps/install_native_linux_dependencies.sh
- name: Tools Versions
run: ./scripts/ci_steps/log_tool_versions.sh
- name: Run CLI Unit Tests
run: |
cd AWSSDKSwiftCLI
swift test
cd ../SPRCLI
unset AWS_SWIFT_SDK_USE_LOCAL_DEPS
swift build
Copy link
Contributor Author

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.

- name: Prepare Protocol & Unit Tests
run: |
./scripts/ci_steps/prepare_protocol_and_unit_tests.sh
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ struct PrepareRelease {
let diffChecker: DiffChecker

/// Prepares a release for the specified repository.
/// If the respository doesn't have any changes, then this does nothing.
/// If the repository doesn't have any changes, then this does nothing.
func run() throws {
try FileManager.default.changeWorkingDirectory(repoPath)

Expand Down Expand Up @@ -235,13 +235,16 @@ struct PrepareRelease {
previousVersion: Version
) throws {
let commits = try Process.git.listOfCommitsBetween("HEAD", "\(previousVersion)")

let featuresReader = FeaturesReader()

Copy link
Contributor Author

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.

let releaseNotes = try ReleaseNotesBuilder(
previousVersion: previousVersion,
newVersion: newVersion,
repoOrg: repoOrg,
repoType: repoType,
commits: commits
commits: commits,
features: featuresReader.getFeaturesFromFile(),
featuresIDToServiceName: featuresReader.getFeaturesIDToServiceNameDictFromFile()
).build()

let manifest = ReleaseManifest(
Expand Down
19 changes: 6 additions & 13 deletions AWSSDKSwiftCLI/Sources/AWSSDKSwiftCLI/Models/Features.swift
Original file line number Diff line number Diff line change
Expand Up @@ -8,25 +8,18 @@
import Foundation
import AWSCLIUtils

struct FeaturesReader: Decodable {
private let requestFilePath: String
private let mappingFilePath: String
Copy link
Contributor Author

@jbelkins jbelkins Oct 23, 2024

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).


public init(
requestFilePath: String = "../build-request.json",
mappingFilePath: String = "../feature-service-id.json"
) {
self.requestFilePath = requestFilePath
self.mappingFilePath = mappingFilePath
}
/// Reads the Trebuchet request & service mapping files from the parent of the current working directory.
struct FeaturesReader {
private let requestFile = "../build-request.json"
private let mappingFile = "../feature-service-id.json"

public func getFeaturesFromFile() throws -> Features {
let fileContents = try FileManager.default.loadContents(atPath: requestFilePath)
let fileContents = try FileManager.default.loadContents(atPath: requestFile)
return try JSONDecoder().decode(Features.self, from: fileContents)
}

public func getFeaturesIDToServiceNameDictFromFile() throws -> [String: String] {
let fileContents = try FileManager.default.loadContents(atPath: mappingFilePath)
let fileContents = try FileManager.default.loadContents(atPath: mappingFile)
return try JSONDecoder().decode([String: String].self, from: fileContents)
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Copy link
Contributor Author

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.


// MARK: - Build

Expand All @@ -41,9 +42,7 @@ struct ReleaseNotesBuilder {
}

func buildServiceChangeSection() throws -> [String] {
let features = try featuresReader.getFeaturesFromFile()
let mapping = try featuresReader.getFeaturesIDToServiceNameDictFromFile()
return buildServiceFeatureSection(features, mapping) + buildServiceDocSection(features, mapping)
return buildServiceFeatureSection(features, featuresIDToServiceName) + buildServiceDocSection(features, featuresIDToServiceName)
Copy link
Contributor Author

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.

}

private func buildServiceFeatureSection(
Expand Down
11 changes: 11 additions & 0 deletions AWSSDKSwiftCLI/Tests/AWSSDKSwiftCLITests/CLITestCase.swift
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,14 @@ import AWSCLIUtils

class CLITestCase: XCTestCase {
let tmpPath = "tmp"
let projectPath = "aws-sdk-swift-or-smithy-swift"
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 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.

private var originalWorkingDirectory: String!

/// Creates a temp directory that contains a project dir.
///
/// The project dir is set as CWD when setup is complete.
/// This folder structure permits Trebuchet artifacts to be written in the parent of the project directory.
/// At the conclusion of the test, the tear-down method deletes the entire temp directory.
override func setUp() {
super.setUp()
try? FileManager.default.removeItem(atPath: tmpPath)
Expand All @@ -23,6 +29,11 @@ class CLITestCase: XCTestCase {
)
originalWorkingDirectory = FileManager.default.currentDirectoryPath
try! FileManager.default.changeWorkingDirectory(tmpPath)
try! FileManager.default.createDirectory(
atPath: projectPath,
withIntermediateDirectories: false
)
try! FileManager.default.changeWorkingDirectory(projectPath)
}

override func tearDown() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,10 +50,10 @@ class PrepareReleaseTests: CLITestCase {
"features": []
}
"""
FileManager.default.createFile(atPath: "build-request.json", contents: Data(buildRequest.utf8))
FileManager.default.createFile(atPath: "../build-request.json", contents: Data(buildRequest.utf8))

let mapping = "{}"
FileManager.default.createFile(atPath: "feature-service-id.json", contents: Data(mapping.utf8))
FileManager.default.createFile(atPath: "../feature-service-id.json", contents: Data(mapping.utf8))

let subject = PrepareRelease.mock(repoType: .awsSdkSwift, diffChecker: { _,_ in true })
try! subject.run()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import XCTest
/*
* Regression tests for protection against change in generated release notes markdown content.
*/
class ReleaseNotesBuilderTests: XCTestCase {
class ReleaseNotesBuilderTests: CLITestCase {
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 test now inherits from CLITestCase, which sets up a temporary directory structure that allows the Trebuchet files to be written to the workspace directory.

/* Reusable feature strings */

// New feature 1
Expand Down Expand Up @@ -195,10 +195,9 @@ class ReleaseNotesBuilderTests: XCTestCase {

private func setUpBuildRequestAndMappingJSONs(_ buildRequest: String, _ mapping: String) {
// In real scenario, the JSON files we need are located one level above, in the workspace directory.
// For tests, due to sandboxing, the dummy files are created in current directory instead of
// in parent directory.
FileManager.default.createFile(atPath: "build-request.json", contents: Data(buildRequest.utf8))
FileManager.default.createFile(atPath: "feature-service-id.json", contents: Data(mapping.utf8))
// So, the dummy files are created in the parent of the current directory to match a real build.
FileManager.default.createFile(atPath: "../build-request.json", contents: Data(buildRequest.utf8))
FileManager.default.createFile(atPath: "../feature-service-id.json", contents: Data(mapping.utf8))
}

private func setUpBuilder(testCommits: [String] = []) throws -> ReleaseNotesBuilder {
Expand All @@ -208,11 +207,9 @@ class ReleaseNotesBuilderTests: XCTestCase {
repoOrg: .awslabs,
repoType: .awsSdkSwift,
commits: testCommits,
// Parametrize behavior of FeaturesReader with paths used to create JSON test files
featuresReader: FeaturesReader(
requestFilePath: "build-request.json",
mappingFilePath: "feature-service-id.json"
)
// Parameterize behavior of FeaturesReader with paths used to create JSON test files
features: FeaturesReader().getFeaturesFromFile(),
featuresIDToServiceName: FeaturesReader().getFeaturesIDToServiceNameDictFromFile()
Copy link
Contributor Author

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.

)
}
}
2 changes: 1 addition & 1 deletion SPRCLI/Package.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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"),
Copy link
Contributor Author

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.

.package(path: "../AWSSDKSwiftCLI"),
],
targets: [
Expand Down
1 change: 0 additions & 1 deletion SPRCLI/Sources/SPR/Process+SPR.swift
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
//

import Foundation
import PackageDescription
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 is an old remnant of the AWSSDKSwiftCLI dependency on the swift-package-manager package.

import AWSCLIUtils

extension Process {
Expand Down
2 changes: 1 addition & 1 deletion SPRCLI/Sources/SPR/SPRPublisher.swift
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ public struct SPRPublisher {
}

private mutating func setOptions() async {
await SDKLoggingSystem.initialize(logLevel: .error)
await SDKLoggingSystem().initialize(logLevel: .error)
Copy link
Contributor Author

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.

let env = ProcessInfo.processInfo.environment
bucket = bucket ?? env["AWS_SDK_SPR_BUCKET"]
if region.isEmpty {
Expand Down
6 changes: 3 additions & 3 deletions SPRCLI/Sources/SPR/UpdateList.swift
Original file line number Diff line number Diff line change
Expand Up @@ -63,9 +63,9 @@ extension SPRPublisher {
throw Error("URL is invalid")
}
return baseURL
.appending(component: scope)
.appending(component: name)
.appending(component: version)
.appendingPathComponent(scope)
.appendingPathComponent(name)
.appendingPathComponent(version)
Copy link
Contributor Author

@jbelkins jbelkins Oct 23, 2024

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.

}
}
}
2 changes: 1 addition & 1 deletion SPRCLI/Sources/SPR/UploadArchive.swift
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ extension SPRPublisher {

mutating func uploadArchive() async throws {
let tmpDirFileURL = FileManager.default.temporaryDirectory
let archiveFileURL = tmpDirFileURL.appending(component: "\(UUID().uuidString).zip")
let archiveFileURL = tmpDirFileURL.appendingPathComponent("\(UUID().uuidString).zip")
let archiveProcess = Process.SPR.archive(name: name, packagePath: path, archiveFileURL: archiveFileURL)
_ = try _runReturningStdOut(archiveProcess)
guard FileManager.default.fileExists(atPath: urlPath(archiveFileURL)) else {
Expand Down
2 changes: 1 addition & 1 deletion SPRCLI/Sources/SPR/UploadManifest.swift
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ extension SPRPublisher {

func uploadManifest() async throws {
let packageFileURL = URL(fileURLWithPath: path).standardizedFileURL
let manifestFileURL = packageFileURL.appending(component: "Package.swift")
let manifestFileURL = packageFileURL.appendingPathComponent("Package.swift")
let s3Client = try S3Client(region: region)
try await verify(s3Client: s3Client)
try await upload(s3Client: s3Client, manifestFileURL: manifestFileURL)
Expand Down
3 changes: 2 additions & 1 deletion SPRCLI/Sources/SPR/UploadMetadata.swift
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,8 @@ extension SPRPublisher {
}

private func createMetadata() -> PackageInfo {
let now = Date().ISO8601Format()
let formatter = ISO8601DateFormatter()
let now = formatter.string(from: Date())
Copy link
Contributor Author

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.

let organization = PackageInfo.Metadata.Author.Organization(name: "Amazon Web Services", email: nil, description: nil, url: URL(string: "https://aws.amazon.com/")!)
let author = PackageInfo.Metadata.Author(name: "AWS SDK for Swift Team", email: nil, description: nil, organization: organization, url: nil)
let resource = Resource(name: "source-archive", type: "application/zip", checksum: checksum, signing: nil)
Expand Down
Loading