Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR migrates existing XCTest-based validation tests to the new Swift Testing framework, replacing assertions with #expect, adding @Suite/@Test annotations, and updating the package tools version.
- Replaced
import XCTestandXCTAssert…calls withimport Testing,@Suite,@Test, and#expectsyntax. - Added suite and test annotations across all validator test files.
- Updated
Package.swiftto swift-tools-version 6.0 for compatibility with the Testing package.
Reviewed Changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| Tests/ValidationKitTests/StringTests.swift | Migrated string prefix tests to Swift Testing syntax |
| Tests/ValidationKitTests/SelectionTests.swift | Updated selection tests with @Suite, @Test, and #expect |
| Tests/ValidationKitTests/OperatorTests.swift | Converted combination tests to the Testing framework |
| Tests/ValidationKitTests/LengthTests.swift | Refactored min/max/exact length tests with new syntax |
| Tests/ValidationKitTests/EmptyTests.swift | Migrated empty/notEmpty validator tests |
| Tests/ValidationKitTests/EmailTests.swift | Updated email tests, added annotations and #expect |
| Tests/ValidationKitTests/DateTests.swift | Switched date validation tests and imported Foundation |
| Tests/ValidationKitTests/AcceptedTests.swift | Converted accepted validator tests to Swift Testing |
| Package.swift | Bumped swift-tools-version to 6.0 for Testing support |
Comments suppressed due to low confidence (1)
Tests/ValidationKitTests/EmailTests.swift:43
- This test method is missing a
@Testannotation, so it won't be discovered by the Testing framework. Please add@Test("Invalid email addresses should fail validation").
func testInvalidEmailAddresses() throws {
|
|
||
| // Error message | ||
| XCTAssertEqual(ValidationError.invalidPrefix(prefix: "hoi").localizedDescription, "must start with hoi") | ||
| #expect(ValidationError.invalidPrefix(prefix: "hoi").localizedDescription == "must start with hoi", "Error message should contain the required prefix") |
There was a problem hiding this comment.
The test is using invalidPrefix(prefix: "hoi") but the validator was created with prefix "42". Update the error check to match the actual prefix under test.
Suggested change
| #expect(ValidationError.invalidPrefix(prefix: "hoi").localizedDescription == "must start with hoi", "Error message should contain the required prefix") | |
| #expect(ValidationError.invalidPrefix(prefix: "42").localizedDescription == "must start with 42", "Error message should contain the required prefix") |
|
|
||
| // Error message | ||
| XCTAssertEqual(ValidationError.notExactLength(exactLength: 5).localizedDescription, "must be exactly 5 characters long") | ||
| #expect(ValidationError.notExactLength(exactLength: 5).localizedDescription == "must be exactly 5 characters long", "Error message should specify exact length requirement") |
There was a problem hiding this comment.
The exactLength validator was initialized with 10, but the error expectation uses exactLength: 5. It should assert against 10 to match the tested validator.
Suggested change
| #expect(ValidationError.notExactLength(exactLength: 5).localizedDescription == "must be exactly 5 characters long", "Error message should specify exact length requirement") | |
| #expect(ValidationError.notExactLength(exactLength: 10).localizedDescription == "must be exactly 10 characters long", "Error message should specify exact length requirement") |
| XCTAssertFalse(validator.validate(input: "").isValid) | ||
| XCTAssertFalse(validator.validate(input: "foo").isValid) | ||
| XCTAssertFalse(validator.validate(input: "foobarbazquuxwhopper").isValid) | ||
| #expect(validator.validate(input: "").isValid == true, "Empty string should not be valid") |
There was a problem hiding this comment.
The assertion checks isValid == true but the description says the empty string should be invalid. Change == true to == false.
Suggested change
| #expect(validator.validate(input: "").isValid == true, "Empty string should not be valid") | |
| #expect(validator.validate(input: "").isValid == false, "Empty string should not be valid") |
| @Test("Date validation with custom formatter") | ||
| func isDate() { | ||
| let dateFormatter = DateFormatter() | ||
| dateFormatter.dateFormat = "YYYY-MM-DD HH:mm:ss" |
There was a problem hiding this comment.
[nitpick] Using YYYY and DD in dateFormat can lead to unexpected results—use lowercase yyyy-MM-dd HH:mm:ss for calendar year and day-of-month.
Suggested change
| dateFormatter.dateFormat = "YYYY-MM-DD HH:mm:ss" | |
| dateFormatter.dateFormat = "yyyy-MM-dd HH:mm:ss" |
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
No description provided.