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

feat: Service client release notes #1783

Merged
merged 20 commits into from
Oct 8, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
dc80f19
Add features struct and feature struct for grabbing info we need from…
Sep 27, 2024
e117102
Add service feature & service documentation sections to release notes…
Sep 27, 2024
eb4fed9
Update existing code that uses ReleaseNotesBuilder to reflect new cha…
Sep 27, 2024
51d58ec
Update test that fails; possibly from previous package manifest change.
Sep 27, 2024
c8891ee
Add regression tests for release notes builder.
Sep 27, 2024
55146a6
Only build service client sections in release notes if the repo is aw…
Sep 27, 2024
956e8ff
Add test flag (grr) to ReleaseNotesBuilder to allow tests. In real sc…
Sep 28, 2024
48710f7
Merge branch 'main' into feat/service-release-notes
Oct 2, 2024
cedd3ea
Merge branch 'main' into feat/service-release-notes
Oct 3, 2024
21177ef
Resolve Josh's PR comments
Oct 3, 2024
bda2b0e
Temporarily comment out repo has change check for running E2E test.
Oct 4, 2024
4dc331d
Uncomment temporarily commented logic for generating empty manfiest i…
Oct 4, 2024
f2c6d4f
Change SDK changes section name from AWS SDK for Swift to Miscellaneo…
Oct 4, 2024
4e1181e
Merge branch 'main' into feat/service-release-notes
Oct 4, 2024
0909578
Handle null value for releaseNotes and add test for it.
Oct 4, 2024
e467853
Merge branch 'main' into feat/service-release-notes
Oct 4, 2024
be86e44
Temporarily disable diff checker for running E2E tests
Oct 7, 2024
6898fe3
Uncomment temporarily commented out diff logic.
Oct 7, 2024
6e703d2
Merge branch 'main' into feat/service-release-notes
Oct 7, 2024
41d7253
Merge branch 'main' into feat/service-release-notes
sichanyoo Oct 8, 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
Original file line number Diff line number Diff line change
Expand Up @@ -236,7 +236,7 @@ struct PrepareRelease {
) throws {
let commits = try Process.git.listOfCommitsBetween("HEAD", "\(previousVersion)")

let releaseNotes = ReleaseNotesBuilder(
let releaseNotes = try ReleaseNotesBuilder(
previousVersion: previousVersion,
newVersion: newVersion,
repoOrg: repoOrg,
Expand Down
50 changes: 50 additions & 0 deletions AWSSDKSwiftCLI/Sources/AWSSDKSwiftCLI/Models/Features.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
//
// Copyright Amazon.com Inc. or its affiliates.
// All Rights Reserved.
//
// SPDX-License-Identifier: Apache-2.0
//

import Foundation
import AWSCLIUtils

struct FeaturesReader: Decodable {
private let requestFilePath: String
private let mappingFilePath: String

public init(
requestFilePath: String = "../build-request.json",
mappingFilePath: String = "../feature-service-id.json"
) {
self.requestFilePath = requestFilePath
self.mappingFilePath = mappingFilePath
}

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

public func getFeaturesIDToServiceNameDictFromFile() throws -> [String: String] {
let fileContents = try FileManager.default.loadContents(atPath: mappingFilePath)
return try JSONDecoder().decode([String: String].self, from: fileContents)
}
}

struct Features: Decodable {
let features: [Feature]
}

struct Feature: Decodable {
let releaseNotes: String?
Copy link
Contributor Author

@sichanyoo sichanyoo Oct 4, 2024

Choose a reason for hiding this comment

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

Made releaseNotes optional !!!!!!

let featureMetadata: FeatureMetadata

struct FeatureMetadata: Decodable {
let trebuchet: Trebuchet

struct Trebuchet: Decodable {
let featureId: String
let featureType: String
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -15,24 +15,62 @@ struct ReleaseNotesBuilder {
let repoOrg: PrepareRelease.Org
let repoType: PrepareRelease.Repo
let commits: [String]

var featuresReader: FeaturesReader = FeaturesReader()

// MARK: - Build

func build() -> String {
let contents = [
"## What's Changed",
buildCommits(),
.newline,
"**Full Changelog**: https://github.com/\(repoOrg.rawValue)/\(repoType.rawValue)/compare/\(previousVersion)...\(newVersion)"

func build() throws -> String {
let sdkChanges: [String] = buildSDKChangeSection()
let serviceClientChanges = repoType == .awsSdkSwift ? (try buildServiceChangeSection()) : []
let fullCommitLogLink = [
"\n**Full Changelog**: https://github.com/\(repoOrg.rawValue)/\(repoType.rawValue)/compare/\(previousVersion)...\(newVersion)"
]
let contents = ["## What's Changed"] + serviceClientChanges + sdkChanges + fullCommitLogLink
return contents.joined(separator: .newline)
}

// Adds a preceding `*` to each commit string
// This renders the list of commits as a list in markdown
func buildCommits() -> String {
commits
.map { "* \($0)"}

func buildSDKChangeSection() -> [String] {
let formattedCommits = commits
.filter { $0.hasPrefix("feat") || $0.hasPrefix("fix") }
.map { "* \($0)" }
.joined(separator: .newline)
if (!formattedCommits.isEmpty) {
return ["### Miscellaneous", formattedCommits]
}
return []
}

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

private func buildServiceFeatureSection(
_ features: Features,
_ mapping: [String: String]
) -> [String] {
let formattedFeatures = features.features
.filter { $0.featureMetadata.trebuchet.featureType == "NEW_FEATURE" }
.map { "* **AWS \(mapping[$0.featureMetadata.trebuchet.featureId]!)**: \($0.releaseNotes ?? "No description provided.")" }
.joined(separator: .newline)
if (!formattedFeatures.isEmpty) {
return ["### Service Features", formattedFeatures]
}
return []
}

private func buildServiceDocSection(
_ features: Features,
_ mapping: [String: String]
) -> [String] {
let formattedDocUpdates = features.features
.filter { $0.featureMetadata.trebuchet.featureType == "DOC_UPDATE" }
.map { "* **AWS \(mapping[$0.featureMetadata.trebuchet.featureId]!)**: \($0.releaseNotes ?? "No description provided.")" }
.joined(separator: .newline)
if (!formattedDocUpdates.isEmpty) {
return ["### Service Documentation", formattedDocUpdates]
}
return []
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,16 @@ class PrepareReleaseTests: CLITestCase {
createPackageVersion(previousVersion)
createNextPackageVersion(newVersion)

let buildRequest = """
{
"features": []
}
"""
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))

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,6 @@ class PackageManifestBuilderTests: XCTestCase {

let expected = """
<contents of prefix>

// MARK: - Dynamic Content

let clientRuntimeVersion: Version = "1.2.3"
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,218 @@
//
// Copyright Amazon.com Inc. or its affiliates.
// All Rights Reserved.
//
// SPDX-License-Identifier: Apache-2.0
//

@testable import AWSSDKSwiftCLI
import AWSCLIUtils
import XCTest

/*
* Regression tests for protection against change in generated release notes markdown content.
*/
class ReleaseNotesBuilderTests: XCTestCase {
/* Reusable feature strings */

// New feature 1
private let feature1 = """
{
"releaseNotes": "New feature description A.",
"featureMetadata": {
"trebuchet": {
"featureId": "feature-id-a",
"featureType": "NEW_FEATURE",
}
}
}
"""

// New feature 2
private let feature2 = """
{
"releaseNotes": "New feature description B.",
"featureMetadata": {
"trebuchet": {
"featureId": "feature-id-b",
"featureType": "NEW_FEATURE",
}
}
}
"""

// Documentation update
private let feature3 = """
{
"releaseNotes": "Doc update description C.",
"featureMetadata": {
"trebuchet": {
"featureId": "feature-id-c",
"featureType": "DOC_UPDATE",
}
}
}
"""

// Feature with null releaseNotes field
private let feature4 = """
{
"releaseNotes": null,
Copy link
Contributor

Choose a reason for hiding this comment

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

This is how preview builds encode release notes: the releaseNotes key is present with a null value?

Do we need to test where the releaseNotes key is omitted entirely?

Copy link
Contributor

Choose a reason for hiding this comment

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

(I think an autogenerated Swift decoder will decode either case to a String? value of nil.)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, it's releaseNotes key present but with a null value.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

And yeah I think in either case Swift decoder just assigns nil as well

"featureMetadata": {
"trebuchet": {
"featureId": "feature-id-d",
"featureType": "DOC_UPDATE",
}
}
}
"""

// Dictionary of feature ID to name of the service
private let mapping = """
{
"feature-id-a": "Service 1",
"feature-id-b": "Service 2",
"feature-id-c": "Service 3",
"feature-id-d": "Service 4"
}
"""

func testAllSectionsPresent() throws {
let buildRequest = """
{ "features": [\(feature1), \(feature2), \(feature3)] }
"""
setUpBuildRequestAndMappingJSONs(buildRequest, mapping)
let builder = try setUpBuilder(testCommits: ["fix: Fix X", "feat: Feat Y"])
let releaseNotes = try builder.build()
let expected = """
## What's Changed
### Service Features
* **AWS Service 1**: New feature description A.
* **AWS Service 2**: New feature description B.
### Service Documentation
* **AWS Service 3**: Doc update description C.
### Miscellaneous
* fix: Fix X
* feat: Feat Y

**Full Changelog**: https://github.com/awslabs/aws-sdk-swift/compare/1.0.0...1.0.1
"""
XCTAssertEqual(releaseNotes, expected)
}

func testNoServiceFeatureSectionPresent() throws {
let buildRequest = """
{ "features": [\(feature3)] }
"""
setUpBuildRequestAndMappingJSONs(buildRequest, mapping)
let builder = try setUpBuilder(testCommits: ["fix: Fix X", "feat: Feat Y"])
let releaseNotes = try builder.build()
let expected = """
## What's Changed
### Service Documentation
* **AWS Service 3**: Doc update description C.
### Miscellaneous
* fix: Fix X
* feat: Feat Y

**Full Changelog**: https://github.com/awslabs/aws-sdk-swift/compare/1.0.0...1.0.1
"""
XCTAssertEqual(releaseNotes, expected)
}

func testNoServiceDocSectionPresent() throws {
let buildRequest = """
{ "features": [\(feature1), \(feature2)] }
"""
setUpBuildRequestAndMappingJSONs(buildRequest, mapping)
let builder = try setUpBuilder(testCommits: ["fix: Fix X", "feat: Feat Y"])
let releaseNotes = try builder.build()
let expected = """
## What's Changed
### Service Features
* **AWS Service 1**: New feature description A.
* **AWS Service 2**: New feature description B.
### Miscellaneous
* fix: Fix X
* feat: Feat Y

**Full Changelog**: https://github.com/awslabs/aws-sdk-swift/compare/1.0.0...1.0.1
"""
XCTAssertEqual(releaseNotes, expected)
}

func testNoSDKChangeSectionPresent() throws {
let buildRequest = """
{ "features": [\(feature1), \(feature2), \(feature3)] }
"""
setUpBuildRequestAndMappingJSONs(buildRequest, mapping)
let builder = try setUpBuilder()
let releaseNotes = try builder.build()
let expected = """
## What's Changed
### Service Features
* **AWS Service 1**: New feature description A.
* **AWS Service 2**: New feature description B.
### Service Documentation
* **AWS Service 3**: Doc update description C.

**Full Changelog**: https://github.com/awslabs/aws-sdk-swift/compare/1.0.0...1.0.1
"""
XCTAssertEqual(releaseNotes, expected)
}

func testNoSectionsPresentAndIrrelevantCommitsAreFiltered() throws {
let buildRequest = """
{ "features":[] }
"""
setUpBuildRequestAndMappingJSONs(buildRequest, mapping)
let builder = try setUpBuilder(testCommits: ["chore: Commit A", "Update X"])
let releaseNotes = try builder.build()
let expected = """
## What's Changed

**Full Changelog**: https://github.com/awslabs/aws-sdk-swift/compare/1.0.0...1.0.1
"""
XCTAssertEqual(releaseNotes, expected)
}

func testNullReleaseNotesFieldGetsHandledWithoutError() throws {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Test added for null releaseNotes case

let buildRequest = """
{ "features": [\(feature4)] }
"""
setUpBuildRequestAndMappingJSONs(buildRequest, mapping)
let builder = try setUpBuilder()
let releaseNotes = try builder.build()
let expected = """
## What's Changed
### Service Documentation
* **AWS Service 4**: No description provided.

**Full Changelog**: https://github.com/awslabs/aws-sdk-swift/compare/1.0.0...1.0.1
"""
XCTAssertEqual(releaseNotes, expected)
}

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

private func setUpBuilder(testCommits: [String] = []) throws -> ReleaseNotesBuilder {
return try ReleaseNotesBuilder(
previousVersion: Version("1.0.0"),
newVersion: Version("1.0.1"),
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"
)
)
}
}
Loading