Fix cases where tests fail to clean up files placed in the Temp folder#26320
Fix cases where tests fail to clean up files placed in the Temp folder#26320sharwell merged 1 commit intodotnet:dev15.7.xfrom
Conversation
| Assert.Contains(source + "(6,16): warning CS0168: The variable 'x' is declared but never used", outWriter.ToString(), StringComparison.Ordinal); | ||
|
|
||
| CleanupAllGeneratedFiles(source); | ||
| CleanupAllGeneratedFiles(Path.Combine(Path.GetDirectoryName(Path.GetDirectoryName(source)), Path.GetFileName(source))); |
There was a problem hiding this comment.
Looking through this made me ask: why isn't TempRoot deleting all the temporary files it creates in it's active Dispose pattern? Seems like it would make most of this superfluous.
There was a problem hiding this comment.
Not sure. Most of the files end up placed under TempRoot.Root, where several files/folders are not cleaned up after tests. However, this folder is actually placed in the RoslynTests subdirectory of the temporary directory, so it doesn't cause a problem or make it difficult to find/delete just the test files.
There was a problem hiding this comment.
I'm tempted to change the TempRoot dispose to have this behavior. It seems pretty sensible. That or have the TempRoot dispose assert / fail if the directory isn't empty.
There was a problem hiding this comment.
IIRC there might have been some file locking issues, but if we're not seeing them with this change, then yes, we should just move it.
|
@jaredpar @jasonmalinowski Any objection to merging this as a test-only change for dev15.7.x and leaving the remaining comments for future work? |
|
@sharwell none from me. My comments were all intended for future work. |
| Assert.Contains(source + "(6,16): warning CS0168: The variable 'x' is declared but never used", outWriter.ToString(), StringComparison.Ordinal); | ||
|
|
||
| CleanupAllGeneratedFiles(source); | ||
| CleanupAllGeneratedFiles(Path.Combine(Path.GetDirectoryName(Path.GetDirectoryName(source)), Path.GetFileName(source))); |
There was a problem hiding this comment.
IIRC there might have been some file locking issues, but if we're not seeing them with this change, then yes, we should just move it.
No description provided.