Skip to content

Commit 9bb1d21

Browse files
Copilotjkotasadamsitnik
authored
Remove dead code PathFeatures.cs and assume all path features are available (#120929)
- [x] Delete PathFeatures.cs file - [x] Remove PathFeatures.cs references from .csproj files (5 files) - [x] Update FileSystemTest.cs to remove PathFeatures usage (3 properties) - [x] Update PathTestsBase.cs to use non-legacy values in TestData_GetPathRoot_DevicePaths (5 lines) - [x] Update PathTests_Windows.cs to remove conditional logic based on PathFeatures (9 usages) - [x] Update zip_ManualAndCompatibilityTests.cs to remove PathFeatures usage (1 property + 1 conditional) - [x] Update ProcessModuleTests.Windows.cs to remove ConditionalFact attribute (1 line) - [x] Build and test affected libraries to ensure all tests pass - System.Runtime.Extensions.Tests: 8280 tests passed - System.IO.FileSystem.Tests: 8716 tests passed - System.IO.Compression.Tests: 1335 tests passed - System.Diagnostics.Process.Tests: 339 tests passed - [x] Run final verification of all changes - [x] Address feedback: Remove AreAllLongPathsAvailable, LongPathsAreNotBlocked, and UsingNewNormalization properties - [x] Address feedback: Update outdated comment about ConditionalFact in ProcessModuleTests.Windows.cs - [x] Address feedback: Remove IsUsingNewPathNormalization property from zip_ManualAndCompatibilityTests.cs - [x] Address feedback: Remove redundant property wrappers in Delete.cs and replace with RemoteExecutor.IsSupported - [x] Address feedback: Merge GetFullPath_Windows_UNC_Valid_LegacyPathSupport test cases into GetFullPath_UNC_Valid - [x] Fix CI failure: Add typeof(RemoteExecutor) parameter to ConditionalFact attributes ## Summary Successfully removed PathFeatures.cs dead code and all its references. Replaced all conditional test attributes that depended on always-true properties with unconditional Fact/Theory attributes. Removed redundant property wrappers and merged duplicate test methods. Fixed ConditionalFact attributes to properly reference RemoteExecutor.IsSupported. All tests now assume: - All long paths are available (no legacy MAX_PATH limitations) - New path normalization is always used (no legacy path handling) - No conditional behavior based on runtime capabilities <!-- START COPILOT CODING AGENT SUFFIX --> <details> <summary>Original prompt</summary> > > ---- > > *This section details on the original issue you should resolve* > > <issue_title>PathFeatures.cs is dead code</issue_title> > <issue_description>PathFeatures.cs is only used in tests in target current netcoreapp app. All path features are available in current netcoreapp.</issue_description> > > <agent_instructions>Please delete PathFeatures.cs and all references to it. Assume that all Path features are available and no legacy behaviors are present.</agent_instructions> > > ## Comments on the Issue (you are @copilot in this section) > > <comments> > </comments> > </details> Fixes #120928 <!-- START COPILOT CODING AGENT TIPS --> --- 💬 Share your feedback on Copilot coding agent for the chance to win a $200 gift card! Click [here](https://survey3.medallia.com/?EAHeSx-AP01bZqG0Ld9QLQ) to start the survey. --------- Co-authored-by: copilot-swe-agent[bot] <[email protected]> Co-authored-by: jkotas <[email protected]> Co-authored-by: adamsitnik <[email protected]>
1 parent 8c90a74 commit 9bb1d21

File tree

21 files changed

+59
-245
lines changed

21 files changed

+59
-245
lines changed

src/libraries/Common/tests/System/IO/PathFeatures.cs

Lines changed: 0 additions & 98 deletions
This file was deleted.

src/libraries/System.Diagnostics.Process/tests/ProcessModuleTests.Windows.cs

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,12 +10,11 @@ namespace System.Diagnostics.Tests
1010
{
1111
public partial class ProcessModuleTests : ProcessTestBase
1212
{
13-
[ConditionalFact(typeof(PathFeatures), nameof(PathFeatures.AreAllLongPathsAvailable))]
13+
[Fact]
1414
public void LongModuleFileNamesAreSupported()
1515
{
1616
// To be able to test Long Path support for ProcessModule.FileName we need a .dll that has a path > 260 chars.
17-
// Since Long Paths support can be disabled (see the ConditionalFact attribute usage above),
18-
// we just copy "LongName.dll" from bin to a temp directory with a long name and load it from there.
17+
// We just copy "LongName.dll" from bin to a temp directory with a long name and load it from there.
1918
// Loading from new path is possible because the type exposed by the assembly is not referenced in any explicit way.
2019
const string libraryName = "LongPath.dll";
2120
const int minPathLength = 261;

src/libraries/System.Diagnostics.Process/tests/System.Diagnostics.Process.Tests.csproj

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,8 +20,6 @@
2020
Link="Common\Microsoft\Win32\TempRegistryKey.cs" />
2121
<Compile Include="$(CommonPath)System\Text\ValueStringBuilder.cs"
2222
Link="Common\System\Text\ValueStringBuilder.cs" />
23-
<Compile Include="$(CommonTestPath)System\IO\PathFeatures.cs"
24-
Link="Common\System\IO\PathFeatures.cs" />
2523
<Compile Include="DelegateSynchronizeInvoke.cs" />
2624
<Compile Include="FileAssociations.cs" />
2725
<Compile Include="Helpers.cs" />

src/libraries/System.IO.Compression/tests/System.IO.Compression.Tests.csproj

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,6 @@
2929
<Compile Include="ZipArchive\zip_ReadTests.cs" />
3030
<Compile Include="ZipArchive\zip_UpdateTests.cs" />
3131
<Compile Include="ZipArchive\zip_UpdateTests.Comments.cs" />
32-
<Compile Include="$(CommonTestPath)System\IO\PathFeatures.cs" Link="Common\System\IO\PathFeatures.cs" />
3332
<Compile Include="$(CommonTestPath)System\IO\CallTrackingStream.cs" Link="Common\System\IO\CallTrackingStream.cs" />
3433
<Compile Include="$(CommonTestPath)System\IO\Compression\CRC.cs" Link="Common\System\IO\Compression\CRC.cs" />
3534
<Compile Include="$(CommonTestPath)System\IO\Compression\CompressionStreamTestBase.cs" Link="Common\System\IO\Compression\CompressionStreamTestBase.cs" />

src/libraries/System.IO.Compression/tests/ZipArchive/zip_ManualAndCompatibilityTests.cs

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,6 @@ namespace System.IO.Compression.Tests
1010
{
1111
public class zip_ManualAndCompatibilityTests : ZipFileTestBase
1212
{
13-
public static bool IsUsingNewPathNormalization => !PathFeatures.IsUsingLegacyPathNormalization();
14-
1513
public static IEnumerable<object[]> Get_CompatibilityTests_Data()
1614
{
1715
foreach (bool async in _bools)
@@ -67,7 +65,7 @@ public static async Task CompatibilityTestsMsFiles(string withTrailing, string w
6765
/// For example, the file "aa\bb\cc\dd" in a zip created on Unix should be one file "aa\bb\cc\dd" whereas the same file
6866
/// in a zip created on Windows should be interpreted as the file "dd" underneath three subdirectories.
6967
/// </summary>
70-
[ConditionalTheory(nameof(IsUsingNewPathNormalization))]
68+
[Theory]
7169
[InlineData("backslashes_FromUnix.zip", "aa\\bb\\cc\\dd")]
7270
[InlineData("backslashes_FromWindows.zip", "dd")]
7371
[InlineData("WindowsInvalid_FromUnix.zip", "aa<b>d")]

src/libraries/System.Runtime/tests/System.IO.FileSystem.Tests/Directory/CreateDirectory.cs

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -152,7 +152,7 @@ public void ValidPathWithTrailingSlash(string component)
152152
Assert.True(result.Exists);
153153
}
154154

155-
[ConditionalTheory(nameof(UsingNewNormalization)),
155+
[Theory,
156156
MemberData(nameof(ValidPathComponentNames))]
157157
[PlatformSpecific(TestPlatforms.Windows)] // trailing slash
158158
public void ValidExtendedPathWithTrailingSlash(string component)
@@ -239,7 +239,7 @@ public void PathsWithInvalidColons_ThrowIOException_Core(string invalidPath)
239239
Assert.ThrowsAny<IOException>(() => Create(invalidPath));
240240
}
241241

242-
[ConditionalFact(nameof(AreAllLongPathsAvailable))]
242+
[Fact]
243243
[PlatformSpecific(TestPlatforms.Windows)] // long directory path succeeds
244244
public void DirectoryLongerThanMaxPath_Succeeds()
245245
{
@@ -262,7 +262,7 @@ public void DirectoryLongerThanMaxLongPath_ThrowsPathTooLongException()
262262
});
263263
}
264264

265-
[ConditionalFact(nameof(LongPathsAreNotBlocked), nameof(UsingNewNormalization))]
265+
[Fact]
266266
[PlatformSpecific(TestPlatforms.Windows)]
267267
public void DirectoryLongerThanMaxLongPathWithExtendedSyntax_ThrowsException()
268268
{
@@ -274,7 +274,7 @@ public void DirectoryLongerThanMaxLongPathWithExtendedSyntax_ThrowsException()
274274
AssertExtensions.ThrowsAny<PathTooLongException, DirectoryNotFoundException, IOException>(() => Create(path)));
275275
}
276276

277-
[ConditionalFact(nameof(LongPathsAreNotBlocked), nameof(UsingNewNormalization))]
277+
[Fact]
278278
[PlatformSpecific(TestPlatforms.Windows)] // long directory path with extended syntax succeeds
279279
public void ExtendedDirectoryLongerThanLegacyMaxPath_Succeeds()
280280
{
@@ -285,7 +285,7 @@ public void ExtendedDirectoryLongerThanLegacyMaxPath_Succeeds()
285285
});
286286
}
287287

288-
[ConditionalFact(nameof(AreAllLongPathsAvailable))]
288+
[Fact]
289289
[PlatformSpecific(TestPlatforms.Windows)] // long directory path succeeds
290290
public void DirectoryLongerThanMaxDirectoryAsPath_Succeeds()
291291
{
@@ -374,7 +374,7 @@ public void TrailingSpace_NotTrimmed(string component)
374374
Assert.Equal(testDir.FullName, IOServices.RemoveTrailingSlash(result.FullName));
375375
}
376376

377-
[ConditionalTheory(nameof(UsingNewNormalization)),
377+
[Theory,
378378
MemberData(nameof(SimpleWhiteSpace))]
379379
[PlatformSpecific(TestPlatforms.Windows)] // extended syntax with whitespace
380380
public void WindowsExtendedSyntaxWhiteSpace(string path)
@@ -423,7 +423,7 @@ public void PathWithReservedDeviceNameAsPath_ThrowsDirectoryNotFoundException(st
423423
Assert.Throws<DirectoryNotFoundException>(() => Create(path));
424424
}
425425

426-
[ConditionalTheory(nameof(ReservedDeviceNamesAreBlocked), nameof(UsingNewNormalization))] // device name prefixes
426+
[ConditionalTheory(nameof(ReservedDeviceNamesAreBlocked))] // device name prefixes
427427
[MemberData(nameof(ReservedDeviceNames))]
428428
public void PathWithReservedDeviceNameAsExtendedPath(string path)
429429
{

src/libraries/System.Runtime/tests/System.IO.FileSystem.Tests/Directory/Delete.cs

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -14,10 +14,6 @@ public class Directory_Delete_str : FileSystemTest
1414

1515
static bool IsBindMountSupportedAndPrivilegedProcess => IsBindMountSupported && PlatformDetection.IsPrivilegedProcess;
1616

17-
static bool IsRemoteExecutorSupportedAndUsingNewNormalization => RemoteExecutor.IsSupported && UsingNewNormalization;
18-
19-
static bool IsRemoteExecutorSupportedAndLongPathsAreNotBlockedAndUsingNewNormalization => RemoteExecutor.IsSupported && LongPathsAreNotBlocked && UsingNewNormalization;
20-
2117
#region Utilities
2218

2319
protected virtual void Delete(string path)
@@ -126,7 +122,7 @@ public void DeletingSymLinkDoesntDeleteTarget()
126122
Assert.False(Directory.Exists(linkPath), "linkPath should no longer exist");
127123
}
128124

129-
[ConditionalFact(nameof(IsRemoteExecutorSupportedAndUsingNewNormalization))]
125+
[ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))]
130126
public void ExtendedDirectoryWithSubdirectories()
131127
{
132128
RemoteExecutor.Invoke(() =>
@@ -139,7 +135,7 @@ public void ExtendedDirectoryWithSubdirectories()
139135
}).Dispose();
140136
}
141137

