Migrate URL and String utility tests to Swift Testing#1547
Migrate URL and String utility tests to Swift Testing#1547franklinsch wants to merge 10 commits into
Conversation
franklinsch
force-pushed
the
migrate-url-and-string-utility-tests-to-swift-testing
branch
from
6d3b9aa to
b4116c9
Compare
There was a problem hiding this comment.
minor: Many tests in this file could benefit from both parameterization and a more descriptive test name.
There was a problem hiding this comment.
For the expectations that have a comment associated to them, I left them as is so that we preserve it.
There was a problem hiding this comment.
minor: The tests in this file could benefit from more descriptive test names.
There was a problem hiding this comment.
minor: The tests in this file could benefit from slightly more descriptive test names.
| @Test | ||
| func splitsAStringWithNoTrailingNewlines() { | ||
| #expect("hello\nworld".splitByNewlines == ["hello", "world"]) | ||
| } | ||
|
|
||
| func testSplitsAStringWithOneTrailingNewline() { | ||
| XCTAssertEqual("hello\nworld\n".splitByNewlines, ["hello", "world"]) | ||
| @Test | ||
| func splitsAStringWithOneTrailingNewline() { | ||
| #expect("hello\nworld\n".splitByNewlines == ["hello", "world"]) |
There was a problem hiding this comment.
minor: These two tests could be combined into a single parameterized test.
| @Test | ||
| func variousSeparators() { |
There was a problem hiding this comment.
minor: This could benefit from parameterization.
There was a problem hiding this comment.
These are great suggestions, thanks. I'll address these as part of this PR.
franklinsch
force-pushed
the
migrate-url-and-string-utility-tests-to-swift-testing
branch
from
b4116c9 to
8ca587b
Compare
franklinsch
force-pushed
the
migrate-url-and-string-utility-tests-to-swift-testing
branch
from
8ca587b to
5d5bfe7
Compare
|
@swift-ci please test |
| ("hello, world", "Hello, world"), | ||
| ("twenty-one", "Twenty-One"), | ||
| ("hello! world", "Hello! world"), | ||
| ("hello: world", "Hello: world"), | ||
| ("l'ocean world", "L'ocean world"), |
There was a problem hiding this comment.
FYI: You can write this as a dictionary literal as well
| ("hello, world", "Hello, world"), | |
| ("twenty-one", "Twenty-One"), | |
| ("hello! world", "Hello! world"), | |
| ("hello: world", "Hello: world"), | |
| ("l'ocean world", "L'ocean world"), | |
| "hello, world": "Hello, world", | |
| "twenty-one": "Twenty-One", | |
| "hello! world": "Hello! world", | |
| "hello: world": "Hello: world", | |
| "l'ocean world": "L'ocean world", |
There was a problem hiding this comment.
That's good to know. Do you feel like that is more idiomatic?
| func testUSRHash() { | ||
| // Test that the results are stable for the given inputs | ||
| @Test(arguments: [ | ||
| ("", "ztntfp"), |
There was a problem hiding this comment.
Question: do you know why ExternalIdentifier.usr("").hash is expected to produce a different value compared to "".stableHashString?
I can see in the implementation that ExternalIdentifier.usr("").hash doesn't "fold" the hash into 24 bits before converting it to a base 36 string, but without a code comment in the implementation that feels like a possible bug to me.
2 participants
Bug/issue #, if applicable: N/A
Summary
Migrates utility tests for URL and String utilities to Swift Testing.
Dependencies
N/A
Testing
No functional change
Checklist
./bin/testscript and it succeeded