Skip to content

Commit 06e000f

Browse files
authored
Adopt Swift Testing in metadata extractor tests (#51)
* First, perform a "direct" conversion, making only the most minimal set of changes which are strictly necessary to convert XCTest API usages into their Swift Testing counterparts * Adopt raw identifiers to make suite and test function names more readable and idiomatic * Make test helper functions private * Move helper functions out of struct in anticipation of a subsequent refactor where they will be needed at other scopes * Parameterize `Breaking changes` test * Decompose `All proposals` @test function into three smaller and more focused test functions in a sub-suite * Decompose `Warnings and errors` @test function into two smaller and more focused test functions in a new sub-suite named `Malformed proposals` (after the snapshot it references) * Parameterize the `Bad dates` @test function. I also considered doing this for `Good dates` test function but it has many more arguments and its performance in Xcode is somewhat poor currently * Finally, remove the old XCTest file * Incorporate PR feedback
1 parent c8782ce commit 06e000f

File tree

1 file changed

+127
-108
lines changed

1 file changed

+127
-108
lines changed
Lines changed: 127 additions & 108 deletions
Original file line numberDiff line numberDiff line change
@@ -1,168 +1,187 @@
11

22
// This source file is part of the Swift.org open source project
33
//
4-
// Copyright (c) 2024 Apple Inc. and the Swift project authors
4+
// Copyright (c) 2025 Apple Inc. and the Swift project authors
55
// Licensed under Apache License v2.0 with Runtime Library Exception
66
//
77
// See https://swift.org/LICENSE.txt for license information
88
// See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors
99

10-
import XCTest
10+
import Testing
1111
import Foundation
1212

1313
@testable import EvolutionMetadataModel
1414
@testable import EvolutionMetadataExtraction
1515

16-
final class ExtractionTests: XCTestCase {
17-
18-
func urlForSnapshot(named snapshotName: String) throws -> URL {
19-
try XCTUnwrap(Bundle.module.url(forResource: snapshotName, withExtension: "evosnapshot", subdirectory: "Resources"), "Unable to find snapshot \(snapshotName).evosnapshot in test bundle resources.")
20-
}
21-
22-
func data(forResource name: String, withExtension ext: String) throws -> Data {
23-
let url = try XCTUnwrap(Bundle.module.url(forResource: name, withExtension: ext, subdirectory: "Resources"), "Unable to find resource \(name).\(ext) in test bundle resources.")
24-
let data = try XCTUnwrap(Data(contentsOf: url), "Unable to read data from \(name).\(ext)")
25-
return data
26-
}
27-
28-
func string(forResource name: String, withExtension ext: String) throws -> String {
29-
let data = try data(forResource: name, withExtension: ext)
30-
let string = try XCTUnwrap(String(data: data, encoding: .utf8), "Unable to make string from contents of \(name).\(ext)")
31-
return string
32-
}
33-
34-
// Convenience to write expected and actual metadata files to disk for comparison in a diff tool
35-
func writeJSONFilesToPath(expected: Data, actual: Data, path: String, prefix: String? = nil) throws {
36-
let filePrefix: String
37-
if let prefix { filePrefix = "\(prefix)-" } else { filePrefix = "" }
38-
try expected.write(to: FileUtilities.expandedAndStandardizedURL(for: path).appending(path: "\(filePrefix)expected.json"))
39-
try actual.write(to: FileUtilities.expandedAndStandardizedURL(for: path).appending(path: "\(filePrefix)actual.json"))
40-
}
16+
@Suite
17+
struct `Extraction Tests` {
18+
19+
struct `All Proposals` {
20+
private var snapshotURL: URL
21+
private var extractionJob: ExtractionJob
22+
private var extractedEvolutionMetadata: EvolutionMetadata
23+
24+
init() async throws {
25+
snapshotURL = try urlForSnapshot(named: "AllProposals")
26+
extractionJob = try await ExtractionJob.makeExtractionJob(from: .snapshot(snapshotURL), output: .none, ignorePreviousResults: true)
27+
extractedEvolutionMetadata = try await EvolutionMetadataExtractor.extractEvolutionMetadata(for: extractionJob)
28+
}
29+
30+
@Test func `Extracted Metadata`() async throws {
31+
let expectedResults = try #require(extractionJob.expectedResults, "Snapshot from source '\(snapshotURL.absoluteString)' does not contain expected results.")
32+
33+
// Check top-level properties
34+
#expect(extractedEvolutionMetadata.commit == expectedResults.commit)
35+
#expect(extractedEvolutionMetadata.creationDate == expectedResults.creationDate)
36+
#expect(extractedEvolutionMetadata.implementationVersions == expectedResults.implementationVersions)
37+
#expect(extractedEvolutionMetadata.schemaVersion == expectedResults.schemaVersion)
38+
#expect(extractedEvolutionMetadata.toolVersion == expectedResults.toolVersion)
39+
}
4140

42-
func testAllProposals() async throws {
43-
44-
let snapshotURL = try urlForSnapshot(named: "AllProposals")
45-
let extractionJob = try await ExtractionJob.makeExtractionJob(from: .snapshot(snapshotURL), output: .none, ignorePreviousResults: true)
46-
let expectedResults = try XCTUnwrap(extractionJob.expectedResults, "Snapshot at '\(snapshotURL.absoluteString)' does not contain expected results.")
47-
let expectedResultsByProposalID = expectedResults.proposals.reduce(into: [:]) { $0[$1.id] = $1 }
48-
49-
let extractedEvolutionMetadata = try await EvolutionMetadataExtractor.extractEvolutionMetadata(for: extractionJob)
50-
51-
// Check top-level properties
52-
XCTAssertEqual(extractedEvolutionMetadata.commit, expectedResults.commit)
53-
XCTAssertEqual(extractedEvolutionMetadata.creationDate, expectedResults.creationDate)
54-
XCTAssertEqual(extractedEvolutionMetadata.implementationVersions, expectedResults.implementationVersions)
55-
XCTAssertEqual(extractedEvolutionMetadata.schemaVersion, expectedResults.schemaVersion)
56-
XCTAssertEqual(extractedEvolutionMetadata.toolVersion, expectedResults.toolVersion)
57-
5841
// Check proposals
59-
for newProposal in extractedEvolutionMetadata.proposals {
60-
let id = newProposal.id
61-
guard !id.isEmpty else {
62-
continue
63-
}
64-
65-
guard let sourceProposal = expectedResultsByProposalID[id] else {
66-
XCTFail("Unable to find id \(id) in expected results.")
67-
continue
42+
@Test func `Expected Proposals`() async throws {
43+
let expectedResults = try #require(extractionJob.expectedResults, "Snapshot at '\(snapshotURL.absoluteString)' does not contain expected results.")
44+
let expectedResultsByProposalID = expectedResults.proposals.reduce(into: [:]) { $0[$1.id] = $1 }
45+
46+
for newProposal in extractedEvolutionMetadata.proposals {
47+
let id = newProposal.id
48+
guard !id.isEmpty else {
49+
continue
50+
}
51+
52+
guard let sourceProposal = expectedResultsByProposalID[id] else {
53+
Issue.record("Unable to find id \(id) in expected results.")
54+
continue
55+
}
56+
57+
#expect(newProposal == sourceProposal)
6858
}
69-
70-
XCTAssertEqual(newProposal, sourceProposal)
7159
}
72-
60+
7361
// Check the generated JSON to catch issues such as removing properties from the schema
74-
let expectedResultsURL = snapshotURL.appending(path: "expected-results.json")
75-
let expectedJSONData = try Data(contentsOf: expectedResultsURL)
76-
let actualJSONData = try extractedEvolutionMetadata.jsonRepresentation
77-
XCTAssertEqual(expectedJSONData, actualJSONData)
78-
79-
// Uncomment to write full JSON files out to compare in a diff tool
80-
// try writeJSONFilesToPath(expected: expectedJSONData, actual: actualJSONData, path: "~/Desktop")
62+
@Test func `Expected Serialization`() throws {
63+
let expectedResultsURL = snapshotURL.appending(path: "expected-results.json")
64+
let expectedJSONData = try Data(contentsOf: expectedResultsURL)
65+
let actualJSONData = try extractedEvolutionMetadata.jsonRepresentation
66+
#expect(expectedJSONData == actualJSONData)
8167

68+
// Uncomment to write full JSON files out to compare in a diff tool
69+
// try writeJSONFilesToPath(expected: expectedJSONData, actual: actualJSONData, path: "~/Desktop")
70+
}
8271
}
83-
84-
func testWarningsAndErrors() async throws {
85-
72+
73+
private static var warningsAndErrorsArguments: Zip2Sequence<[Proposal], [Proposal]> {
74+
get async throws {
8675
let snapshotURL = try urlForSnapshot(named: "Malformed")
8776
let extractionJob = try await ExtractionJob.makeExtractionJob(from: .snapshot(snapshotURL), output: .none, ignorePreviousResults: true)
88-
let expectedResults = try XCTUnwrap(extractionJob.expectedResults, "No expected results found in bundle '\(snapshotURL.absoluteString)'")
89-
90-
// VALIDATION ENHANCEMENT: Tools like Xcode like to add a newline character to empty files
91-
// Possibly instead of using 0007-empty-file.md to test, test separately?
92-
// Or test that the file hasn't been corrupted. (Can you even check it into github?)
93-
77+
let expectedResults = try #require(extractionJob.expectedResults, "No expected results found for extraction job with source '\(extractionJob.source)'")
9478
let extractionMetadata = try await EvolutionMetadataExtractor.extractEvolutionMetadata(for: extractionJob)
95-
96-
// This test zips the extraction results with the expected results
97-
// If the two arrays don't have the same count, the test data itself has an error
98-
XCTAssertEqual(extractionMetadata.proposals.count, expectedResults.proposals.count)
99-
100-
for (actualResult, expectedResult) in zip(extractionMetadata.proposals, expectedResults.proposals) {
101-
XCTAssertEqual(actualResult, expectedResult)
102-
}
79+
80+
// This test zips the extraction results with the expected results. If
81+
// the two arrays don't have the same count, the test data itself may have an error.
82+
try #require(extractionMetadata.proposals.count == expectedResults.proposals.count)
83+
84+
return zip(extractionMetadata.proposals, expectedResults.proposals)
85+
}
10386
}
104-
87+
88+
// VALIDATION ENHANCEMENT: Tools like Xcode like to add a newline character to empty files
89+
// Possibly instead of using 0007-empty-file.md to test, test separately?
90+
// Or test that the file hasn't been corrupted. (Can you even check it into github?)
91+
@Test(arguments: try await warningsAndErrorsArguments)
92+
func `Warnings and Errors`(actualResult: Proposal, expectedResult: Proposal) async throws {
93+
#expect(actualResult == expectedResult)
94+
}
95+
10596
// The lines of text in review-dates-good.txt are status headers from swift-evolution repository history
106-
func testGoodDates() throws {
97+
@Test func `Good Dates`() throws {
10798

10899
let reviewDatesContents = try string(forResource: "review-dates-good", withExtension: "txt")
109100

110101
let statusStrings = reviewDatesContents.split(separator: "\n")
111102
for statusString in statusStrings {
112103
// NOTE: This is something that should be validated!
113104
// It seems a common mistake to leave out closing parenthesis or put strong marker inside closing paren
114-
let match = try XCTUnwrap(statusString.firstMatch(of: /\((.*)\)/), "Every review item in status strings should have parenthesis with contents. \(statusString) DOES NOT MATCH PATTERN" )
105+
let match = try #require(statusString.firstMatch(of: /\((.*)\)/), "Every review item in status strings should have parenthesis with contents. \(statusString) DOES NOT MATCH PATTERN" )
115106

116107
let statusDetail = String(match.1)
117108

118-
XCTAssertNotNil(StatusExtractor.datesForString(statusDetail, processingDate: Date.now), "Unable to parse '\(statusDetail)'")
109+
#expect(StatusExtractor.datesForString(statusDetail, processingDate: Date.now) != nil, "Unable to parse '\(statusDetail)'")
119110
}
120111
}
121112

122-
// The lines of text in review-dates-bad.txt are status headers from swift-evolution repository history
123-
func testBadDates() throws {
124-
113+
@Test(arguments: try {
114+
// The lines of text in review-dates-bad.txt are status headers from swift-evolution repository history
125115
let reviewDatesContents = try string(forResource: "review-dates-bad", withExtension: "txt")
116+
return reviewDatesContents.split(separator: "\n").map(String.init)
117+
}())
118+
func `Bad Dates`(statusString: String) throws {
126119

127-
let statusStrings = reviewDatesContents.split(separator: "\n")
128-
for statusString in statusStrings {
129-
// NOTE: This is something that should be validated!
130-
// It seems a common mistake to leave out closing parenthesis or put strong marker inside closing paren
131-
let match = try XCTUnwrap(statusString.firstMatch(of: /\((.*)\)/), "Every review item in status strings should have parenthesis with contents. \(statusString) DOES NOT MATCH PATTERN" )
132-
133-
let statusDetail = String(match.1)
134-
135-
XCTAssertNil(StatusExtractor.datesForString(statusDetail, processingDate: Date.now), "Unexpectedly able to parse '\(statusDetail)'")
136-
}
120+
// NOTE: This is something that should be validated!
121+
// It seems a common mistake to leave out closing parenthesis or put strong marker inside closing paren
122+
let match = try #require(statusString.firstMatch(of: /\((.*)\)/), "Every review item in status strings should have parenthesis with contents. \(statusString) DOES NOT MATCH PATTERN" )
123+
124+
let statusDetail = String(match.1)
125+
126+
#expect(StatusExtractor.datesForString(statusDetail, processingDate: Date.now) == nil, "Unexpectedly able to parse '\(statusDetail)'")
137127
}
138128

139129
/* Tests for breaking schema changes by serializing using the current model and then attempting to decode using a baseline version of the model from the last major schema release. The baseline model is located in the BaselineModel directory.
140130

141131
Note that this test assumes that the referenced snapshots are updated to be correct for the current model.
142132
*/
143-
func testForBreakingChanges() async throws {
133+
@Test(arguments: [
134+
"AllProposals",
135+
"Malformed",
136+
])
137+
func `Breaking Changes`(snapshotName: String) async throws {
144138

145-
let allProposalsURL = try urlForSnapshot(named: "AllProposals")
146-
let malformedProposalsURL = try urlForSnapshot(named: "Malformed")
147-
148139
let encoder = JSONEncoder()
149140
let decoder = JSONDecoder()
150141

151-
for snapshotURL in [allProposalsURL, malformedProposalsURL] {
152-
let extractionJob = try await ExtractionJob.makeExtractionJob(from: .snapshot(snapshotURL), output: .none, ignorePreviousResults: true)
153-
let extractedMetadata = try await EvolutionMetadataExtractor.extractEvolutionMetadata(for: extractionJob)
154-
let data = try encoder.encode(extractedMetadata)
155-
_ = try decoder.decode(EvolutionMetadata_v1.self, from: data)
156-
}
142+
let snapshotURL = try urlForSnapshot(named: snapshotName)
143+
let extractionJob = try await ExtractionJob.makeExtractionJob(from: .snapshot(snapshotURL), output: .none, ignorePreviousResults: true)
144+
let extractedMetadata = try await EvolutionMetadataExtractor.extractEvolutionMetadata(for: extractionJob)
145+
let data = try encoder.encode(extractedMetadata)
146+
_ = try decoder.decode(EvolutionMetadata_v1.self, from: data)
157147
}
158148

159149
/* Test that if an unknown proposal status is encountered, decoding does not fail and decodes to an .error status with the unknown status value as part of the associated reason string.
160150
The 'unknown-status.json' file contains the metadata of a single proposal with the fictional unknown status of 'appealed'.
161151
*/
162-
func testUnknownStatus() throws {
152+
@Test func `Unknown Status`() throws {
163153
let unknownStatusData = try data(forResource: "unknown-status", withExtension: "json")
164154
let proposal = try JSONDecoder().decode(Proposal.self, from: unknownStatusData)
165-
XCTAssertEqual(proposal.status, .unknownStatus("appealed"))
155+
#expect(proposal.status == .unknownStatus("appealed"))
166156
}
167157
}
168158

159+
// MARK: - Helpers
160+
161+
private func urlForSnapshot(named snapshotName: String) throws -> URL {
162+
try #require(Bundle.module.url(forResource: snapshotName, withExtension: "evosnapshot", subdirectory: "Resources"), "Unable to find snapshot \(snapshotName).evosnapshot in test bundle resources.")
163+
}
164+
165+
private func data(forResource name: String, withExtension ext: String) throws -> Data {
166+
let url = try #require(Bundle.module.url(forResource: name, withExtension: ext, subdirectory: "Resources"), "Unable to find resource \(name).\(ext) in test bundle resources.")
167+
let data = try Data(contentsOf: url)
168+
return data
169+
}
170+
171+
private func string(forResource name: String, withExtension ext: String) throws -> String {
172+
let data = try data(forResource: name, withExtension: ext)
173+
let string = try #require(String(data: data, encoding: .utf8), "Unable to make string from contents of \(name).\(ext)")
174+
return string
175+
}
176+
177+
// Convenience to write expected and actual metadata files to disk for comparison in a diff tool
178+
private func writeJSONFilesToPath(expected: Data, actual: Data, path: String, prefix: String? = nil) throws {
179+
let filePrefix: String
180+
if let prefix { filePrefix = "\(prefix)-" } else { filePrefix = "" }
181+
try expected.write(to: FileUtilities.expandedAndStandardizedURL(for: path).appending(path: "\(filePrefix)expected.json"))
182+
try actual.write(to: FileUtilities.expandedAndStandardizedURL(for: path).appending(path: "\(filePrefix)actual.json"))
183+
}
184+
185+
extension Proposal: CustomTestStringConvertible {
186+
public var testDescription: String { id.isEmpty ? "No ID" : id }
187+
}

0 commit comments

Comments
 (0)