-
Notifications
You must be signed in to change notification settings - Fork 10.6k
[stdlib] Expanded string slicing test #2367
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
Conversation
test/1_stdlib/subString.swift
Outdated
| expectEqual(x[i], y[i]) | ||
| } | ||
|
|
||
| let s = "abcdefg" |
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.
Could you move this variable into each test that uses it? This would make tests standalone.
|
Thanks @natecook1000! |
| expectEqual(String(t), "Cdefg") | ||
| expectEqual(s, "abcdefg") | ||
|
|
||
| checkMatch(s.characters, t, u.startIndex) |
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.
I don't think this is a valid assertion. t has been mutated before the position referenced by u.startIndex
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.
Good point—I'll change it so it only tests string equality after a mutation. The mutations are there to ensure value semantics post-fix; those all pass today.
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.
Checking that indices are interchangeable after mutation is a valid thing to do, we just need to mutate the tail part of the string that occurs after the index position.
| expectEqual(String(t), "Cdefg") | ||
| expectEqual(s, "abcdefg") | ||
|
|
||
| checkMatch(s.unicodeScalars, t, u.startIndex) |
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.
Same as above?
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.
Both groups should be fixed now.
|
@swift-ci Please test and merge |
1 similar comment
|
@swift-ci Please test and merge |
|
@swift-ci test |
|
CI issues are unrelated. |
What's in this pull request?
This updates the tests for substrings to include sharing indices with string view slices. The tests are currently
xfail'd for CharacterView, UnicodeScalarView, and UTF16View.Current test output:
Before merging this pull request to apple/swift repository:
Triggering Swift CI
The swift-ci is triggered by writing a comment on this PR addressed to the GitHub user @swift-ci. Different tests will run depending on the specific comment that you use. The currently available comments are:
Smoke Testing
Validation Testing
Note: Only members of the Apple organization can trigger swift-ci.