Align path prefix checks with platform case sensitivity - #227
Conversation
| [Theory] | ||
| [InlineData("Foo", "foo")] | ||
| [InlineData("Foo", "foo/file.txt")] | ||
| [InlineData("Foo", "foobar")] | ||
| public void IsPrefixOfFollowsPlatformDefaultCaseSensitivity(string prefix, string other) | ||
| { | ||
| var root = new AbsolutePath(OperatingSystem.IsWindows() ? @"A:\" : "/"); | ||
| var a = root / prefix; | ||
| var b = root / other; | ||
| var expected = (OperatingSystem.IsWindows() || OperatingSystem.IsMacOS()) && other != "foobar"; | ||
|
|
||
| Assert.Equal(expected, a.IsPrefixOf(b)); | ||
| Assert.Equal(expected, b.StartsWith(a)); | ||
| if (other == "foo") Assert.Equal(a == b, a.IsPrefixOf(b)); | ||
| } |
There was a problem hiding this comment.
A bit too complex, I don't like checks for OS and other == "foobar" / other == "foo" in the test body. Perhaps it's be better to have three separate tests?
There was a problem hiding this comment.
Split it into 3 separate tests, with the OS check moved into a Utils.IsPlatformCaseInsensitive() helper.
| var startsWith = b.Value.StartsWith(a.Value, OperatingSystem.IsWindows() || OperatingSystem.IsMacOS() | ||
| ? StringComparison.OrdinalIgnoreCase | ||
| : StringComparison.Ordinal); |
There was a problem hiding this comment.
I think this should be better replaced with b.StartsWith(a).
There was a problem hiding this comment.
Thanks for the review. It's Done, now uses b.StartsWith(a).
| [Theory] | ||
| [InlineData("Foo", "foo")] | ||
| [InlineData("Foo", "foo/bar")] | ||
| [InlineData("Foo", "foobar")] | ||
| public void IsPrefixOfFollowsPlatformDefaultCaseSensitivity(string prefix, string other) | ||
| { | ||
| var a = new LocalPath(prefix); | ||
| var b = new LocalPath(other); | ||
| var expected = (OperatingSystem.IsWindows() || OperatingSystem.IsMacOS()) && other != "foobar"; | ||
|
|
||
| Assert.Equal(expected, a.IsPrefixOf(b)); | ||
| Assert.Equal(expected, b.StartsWith(a)); | ||
| if (other == "foo") Assert.Equal(a == b, a.IsPrefixOf(b)); | ||
| } |
There was a problem hiding this comment.
Same comment as on the AbsolutePathTests: let's somehow get rid of the checks for particular cases inside of the test body.
There was a problem hiding this comment.
split it into three tests with no per-case checks in the body
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
2 participants
Fixes #225.
Path prefix checks now use the same platform-default case sensitivity as path equality. This keeps IsPrefixOf and StartsWith consistent with Equals while preserving whole-path-segment matching.
Updated the API documentation and added cross-platform tests.
Validation: