From c23a82065bbb7d23df2412d6320cdf9f9a67ab1b Mon Sep 17 00:00:00 2001 From: Stuart Montgomery Date: Sun, 5 Oct 2025 19:15:44 -0500 Subject: [PATCH 01/10] 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 --- Tests/ExtractionTests/ExtractionTests.swift | 60 +++---- .../ExtractionTests/ExtractionTests_old.swift | 168 ++++++++++++++++++ 2 files changed, 198 insertions(+), 30 deletions(-) create mode 100644 Tests/ExtractionTests/ExtractionTests_old.swift diff --git a/Tests/ExtractionTests/ExtractionTests.swift b/Tests/ExtractionTests/ExtractionTests.swift index 0eaa54d..0f91db1 100644 --- a/Tests/ExtractionTests/ExtractionTests.swift +++ b/Tests/ExtractionTests/ExtractionTests.swift @@ -1,33 +1,33 @@ // 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 { +struct ExtractionTests { 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.") + try #require(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)") + 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 } 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)") + let string = try #require(String(data: data, encoding: .utf8), "Unable to make string from contents of \(name).\(ext)") return string } @@ -39,21 +39,21 @@ final class ExtractionTests: XCTestCase { try actual.write(to: FileUtilities.expandedAndStandardizedURL(for: path).appending(path: "\(filePrefix)actual.json")) } - func testAllProposals() async throws { + @Test 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 expectedResults = try #require(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) + #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) // Check proposals for newProposal in extractedEvolutionMetadata.proposals { @@ -63,29 +63,29 @@ final class ExtractionTests: XCTestCase { } guard let sourceProposal = expectedResultsByProposalID[id] else { - XCTFail("Unable to find id \(id) in expected results.") + Issue.record("Unable to find id \(id) in expected results.") continue } - XCTAssertEqual(newProposal, sourceProposal) + #expect(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) + #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 { + @Test func testWarningsAndErrors() 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)'") + let expectedResults = try #require(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? @@ -95,15 +95,15 @@ final class ExtractionTests: XCTestCase { // 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) + #expect(extractionMetadata.proposals.count == expectedResults.proposals.count) for (actualResult, expectedResult) in zip(extractionMetadata.proposals, expectedResults.proposals) { - XCTAssertEqual(actualResult, expectedResult) + #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 testGoodDates() throws { let reviewDatesContents = try string(forResource: "review-dates-good", withExtension: "txt") @@ -111,16 +111,16 @@ final class ExtractionTests: XCTestCase { 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 func testBadDates() throws { let reviewDatesContents = try string(forResource: "review-dates-bad", withExtension: "txt") @@ -128,11 +128,11 @@ final class ExtractionTests: XCTestCase { 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) - XCTAssertNil(StatusExtractor.datesForString(statusDetail, processingDate: Date.now), "Unexpectedly able to parse '\(statusDetail)'") + #expect(StatusExtractor.datesForString(statusDetail, processingDate: Date.now) == nil, "Unexpectedly able to parse '\(statusDetail)'") } } @@ -140,7 +140,7 @@ final class ExtractionTests: XCTestCase { Note that this test assumes that the referenced snapshots are updated to be correct for the current model. */ - func testForBreakingChanges() async throws { + @Test func testForBreakingChanges() async throws { let allProposalsURL = try urlForSnapshot(named: "AllProposals") let malformedProposalsURL = try urlForSnapshot(named: "Malformed") @@ -159,10 +159,10 @@ final class ExtractionTests: XCTestCase { /* 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 testUnknownStatus() 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")) } } diff --git a/Tests/ExtractionTests/ExtractionTests_old.swift b/Tests/ExtractionTests/ExtractionTests_old.swift new file mode 100644 index 0000000..9982145 --- /dev/null +++ b/Tests/ExtractionTests/ExtractionTests_old.swift @@ -0,0 +1,168 @@ + +// This source file is part of the Swift.org open source project +// +// Copyright (c) 2024 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 Foundation + +@testable import EvolutionMetadataModel +@testable import EvolutionMetadataExtraction + +final class ExtractionTests_old: 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")) + } + + 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 + } + + 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") + + } + + func testWarningsAndErrors() 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 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) + } + } + + // The lines of text in review-dates-good.txt are status headers from swift-evolution repository history + func testGoodDates() 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 statusDetail = String(match.1) + + XCTAssertNotNil(StatusExtractor.datesForString(statusDetail, processingDate: Date.now), "Unable to parse '\(statusDetail)'") + } + } + + // The lines of text in review-dates-bad.txt are status headers from swift-evolution repository history + func testBadDates() throws { + + let reviewDatesContents = try string(forResource: "review-dates-bad", 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 statusDetail = String(match.1) + + XCTAssertNil(StatusExtractor.datesForString(statusDetail, processingDate: Date.now), "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 { + + 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) + } + } + + /* 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 { + let unknownStatusData = try data(forResource: "unknown-status", withExtension: "json") + let proposal = try JSONDecoder().decode(Proposal.self, from: unknownStatusData) + XCTAssertEqual(proposal.status, .unknownStatus("appealed")) + } +} + From 9b4c10ba73a20f83c16c75f84a76c1349c284171 Mon Sep 17 00:00:00 2001 From: Stuart Montgomery Date: Sun, 5 Oct 2025 19:18:57 -0500 Subject: [PATCH 02/10] Adopt raw identifiers to make suite and test function names more readable and idiomatic --- Tests/ExtractionTests/ExtractionTests.swift | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/Tests/ExtractionTests/ExtractionTests.swift b/Tests/ExtractionTests/ExtractionTests.swift index 0f91db1..c954fa3 100644 --- a/Tests/ExtractionTests/ExtractionTests.swift +++ b/Tests/ExtractionTests/ExtractionTests.swift @@ -13,7 +13,7 @@ import Foundation @testable import EvolutionMetadataModel @testable import EvolutionMetadataExtraction -struct ExtractionTests { +struct `Extraction tests` { 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.") @@ -39,7 +39,7 @@ struct ExtractionTests { try actual.write(to: FileUtilities.expandedAndStandardizedURL(for: path).appending(path: "\(filePrefix)actual.json")) } - @Test func testAllProposals() async throws { + @Test func `All proposals`() async throws { let snapshotURL = try urlForSnapshot(named: "AllProposals") let extractionJob = try await ExtractionJob.makeExtractionJob(from: .snapshot(snapshotURL), output: .none, ignorePreviousResults: true) @@ -81,7 +81,7 @@ struct ExtractionTests { } - @Test func testWarningsAndErrors() async throws { + @Test func `Warnings and errors`() async throws { let snapshotURL = try urlForSnapshot(named: "Malformed") let extractionJob = try await ExtractionJob.makeExtractionJob(from: .snapshot(snapshotURL), output: .none, ignorePreviousResults: true) @@ -103,7 +103,7 @@ struct ExtractionTests { } // The lines of text in review-dates-good.txt are status headers from swift-evolution repository history - @Test func testGoodDates() throws { + @Test func `Good dates`() throws { let reviewDatesContents = try string(forResource: "review-dates-good", withExtension: "txt") @@ -120,7 +120,7 @@ struct ExtractionTests { } // The lines of text in review-dates-bad.txt are status headers from swift-evolution repository history - @Test func testBadDates() throws { + @Test func `Bad dates`() throws { let reviewDatesContents = try string(forResource: "review-dates-bad", withExtension: "txt") @@ -140,7 +140,7 @@ struct ExtractionTests { Note that this test assumes that the referenced snapshots are updated to be correct for the current model. */ - @Test func testForBreakingChanges() async throws { + @Test func `Breaking changes`() async throws { let allProposalsURL = try urlForSnapshot(named: "AllProposals") let malformedProposalsURL = try urlForSnapshot(named: "Malformed") @@ -159,7 +159,7 @@ struct ExtractionTests { /* 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'. */ - @Test 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) #expect(proposal.status == .unknownStatus("appealed")) From 486dcdf7ac72e997262050e4e62b932c4e3b5cad Mon Sep 17 00:00:00 2001 From: Stuart Montgomery Date: Sun, 5 Oct 2025 19:19:50 -0500 Subject: [PATCH 03/10] Make test helper functions private --- Tests/ExtractionTests/ExtractionTests.swift | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/Tests/ExtractionTests/ExtractionTests.swift b/Tests/ExtractionTests/ExtractionTests.swift index c954fa3..0c49b9e 100644 --- a/Tests/ExtractionTests/ExtractionTests.swift +++ b/Tests/ExtractionTests/ExtractionTests.swift @@ -15,24 +15,24 @@ import Foundation struct `Extraction tests` { - func urlForSnapshot(named snapshotName: String) throws -> URL { + 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.") } - func data(forResource name: String, withExtension ext: String) throws -> Data { + 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 } - func string(forResource name: String, withExtension ext: String) throws -> String { + 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 - func writeJSONFilesToPath(expected: Data, actual: Data, path: String, prefix: String? = nil) throws { + 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")) From 7ebf23133fa8c69c148e7516c40940cfebdfc927 Mon Sep 17 00:00:00 2001 From: Stuart Montgomery Date: Sun, 5 Oct 2025 19:36:38 -0500 Subject: [PATCH 04/10] Move helper functions out of struct in anticipation of a subsequent refactor where they will be needed at other scopes --- Tests/ExtractionTests/ExtractionTests.swift | 49 +++++++++++---------- 1 file changed, 25 insertions(+), 24 deletions(-) diff --git a/Tests/ExtractionTests/ExtractionTests.swift b/Tests/ExtractionTests/ExtractionTests.swift index 0c49b9e..5de9d8c 100644 --- a/Tests/ExtractionTests/ExtractionTests.swift +++ b/Tests/ExtractionTests/ExtractionTests.swift @@ -14,30 +14,6 @@ import Foundation @testable import EvolutionMetadataExtraction struct `Extraction tests` { - - 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")) - } @Test func `All proposals`() async throws { @@ -166,3 +142,28 @@ struct `Extraction tests` { } } +// 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")) +} From 819f941bf9fc31da1fbcc06e58785c3f7c2a2648 Mon Sep 17 00:00:00 2001 From: Stuart Montgomery Date: Sun, 5 Oct 2025 20:02:33 -0500 Subject: [PATCH 05/10] Parameterize `Breaking changes` test --- Tests/ExtractionTests/ExtractionTests.swift | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/Tests/ExtractionTests/ExtractionTests.swift b/Tests/ExtractionTests/ExtractionTests.swift index 5de9d8c..730fb12 100644 --- a/Tests/ExtractionTests/ExtractionTests.swift +++ b/Tests/ExtractionTests/ExtractionTests.swift @@ -116,20 +116,20 @@ struct `Extraction tests` { Note that this test assumes that the referenced snapshots are updated to be correct for the current model. */ - @Test func `Breaking changes`() 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. From 2ba4fa47da368746031ce47c54f35332ec6c74ea Mon Sep 17 00:00:00 2001 From: Stuart Montgomery Date: Sun, 5 Oct 2025 20:22:19 -0500 Subject: [PATCH 06/10] Decompose `All proposals` @Test function into three smaller and more focused test functions in a sub-suite --- Tests/ExtractionTests/ExtractionTests.swift | 84 ++++++++++++--------- 1 file changed, 48 insertions(+), 36 deletions(-) diff --git a/Tests/ExtractionTests/ExtractionTests.swift b/Tests/ExtractionTests/ExtractionTests.swift index 730fb12..9ac633b 100644 --- a/Tests/ExtractionTests/ExtractionTests.swift +++ b/Tests/ExtractionTests/ExtractionTests.swift @@ -15,46 +15,58 @@ import Foundation struct `Extraction tests` { - @Test func `All proposals`() async throws { - - let snapshotURL = try urlForSnapshot(named: "AllProposals") - let extractionJob = try await ExtractionJob.makeExtractionJob(from: .snapshot(snapshotURL), output: .none, ignorePreviousResults: true) - 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 } - - let extractedEvolutionMetadata = try await EvolutionMetadataExtractor.extractEvolutionMetadata(for: extractionJob) - - // 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) - + 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) + } + // Check proposals - for newProposal in extractedEvolutionMetadata.proposals { - let id = newProposal.id - guard !id.isEmpty else { - 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) } - - guard let sourceProposal = expectedResultsByProposalID[id] else { - Issue.record("Unable to find id \(id) in expected results.") - continue - } - - #expect(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 - #expect(expectedJSONData == actualJSONData) - - // Uncomment to write full JSON files out to compare in a diff tool - // try writeJSONFilesToPath(expected: expectedJSONData, actual: actualJSONData, path: "~/Desktop") + // Check the generated JSON to catch issues such as removing properties from the schema + @Test func `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") + } } @Test func `Warnings and errors`() async throws { From d30ccb3c50a3ce96bbd9220f9aed785bce4bc2a3 Mon Sep 17 00:00:00 2001 From: Stuart Montgomery Date: Sun, 5 Oct 2025 20:23:32 -0500 Subject: [PATCH 07/10] 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) --- Tests/ExtractionTests/ExtractionTests.swift | 49 ++++++++++++++------- 1 file changed, 32 insertions(+), 17 deletions(-) diff --git a/Tests/ExtractionTests/ExtractionTests.swift b/Tests/ExtractionTests/ExtractionTests.swift index 9ac633b..24019ca 100644 --- a/Tests/ExtractionTests/ExtractionTests.swift +++ b/Tests/ExtractionTests/ExtractionTests.swift @@ -68,24 +68,39 @@ struct `Extraction tests` { // try writeJSONFilesToPath(expected: expectedJSONData, actual: actualJSONData, path: "~/Desktop") } } - - @Test func `Warnings and errors`() async throws { - - let snapshotURL = try urlForSnapshot(named: "Malformed") - let extractionJob = try await ExtractionJob.makeExtractionJob(from: .snapshot(snapshotURL), output: .none, ignorePreviousResults: true) - let expectedResults = try #require(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 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 - #expect(extractionMetadata.proposals.count == expectedResults.proposals.count) - - for (actualResult, expectedResult) in zip(extractionMetadata.proposals, expectedResults.proposals) { + struct `Malformed proposals` { + private static let snapshotName = "Malformed" + + private static var extractionJob: ExtractionJob { + get async throws { + let snapshotURL = try urlForSnapshot(named: snapshotName) + return try await ExtractionJob.makeExtractionJob(from: .snapshot(snapshotURL), output: .none, ignorePreviousResults: true) + } + } + + @Test func `Extraction metadata`() async throws { + let extractionJob = try await Self.extractionJob + let expectedResults = try #require(extractionJob.expectedResults, "No expected results found for extraction job with source '\(extractionJob.source)'") + + // 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 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 + #expect(extractionMetadata.proposals.count == expectedResults.proposals.count) + } + + @Test(arguments: try await { + let extractionJob = try await Self.extractionJob + 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) + return zip(extractionMetadata.proposals, expectedResults.proposals) + }()) + func `Warnings and errors`(actualResult: Proposal, expectedResult: Proposal) async throws { #expect(actualResult == expectedResult) } } From c106988b87e1bbdd99a8bb7ff35d86b5a4a28411 Mon Sep 17 00:00:00 2001 From: Stuart Montgomery Date: Sun, 5 Oct 2025 20:24:27 -0500 Subject: [PATCH 08/10] 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 --- Tests/ExtractionTests/ExtractionTests.swift | 25 ++++++++++----------- 1 file changed, 12 insertions(+), 13 deletions(-) diff --git a/Tests/ExtractionTests/ExtractionTests.swift b/Tests/ExtractionTests/ExtractionTests.swift index 24019ca..88b4b56 100644 --- a/Tests/ExtractionTests/ExtractionTests.swift +++ b/Tests/ExtractionTests/ExtractionTests.swift @@ -122,21 +122,20 @@ struct `Extraction tests` { } } - // The lines of text in review-dates-bad.txt are status headers from swift-evolution repository history - @Test func `Bad dates`() 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 #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)'") - } + // 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. From 9dfe0f64f641e6e1471edc89c436fe64ce960883 Mon Sep 17 00:00:00 2001 From: Stuart Montgomery Date: Sun, 5 Oct 2025 20:25:43 -0500 Subject: [PATCH 09/10] Finally, remove the old XCTest file --- .../ExtractionTests/ExtractionTests_old.swift | 168 ------------------ 1 file changed, 168 deletions(-) delete mode 100644 Tests/ExtractionTests/ExtractionTests_old.swift diff --git a/Tests/ExtractionTests/ExtractionTests_old.swift b/Tests/ExtractionTests/ExtractionTests_old.swift deleted file mode 100644 index 9982145..0000000 --- a/Tests/ExtractionTests/ExtractionTests_old.swift +++ /dev/null @@ -1,168 +0,0 @@ - -// This source file is part of the Swift.org open source project -// -// Copyright (c) 2024 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 Foundation - -@testable import EvolutionMetadataModel -@testable import EvolutionMetadataExtraction - -final class ExtractionTests_old: 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")) - } - - 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 - } - - 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") - - } - - func testWarningsAndErrors() 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 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) - } - } - - // The lines of text in review-dates-good.txt are status headers from swift-evolution repository history - func testGoodDates() 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 statusDetail = String(match.1) - - XCTAssertNotNil(StatusExtractor.datesForString(statusDetail, processingDate: Date.now), "Unable to parse '\(statusDetail)'") - } - } - - // The lines of text in review-dates-bad.txt are status headers from swift-evolution repository history - func testBadDates() throws { - - let reviewDatesContents = try string(forResource: "review-dates-bad", 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 statusDetail = String(match.1) - - XCTAssertNil(StatusExtractor.datesForString(statusDetail, processingDate: Date.now), "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 { - - 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) - } - } - - /* 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 { - let unknownStatusData = try data(forResource: "unknown-status", withExtension: "json") - let proposal = try JSONDecoder().decode(Proposal.self, from: unknownStatusData) - XCTAssertEqual(proposal.status, .unknownStatus("appealed")) - } -} - From 6f30cc23101f03348af18f0010d5034133f5fcc7 Mon Sep 17 00:00:00 2001 From: Stuart Montgomery Date: Fri, 14 Nov 2025 11:12:04 -0600 Subject: [PATCH 10/10] Incorporate PR feedback --- Tests/ExtractionTests/ExtractionTests.swift | 74 +++++++++------------ 1 file changed, 33 insertions(+), 41 deletions(-) diff --git a/Tests/ExtractionTests/ExtractionTests.swift b/Tests/ExtractionTests/ExtractionTests.swift index 88b4b56..2fb9adb 100644 --- a/Tests/ExtractionTests/ExtractionTests.swift +++ b/Tests/ExtractionTests/ExtractionTests.swift @@ -13,9 +13,10 @@ import Foundation @testable import EvolutionMetadataModel @testable import EvolutionMetadataExtraction -struct `Extraction tests` { +@Suite +struct `Extraction Tests` { - struct `All proposals` { + struct `All Proposals` { private var snapshotURL: URL private var extractionJob: ExtractionJob private var extractedEvolutionMetadata: EvolutionMetadata @@ -26,7 +27,7 @@ struct `Extraction tests` { extractedEvolutionMetadata = try await EvolutionMetadataExtractor.extractEvolutionMetadata(for: extractionJob) } - @Test func `Extracted metadata`() async throws { + @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 @@ -38,7 +39,7 @@ struct `Extraction tests` { } // Check proposals - @Test func `Expected proposals`() async throws { + @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 } @@ -58,7 +59,7 @@ struct `Extraction tests` { } // Check the generated JSON to catch issues such as removing properties from the schema - @Test func `Serialization`() throws { + @Test func `Expected Serialization`() throws { let expectedResultsURL = snapshotURL.appending(path: "expected-results.json") let expectedJSONData = try Data(contentsOf: expectedResultsURL) let actualJSONData = try extractedEvolutionMetadata.jsonRepresentation @@ -69,44 +70,31 @@ struct `Extraction tests` { } } - struct `Malformed proposals` { - private static let snapshotName = "Malformed" - - private static var extractionJob: ExtractionJob { - get async throws { - let snapshotURL = try urlForSnapshot(named: snapshotName) - return try await ExtractionJob.makeExtractionJob(from: .snapshot(snapshotURL), output: .none, ignorePreviousResults: true) - } - } - - @Test func `Extraction metadata`() async throws { - let extractionJob = try await Self.extractionJob - let expectedResults = try #require(extractionJob.expectedResults, "No expected results found for extraction job with source '\(extractionJob.source)'") - - // 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?) + 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 #require(extractionJob.expectedResults, "No expected results found for extraction job with source '\(extractionJob.source)'") + let extractionMetadata = try await EvolutionMetadataExtractor.extractEvolutionMetadata(for: extractionJob) - 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 may have an error. + try #require(extractionMetadata.proposals.count == expectedResults.proposals.count) - // 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 - #expect(extractionMetadata.proposals.count == expectedResults.proposals.count) - } + return zip(extractionMetadata.proposals, expectedResults.proposals) + } + } - @Test(arguments: try await { - let extractionJob = try await Self.extractionJob - 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) - return zip(extractionMetadata.proposals, expectedResults.proposals) - }()) - func `Warnings and errors`(actualResult: Proposal, expectedResult: Proposal) async throws { - #expect(actualResult == expectedResult) - } + // 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 - @Test func `Good dates`() throws { + @Test func `Good Dates`() throws { let reviewDatesContents = try string(forResource: "review-dates-good", withExtension: "txt") @@ -127,7 +115,7 @@ struct `Extraction tests` { let reviewDatesContents = try string(forResource: "review-dates-bad", withExtension: "txt") return reviewDatesContents.split(separator: "\n").map(String.init) }()) - func `Bad dates`(statusString: String) throws { + func `Bad Dates`(statusString: String) throws { // 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 @@ -146,7 +134,7 @@ struct `Extraction tests` { "AllProposals", "Malformed", ]) - func `Breaking changes`(snapshotName: String) async throws { + func `Breaking Changes`(snapshotName: String) async throws { let encoder = JSONEncoder() let decoder = JSONDecoder() @@ -161,7 +149,7 @@ struct `Extraction tests` { /* 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'. */ - @Test func `Unknown status`() throws { + @Test func `Unknown Status`() throws { let unknownStatusData = try data(forResource: "unknown-status", withExtension: "json") let proposal = try JSONDecoder().decode(Proposal.self, from: unknownStatusData) #expect(proposal.status == .unknownStatus("appealed")) @@ -193,3 +181,7 @@ private func writeJSONFilesToPath(expected: Data, actual: Data, path: String, pr 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 } +}