142-
[ConditionalFact(nameof(IsRemoteExecutorSupportedAndLongPathsAreNotBlockedAndUsingNewNormalization))]
138+
[ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))]
143139
public void LongPathExtendedDirectory()
144140
{
145141
RemoteExecutor.Invoke(() =>
@@ -166,7 +162,7 @@ public void WindowsDeleteReadOnlyDirectory()
166162
testDir.Attributes = FileAttributes.Normal;
167163
}
168164

169-
[ConditionalFact(nameof(UsingNewNormalization))]
165+
[Fact]
170166
[PlatformSpecific(TestPlatforms.Windows)] // Deleting extended readonly directory throws IOException
171167
public void WindowsDeleteExtendedReadOnlyDirectory()
172168
{
@@ -197,7 +193,7 @@ public void WindowsShouldBeAbleToDeleteHiddenDirectory()
197193
Assert.False(testDir.Exists);
198194
}
199195

200-
[ConditionalFact(nameof(UsingNewNormalization))]
196+
[Fact]
201197
[PlatformSpecific(TestPlatforms.Windows)] // Deleting extended hidden directory succeeds
202198
public void WindowsShouldBeAbleToDeleteExtendedHiddenDirectory()
203199
{

src/libraries/System.Runtime/tests/System.IO.FileSystem.Tests/Directory/Exists.cs

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -159,7 +159,7 @@ public void SymlinkToNewDirectory()
159159

160160
#region PlatformSpecific
161161

162-
[ConditionalTheory(nameof(UsingNewNormalization)),
162+
[Theory,
163163
MemberData(nameof(ValidPathComponentNames))]
164164
[PlatformSpecific(TestPlatforms.Windows)] // Extended path exists
165165
public void ValidExtendedPathExists_ReturnsTrue(string component)
@@ -169,7 +169,7 @@ public void ValidExtendedPathExists_ReturnsTrue(string component)
169169
Assert.True(Exists(path));
170170
}
171171

172-
[ConditionalFact(nameof(UsingNewNormalization))]
172+
[Fact]
173173
[PlatformSpecific(TestPlatforms.Windows)] // Extended path already exists as directory
174174
public void ExtendedPathAlreadyExistsAsDirectory()
175175
{
@@ -181,7 +181,7 @@ public void ExtendedPathAlreadyExistsAsDirectory()
181181
Assert.True(Exists(IOServices.RemoveTrailingSlash(IOServices.AddTrailingSlashIfNeeded(path))));
182182
}
183183

184-
[ConditionalFact(nameof(AreAllLongPathsAvailable))]
184+
[Fact]
185185
[PlatformSpecific(TestPlatforms.Windows)] // Long directory path doesn't throw on Exists
186186
public void DirectoryLongerThanMaxDirectoryAsPath_DoesntThrow()
187187
{
@@ -219,7 +219,7 @@ public void DoesCaseSensitiveComparisons()
219219
Assert.False(Exists(testDir.FullName.ToLowerInvariant()));
220220
}
221221

222-
[ConditionalTheory(nameof(UsingNewNormalization)),
222+
[Theory,
223223
MemberData(nameof(SimpleWhiteSpace))]
224224
[PlatformSpecific(TestPlatforms.Windows)] // In Windows, trailing whitespace in a path is trimmed appropriately
225225
public void TrailingWhitespaceExistence_SimpleWhiteSpace(string component)
@@ -347,7 +347,7 @@ public void DriveAsPath()
347347
Assert.Contains(IOServices.GetReadyDrives(), drive => Exists(drive));
348348
}
349349

350-
[ConditionalFact(nameof(UsingNewNormalization))]
350+
[Fact]
351351
[PlatformSpecific(TestPlatforms.Windows)] // drive labels
352352
public void ExtendedDriveAsPath()
353353
{
@@ -380,7 +380,7 @@ public void PathAlreadyExistsAsFile()
380380
Assert.False(Directory.Exists(IOServices.RemoveTrailingSlash(IOServices.AddTrailingSlashIfNeeded(path))));
381381
}
382382

383-
[ConditionalFact(nameof(UsingNewNormalization))]
383+
[Fact]
384384
[PlatformSpecific(TestPlatforms.Windows)] // Extended path already exists as file
385385
public void ExtendedPathAlreadyExistsAsFile()
386386
{

src/libraries/System.Runtime/tests/System.IO.FileSystem.Tests/Directory/GetFileSystemEntries_str_str.cs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -300,7 +300,7 @@ public void PatternTests_DosDot(string pattern, string[] sourceFiles, string[] e
300300
}
301301

302302
// Can't do these without extended path support on Windows, UsingNewNormalization filters appropriately
303-
[ConditionalTheory(nameof(UsingNewNormalization)),
303+
[Theory,
304304
// Periods are optional if left of * or ? and end of match
305305
InlineData(
306306
"foo.*",
@@ -357,7 +357,7 @@ public void PatternTests_DosStar(string pattern, string[] sourceFiles, string[]
357357
}
358358

359359
// Can't do these without extended path support on Windows, UsingNewNormalization filters appropriately
360-
[ConditionalTheory(nameof(UsingNewNormalization)),
360+
[Theory,
361361
InlineData(
362362
"foo*.",
363363
new string[] { @"foo", @"foo.", @"foo.t", @"foo.tx", @"foo.txt", @"bar.txt", @"foo..", @"foo...", @"foo .", @"foo. . .", @"foo. t" },
@@ -580,7 +580,7 @@ public void WindowsSearchPatternLongSegment()
580580
GetEntries(testDir.FullName, longName);
581581
}
582582

583-
[ConditionalFact(nameof(AreAllLongPathsAvailable))]
583+
[Fact]
584584
public void SearchPatternLongPath()
585585
{
586586
// Create a destination path longer than the traditional Windows limit of 256 characters

src/libraries/System.Runtime/tests/System.IO.FileSystem.Tests/Directory/GetFiles.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ public void EnumerateWithSymLinkToFile()
4141
Assert.Equal(0, GetEntries(containingFolder.FullName).Count());
4242
}
4343

44-
[ConditionalFact(nameof(AreAllLongPathsAvailable))]
44+
[Fact]
4545
public void EnumerateFilesOverLegacyMaxPath()
4646
{
4747
// We want to test that directories under the legacy MAX_PATH (260 characters, including the null) can iterate files
@@ -61,7 +61,7 @@ public void EnumerateFilesOverLegacyMaxPath()
6161
Assert.Equal(6, files.Length);
6262
}
6363

64-
[ConditionalFact(nameof(AreAllLongPathsAvailable))]
64+
[Fact]
6565
public void EnumerateFilesDirectoryOverLegacyMaxPath()
6666
{
6767
// Check enumerating when the entire path is over MAX_PATH

0 commit comments

Comments
 (0)