-
Notifications
You must be signed in to change notification settings - Fork 4.9k
Fix System.IO.Packaging.Tests in uap to create files in temp directory using FileCleanupTestBase helper class #21029
Conversation
// Console.WriteLine("{0}:{1}", test, fiGuidName.Name); | ||
return fiGuidName; | ||
var existingDoc = new FileInfo(existingFileName); | ||
var content = File.ReadAllBytes(existingDoc.FullName); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: var => byte[]
{ | ||
private const string Mime_MediaTypeNames_Text_Xml = "text/xml"; | ||
private const string Mime_MediaTypeNames_Image_Jpeg = "image/jpeg"; // System.Net.Mime.MediaTypeNames.Image.Jpeg | ||
private const string s_DocumentXml = @"<Hello>Test</Hello>"; | ||
private const string s_ResourceXml = @"<Resource>Test</Resource>"; | ||
|
||
private static FileInfo GetFileSavedWithGuidName(string test, string name) | ||
private FileInfo GetFileSavedWithGuidName(string existingFileName) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: does this function need a different name now?
} | ||
|
||
public static FileInfo GetGuidNameForNewFile(string test, string extension) | ||
public FileInfo GetGuidNameForNewFile(string extension) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: same question about function name
// If a test leaves a Test-* file in existence, then uncomment following line to determine which test it was. | ||
// Console.WriteLine("{0}:{1}", test, fiDoc.Name); | ||
return fiDoc; | ||
return new FileInfo($"{GetTestFilePath()}.{extension}"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One of the benefits of GetTestFilePath is it includes in the file name information about the test being used, in case the file gets left behind or there's an issue with it, or whatnot. That's done using Caller attributes, which are filled in by the compiler based on the call site. But that scheme is then defeated somewhat when the GetTestFilePath is in a helper method rather than the actual test. You might consider adding such Caller attribute arguments to these helpers and then passing the state through explicitly to GetTestFilePath.
f1d0274
to
5e805bf
Compare
…y using FileCleanupTestBase helper class
5e805bf
to
87cf02e
Compare
I've addressed PR feedback. Thanks @stephentoub for the feedback :) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM.
System.IO.Packaging.Tests where failing in uap due to UnauthorizedAccessException because it was trying to create files inside the test folder and since they run inside appx it is not authorized.
I refactored the tests to use FileCleanupTestBase helper class as a base class to create and remove files. This fixes all test failures in Uap for this test project.
cc: @danmosemsft @JeremyKuhne @joperezr