Update tests for Path changes #27348
Conversation
| Assert.ThrowsAny<IOException>(() => | ||
| { | ||
| var catalog = catalogCreator("??||>"); | ||
| }); |
There was a problem hiding this comment.
Nit: doesn't need to store the result:
Assert.ThrowsAny<IOException>(() => catalogCreator("??||>"));There was a problem hiding this comment.
I was following along with the rest of the file's style. I'll go ahead and change all of them.
| public void PathWithInvalidCharactersAsPath_Core(string invalidPath) | ||
| { | ||
| if (invalidPath.Contains('\0')) | ||
| Assert.Throws<ArgumentException>(() => Create(invalidPath)); |
There was a problem hiding this comment.
Is there a param name we can include/test here?
There was a problem hiding this comment.
Not for desktop, but now that I've split it up I can (and will) add it for core.
| Assert.Throws<NotSupportedException>(() => Create(invalidPath)); | ||
| if (PathFeatures.IsUsingLegacyPathNormalization()) | ||
| { | ||
| Assert.Throws<ArgumentException>(() => Create(invalidPath)); |
There was a problem hiding this comment.
Same question for all of these arg exceptions.
There was a problem hiding this comment.
For desktop the answer is no for ArgumentException. IOException doesn't have a parameter and I'm not sure we could add one in a way that made sense.
| public void WindowsControlWhiteSpace_Core(string component) | ||
| { | ||
| // CreateSubdirectory will throw when passed a path with control whitespace e.g. "\t" | ||
| string path = IOServices.RemoveTrailingSlash(GetTestFileName()); |
There was a problem hiding this comment.
I'm wondering why we had this path statement here in the existing test you copied from when it's not going to throw exception? the input argument is component which is being used in the next statement not to compute path.
Am I missing something?
There was a problem hiding this comment.
I just missed the fact that it wasn't used as I plowed through these. Good catch.
Updates for dotnet/coreclr#16478
…same error on Unix)
JeremyKuhne
force-pushed
the
pathtest2
branch
from
c79aadb to
4517e32
Compare
* Update tests for Path changes Updates for dotnet/coreclr#16478 * Add back Unix active issue (wishful thinking that this would get the same error on Unix) * Address feedback Commit migrated from dotnet/corefx@92afa6e
3 participants
Updates for dotnet/coreclr#16478
Tagged these tests with #27269 as they're related. Will update for both PRs from CoreCLR.
cc: @danmosemsft