-
Notifications
You must be signed in to change notification settings - Fork 15
Adopt Swift Testing in metadata extractor tests #51
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
Merged
dempseyatgithub
merged 10 commits into
swiftlang:main
from
stmontgomery:adopt-swift-testing
Nov 14, 2025
Merged
Changes from all commits
Commits
Show all changes
10 commits
Select commit
Hold shift + click to select a range
c23a820
First, perform a "direct" conversion, making only the most minimal se…
stmontgomery 9b4c10b
Adopt raw identifiers to make suite and test function names more read…
stmontgomery 486dcdf
Make test helper functions private
stmontgomery 7ebf231
Move helper functions out of struct in anticipation of a subsequent r…
stmontgomery 819f941
Parameterize `Breaking changes` test
stmontgomery 2ba4fa4
Decompose `All proposals` @Test function into three smaller and more …
stmontgomery d30ccb3
Decompose `Warnings and errors` @Test function into two smaller and m…
stmontgomery c106988
Parameterize the `Bad dates` @Test function. I also considered doing …
stmontgomery 9dfe0f6
Finally, remove the old XCTest file
stmontgomery 6f30cc2
Incorporate PR feedback
stmontgomery File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,168 +1,187 @@ | ||
|
|
||
| // This source file is part of the Swift.org open source project | ||
| // | ||
| // Copyright (c) 2024 Apple Inc. and the Swift project authors | ||
| // Copyright (c) 2025 Apple Inc. and the Swift project authors | ||
| // Licensed under Apache License v2.0 with Runtime Library Exception | ||
| // | ||
| // See https://swift.org/LICENSE.txt for license information | ||
| // See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors | ||
|
|
||
| import XCTest | ||
| import Testing | ||
| import Foundation | ||
|
|
||
| @testable import EvolutionMetadataModel | ||
| @testable import EvolutionMetadataExtraction | ||
|
|
||
| final class ExtractionTests: XCTestCase { | ||
|
|
||
| func urlForSnapshot(named snapshotName: String) throws -> URL { | ||
| try XCTUnwrap(Bundle.module.url(forResource: snapshotName, withExtension: "evosnapshot", subdirectory: "Resources"), "Unable to find snapshot \(snapshotName).evosnapshot in test bundle resources.") | ||
| } | ||
|
|
||
| func data(forResource name: String, withExtension ext: String) throws -> Data { | ||
| let url = try XCTUnwrap(Bundle.module.url(forResource: name, withExtension: ext, subdirectory: "Resources"), "Unable to find resource \(name).\(ext) in test bundle resources.") | ||
| let data = try XCTUnwrap(Data(contentsOf: url), "Unable to read data from \(name).\(ext)") | ||
| return data | ||
| } | ||
|
|
||
| func string(forResource name: String, withExtension ext: String) throws -> String { | ||
| let data = try data(forResource: name, withExtension: ext) | ||
| let string = try XCTUnwrap(String(data: data, encoding: .utf8), "Unable to make string from contents of \(name).\(ext)") | ||
| return string | ||
| } | ||
|
|
||
| // Convenience to write expected and actual metadata files to disk for comparison in a diff tool | ||
| func writeJSONFilesToPath(expected: Data, actual: Data, path: String, prefix: String? = nil) throws { | ||
| let filePrefix: String | ||
| if let prefix { filePrefix = "\(prefix)-" } else { filePrefix = "" } | ||
| try expected.write(to: FileUtilities.expandedAndStandardizedURL(for: path).appending(path: "\(filePrefix)expected.json")) | ||
| try actual.write(to: FileUtilities.expandedAndStandardizedURL(for: path).appending(path: "\(filePrefix)actual.json")) | ||
| } | ||
| @Suite | ||
| struct `Extraction Tests` { | ||
|
|
||
| struct `All Proposals` { | ||
| private var snapshotURL: URL | ||
| private var extractionJob: ExtractionJob | ||
| private var extractedEvolutionMetadata: EvolutionMetadata | ||
|
|
||
| init() async throws { | ||
| snapshotURL = try urlForSnapshot(named: "AllProposals") | ||
| extractionJob = try await ExtractionJob.makeExtractionJob(from: .snapshot(snapshotURL), output: .none, ignorePreviousResults: true) | ||
| extractedEvolutionMetadata = try await EvolutionMetadataExtractor.extractEvolutionMetadata(for: extractionJob) | ||
| } | ||
|
|
||
| @Test func `Extracted Metadata`() async throws { | ||
| let expectedResults = try #require(extractionJob.expectedResults, "Snapshot from source '\(snapshotURL.absoluteString)' does not contain expected results.") | ||
|
|
||
| // Check top-level properties | ||
| #expect(extractedEvolutionMetadata.commit == expectedResults.commit) | ||
| #expect(extractedEvolutionMetadata.creationDate == expectedResults.creationDate) | ||
| #expect(extractedEvolutionMetadata.implementationVersions == expectedResults.implementationVersions) | ||
| #expect(extractedEvolutionMetadata.schemaVersion == expectedResults.schemaVersion) | ||
| #expect(extractedEvolutionMetadata.toolVersion == expectedResults.toolVersion) | ||
| } | ||
|
|
||
| func testAllProposals() async throws { | ||
|
|
||
| let snapshotURL = try urlForSnapshot(named: "AllProposals") | ||
| let extractionJob = try await ExtractionJob.makeExtractionJob(from: .snapshot(snapshotURL), output: .none, ignorePreviousResults: true) | ||
| let expectedResults = try XCTUnwrap(extractionJob.expectedResults, "Snapshot at '\(snapshotURL.absoluteString)' does not contain expected results.") | ||
| let expectedResultsByProposalID = expectedResults.proposals.reduce(into: [:]) { $0[$1.id] = $1 } | ||
|
|
||
| let extractedEvolutionMetadata = try await EvolutionMetadataExtractor.extractEvolutionMetadata(for: extractionJob) | ||
|
|
||
| // Check top-level properties | ||
| XCTAssertEqual(extractedEvolutionMetadata.commit, expectedResults.commit) | ||
| XCTAssertEqual(extractedEvolutionMetadata.creationDate, expectedResults.creationDate) | ||
| XCTAssertEqual(extractedEvolutionMetadata.implementationVersions, expectedResults.implementationVersions) | ||
| XCTAssertEqual(extractedEvolutionMetadata.schemaVersion, expectedResults.schemaVersion) | ||
| XCTAssertEqual(extractedEvolutionMetadata.toolVersion, expectedResults.toolVersion) | ||
|
|
||
| // Check proposals | ||
| for newProposal in extractedEvolutionMetadata.proposals { | ||
| let id = newProposal.id | ||
| guard !id.isEmpty else { | ||
| continue | ||
| } | ||
|
|
||
| guard let sourceProposal = expectedResultsByProposalID[id] else { | ||
| XCTFail("Unable to find id \(id) in expected results.") | ||
| continue | ||
| @Test func `Expected Proposals`() async throws { | ||
| let expectedResults = try #require(extractionJob.expectedResults, "Snapshot at '\(snapshotURL.absoluteString)' does not contain expected results.") | ||
| let expectedResultsByProposalID = expectedResults.proposals.reduce(into: [:]) { $0[$1.id] = $1 } | ||
|
|
||
| for newProposal in extractedEvolutionMetadata.proposals { | ||
| let id = newProposal.id | ||
| guard !id.isEmpty else { | ||
| continue | ||
| } | ||
|
|
||
| guard let sourceProposal = expectedResultsByProposalID[id] else { | ||
| Issue.record("Unable to find id \(id) in expected results.") | ||
| continue | ||
| } | ||
|
|
||
| #expect(newProposal == sourceProposal) | ||
| } | ||
|
|
||
| XCTAssertEqual(newProposal, sourceProposal) | ||
| } | ||
|
|
||
| // Check the generated JSON to catch issues such as removing properties from the schema | ||
| let expectedResultsURL = snapshotURL.appending(path: "expected-results.json") | ||
| let expectedJSONData = try Data(contentsOf: expectedResultsURL) | ||
| let actualJSONData = try extractedEvolutionMetadata.jsonRepresentation | ||
| XCTAssertEqual(expectedJSONData, actualJSONData) | ||
|
|
||
| // Uncomment to write full JSON files out to compare in a diff tool | ||
| // try writeJSONFilesToPath(expected: expectedJSONData, actual: actualJSONData, path: "~/Desktop") | ||
| @Test func `Expected Serialization`() throws { | ||
| let expectedResultsURL = snapshotURL.appending(path: "expected-results.json") | ||
| let expectedJSONData = try Data(contentsOf: expectedResultsURL) | ||
| let actualJSONData = try extractedEvolutionMetadata.jsonRepresentation | ||
| #expect(expectedJSONData == actualJSONData) | ||
|
|
||
| // Uncomment to write full JSON files out to compare in a diff tool | ||
| // try writeJSONFilesToPath(expected: expectedJSONData, actual: actualJSONData, path: "~/Desktop") | ||
| } | ||
| } | ||
| func testWarningsAndErrors() async throws { | ||
|
|
||
| private static var warningsAndErrorsArguments: Zip2Sequence<[Proposal], [Proposal]> { | ||
| get async throws { | ||
| let snapshotURL = try urlForSnapshot(named: "Malformed") | ||
| let extractionJob = try await ExtractionJob.makeExtractionJob(from: .snapshot(snapshotURL), output: .none, ignorePreviousResults: true) | ||
| let expectedResults = try XCTUnwrap(extractionJob.expectedResults, "No expected results found in bundle '\(snapshotURL.absoluteString)'") | ||
|
|
||
| // VALIDATION ENHANCEMENT: Tools like Xcode like to add a newline character to empty files | ||
| // Possibly instead of using 0007-empty-file.md to test, test separately? | ||
| // Or test that the file hasn't been corrupted. (Can you even check it into github?) | ||
|
|
||
| let expectedResults = try #require(extractionJob.expectedResults, "No expected results found for extraction job with source '\(extractionJob.source)'") | ||
| let extractionMetadata = try await EvolutionMetadataExtractor.extractEvolutionMetadata(for: extractionJob) | ||
|
|
||
| // This test zips the extraction results with the expected results | ||
| // If the two arrays don't have the same count, the test data itself has an error | ||
| XCTAssertEqual(extractionMetadata.proposals.count, expectedResults.proposals.count) | ||
|
|
||
| for (actualResult, expectedResult) in zip(extractionMetadata.proposals, expectedResults.proposals) { | ||
| XCTAssertEqual(actualResult, expectedResult) | ||
| } | ||
|
|
||
| // This test zips the extraction results with the expected results. If | ||
| // the two arrays don't have the same count, the test data itself may have an error. | ||
| try #require(extractionMetadata.proposals.count == expectedResults.proposals.count) | ||
|
|
||
| return zip(extractionMetadata.proposals, expectedResults.proposals) | ||
| } | ||
| } | ||
|
|
||
|
|
||
| // VALIDATION ENHANCEMENT: Tools like Xcode like to add a newline character to empty files | ||
| // Possibly instead of using 0007-empty-file.md to test, test separately? | ||
| // Or test that the file hasn't been corrupted. (Can you even check it into github?) | ||
| @Test(arguments: try await warningsAndErrorsArguments) | ||
| func `Warnings and Errors`(actualResult: Proposal, expectedResult: Proposal) async throws { | ||
| #expect(actualResult == expectedResult) | ||
| } | ||
|
|
||
| // The lines of text in review-dates-good.txt are status headers from swift-evolution repository history | ||
| func testGoodDates() throws { | ||
| @Test func `Good Dates`() throws { | ||
|
|
||
| let reviewDatesContents = try string(forResource: "review-dates-good", withExtension: "txt") | ||
|
|
||
| let statusStrings = reviewDatesContents.split(separator: "\n") | ||
| for statusString in statusStrings { | ||
| // NOTE: This is something that should be validated! | ||
| // It seems a common mistake to leave out closing parenthesis or put strong marker inside closing paren | ||
| let match = try XCTUnwrap(statusString.firstMatch(of: /\((.*)\)/), "Every review item in status strings should have parenthesis with contents. \(statusString) DOES NOT MATCH PATTERN" ) | ||
| let match = try #require(statusString.firstMatch(of: /\((.*)\)/), "Every review item in status strings should have parenthesis with contents. \(statusString) DOES NOT MATCH PATTERN" ) | ||
|
|
||
| let statusDetail = String(match.1) | ||
|
|
||
| XCTAssertNotNil(StatusExtractor.datesForString(statusDetail, processingDate: Date.now), "Unable to parse '\(statusDetail)'") | ||
| #expect(StatusExtractor.datesForString(statusDetail, processingDate: Date.now) != nil, "Unable to parse '\(statusDetail)'") | ||
| } | ||
| } | ||
|
|
||
| // The lines of text in review-dates-bad.txt are status headers from swift-evolution repository history | ||
| func testBadDates() throws { | ||
|
|
||
| @Test(arguments: try { | ||
| // The lines of text in review-dates-bad.txt are status headers from swift-evolution repository history | ||
| let reviewDatesContents = try string(forResource: "review-dates-bad", withExtension: "txt") | ||
| return reviewDatesContents.split(separator: "\n").map(String.init) | ||
| }()) | ||
| func `Bad Dates`(statusString: String) throws { | ||
|
|
||
| let statusStrings = reviewDatesContents.split(separator: "\n") | ||
| for statusString in statusStrings { | ||
| // NOTE: This is something that should be validated! | ||
| // It seems a common mistake to leave out closing parenthesis or put strong marker inside closing paren | ||
| let match = try XCTUnwrap(statusString.firstMatch(of: /\((.*)\)/), "Every review item in status strings should have parenthesis with contents. \(statusString) DOES NOT MATCH PATTERN" ) | ||
|
|
||
| let statusDetail = String(match.1) | ||
|
|
||
| XCTAssertNil(StatusExtractor.datesForString(statusDetail, processingDate: Date.now), "Unexpectedly able to parse '\(statusDetail)'") | ||
| } | ||
| // NOTE: This is something that should be validated! | ||
| // It seems a common mistake to leave out closing parenthesis or put strong marker inside closing paren | ||
| let match = try #require(statusString.firstMatch(of: /\((.*)\)/), "Every review item in status strings should have parenthesis with contents. \(statusString) DOES NOT MATCH PATTERN" ) | ||
|
|
||
| let statusDetail = String(match.1) | ||
|
|
||
| #expect(StatusExtractor.datesForString(statusDetail, processingDate: Date.now) == nil, "Unexpectedly able to parse '\(statusDetail)'") | ||
| } | ||
|
|
||
| /* 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. | ||
|
|
||
| Note that this test assumes that the referenced snapshots are updated to be correct for the current model. | ||
| */ | ||
| func testForBreakingChanges() async throws { | ||
| @Test(arguments: [ | ||
| "AllProposals", | ||
| "Malformed", | ||
| ]) | ||
| func `Breaking Changes`(snapshotName: String) async throws { | ||
|
|
||
| let allProposalsURL = try urlForSnapshot(named: "AllProposals") | ||
| let malformedProposalsURL = try urlForSnapshot(named: "Malformed") | ||
|
|
||
| let encoder = JSONEncoder() | ||
| let decoder = JSONDecoder() | ||
|
|
||
| for snapshotURL in [allProposalsURL, malformedProposalsURL] { | ||
| let extractionJob = try await ExtractionJob.makeExtractionJob(from: .snapshot(snapshotURL), output: .none, ignorePreviousResults: true) | ||
| let extractedMetadata = try await EvolutionMetadataExtractor.extractEvolutionMetadata(for: extractionJob) | ||
| let data = try encoder.encode(extractedMetadata) | ||
| _ = try decoder.decode(EvolutionMetadata_v1.self, from: data) | ||
| } | ||
| let snapshotURL = try urlForSnapshot(named: snapshotName) | ||
| let extractionJob = try await ExtractionJob.makeExtractionJob(from: .snapshot(snapshotURL), output: .none, ignorePreviousResults: true) | ||
| let extractedMetadata = try await EvolutionMetadataExtractor.extractEvolutionMetadata(for: extractionJob) | ||
| let data = try encoder.encode(extractedMetadata) | ||
| _ = try decoder.decode(EvolutionMetadata_v1.self, from: data) | ||
| } | ||
|
|
||
| /* 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. | ||
| The 'unknown-status.json' file contains the metadata of a single proposal with the fictional unknown status of 'appealed'. | ||
| */ | ||
| func testUnknownStatus() throws { | ||
| @Test func `Unknown Status`() throws { | ||
| let unknownStatusData = try data(forResource: "unknown-status", withExtension: "json") | ||
| let proposal = try JSONDecoder().decode(Proposal.self, from: unknownStatusData) | ||
| XCTAssertEqual(proposal.status, .unknownStatus("appealed")) | ||
| #expect(proposal.status == .unknownStatus("appealed")) | ||
| } | ||
| } | ||
|
|
||
| // MARK: - Helpers | ||
|
|
||
| private func urlForSnapshot(named snapshotName: String) throws -> URL { | ||
| try #require(Bundle.module.url(forResource: snapshotName, withExtension: "evosnapshot", subdirectory: "Resources"), "Unable to find snapshot \(snapshotName).evosnapshot in test bundle resources.") | ||
| } | ||
|
|
||
| private func data(forResource name: String, withExtension ext: String) throws -> Data { | ||
| let url = try #require(Bundle.module.url(forResource: name, withExtension: ext, subdirectory: "Resources"), "Unable to find resource \(name).\(ext) in test bundle resources.") | ||
| let data = try Data(contentsOf: url) | ||
| return data | ||
| } | ||
|
|
||
| private func string(forResource name: String, withExtension ext: String) throws -> String { | ||
| let data = try data(forResource: name, withExtension: ext) | ||
| let string = try #require(String(data: data, encoding: .utf8), "Unable to make string from contents of \(name).\(ext)") | ||
| return string | ||
| } | ||
|
|
||
| // Convenience to write expected and actual metadata files to disk for comparison in a diff tool | ||
| private func writeJSONFilesToPath(expected: Data, actual: Data, path: String, prefix: String? = nil) throws { | ||
| let filePrefix: String | ||
| if let prefix { filePrefix = "\(prefix)-" } else { filePrefix = "" } | ||
| try expected.write(to: FileUtilities.expandedAndStandardizedURL(for: path).appending(path: "\(filePrefix)expected.json")) | ||
| try actual.write(to: FileUtilities.expandedAndStandardizedURL(for: path).appending(path: "\(filePrefix)actual.json")) | ||
| } | ||
|
|
||
| extension Proposal: CustomTestStringConvertible { | ||
| public var testDescription: String { id.isEmpty ? "No ID" : id } | ||
| } | ||
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Proposaltype could use a custom test description.I don't know what the best practice is for adding test descriptions. (As part of the original definition of the type, as part of the test bundle, etc.)
In the case, it is a small extension and likely will not be joined by other test descriptions, so defining at the top doesn't add much clutter, but makes it obvious where that description is coming from.
Would appreciate any feedback you have in terms of canonical Swift Testing as to where this should live, but
Proposaldefinitely needs a custom test description.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.
Happy to include such an extension and conformance. In terms of where it should go, that's very much a style question and we, the maintainers of Swift Testing, don't really offer canonical guidance about that so I would apply whatever your preferred Swift code style is generally.
My own personal tendency would be to place it within the
// MARK: - Helperssection, which in this file is near the bottom. I generally like the "main" declarations in a file to appear closer to the top. But it's purely subjective/stylistic.