additional edge cases tests for path.rs 🧪
#141105
Open
+87
−1
Conversation
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
rustbot
added
S-waiting-on-review
This comment has been minimized.
This comment has been minimized.
|
It appears that |
|
|
It looks as if |
|
Yes, please fix the tests to only use functions from OsStr and OsString OR cfg the tests. |
|
The job Click to see the possible cause of the failure (guessed by this bot) |
ChrisDenton
reviewed
May 17, 2025
| tp!("foo./.", "bar", r"foo./.\bar"); | ||
| tp!("foo./.", "bar", r"foo././bar"); |
There was a problem hiding this comment.
Why is this changing an existing test?
Comment on lines
+2002
to
+2004
| // Test: Reserved device names (Windows) | ||
| // This test ensures that reserved device names like "CON", "PRN", etc., are handled as normal paths on non-Windows platforms, | ||
| // and as special cases on Windows (if applicable). |
There was a problem hiding this comment.
This comment doesn't seem to describe the test.
Comment on lines
+2015
to
+2017
| // Test: Trailing dots/spaces (Windows) | ||
| // This test checks how Path handles trailing dots or spaces, which are special on Windows. | ||
| // On Unix, these should be treated as normal characters. |
There was a problem hiding this comment.
Path mostly works the same across platforms (separator differences and roots/prefixes not withstanding). If there's a difference then you should have tests for both Windows and non-Windows that shows the difference.
Comment on lines
+2033
to
+2035
| // Test: Only extension (e.g., ".gitignore") | ||
| // This test verifies that files with only an extension and no base name are handled correctly. | ||
| // It checks that the extension is recognized and the file stem is None or empty as appropriate. |
There was a problem hiding this comment.
Do we really not have a test for this anywhere else?
Comment on lines
+2044
to
+2046
| // Test: Long components | ||
| // This test checks that Path can handle very long path components without truncation or error. | ||
| // It ensures that the length of the component is preserved. |
There was a problem hiding this comment.
I'm not sure why this would be useful for us specifically? We don't usually check that slice wrappers work on the whole slice.
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.
This pull request adds a few new edge case tests to the
std::pathmodule. The new tests cover scenarios such as paths with only separators, non-ASCII and Unicode characters, embedded null bytes, etc. Each new test is documented with some helpful in-line comments as well.