Random temp folder implementation for unit testing

Question

I faced a series of problems when unit testing a tool of mine that generates files. They were caused by me putting the files in the same temporary folder and the tests running in parrallel.

Here is an exemple of the unit tests (for @shanif)

public class OutputWriterTests
{
 [Fact]
 public async Task WriteToDirectoryAsync()
 {
 using var tempFolder = new TempFolder();
 var fileWriter = new OutputWriter(tempFolder.Folder);
 var fileCount = tempFolder.Folder.GetFiles().Length;
 await fileWriter.WriteAsync("toto").ConfigureAwait(false);
 Assert.Equal(fileCount + 1, tempFolder.Folder.GetFiles().Length);
 }
 [Fact]
 public async Task AppendToFileAsync()
 {
 using var tempFolder = new TempFolder();
 var testFile = new FileInfo(Path.Combine(tempFolder.Folder.FullName, "test"));
 var fileWriter = new OutputWriter(testFile);
 await fileWriter.WriteAsync("toto").ConfigureAwait(false);
 var text = File.ReadAllText(testFile.FullName);
 Assert.Equal("toto\r\n", text);
 }
}

As a result I created this simplistic class that handles the creation and deletion of a temporary folder. As it implements IDisposable it's meant to be used inside a using block, or manually disposed.

public class TempFolder : IDisposable
{
 public DirectoryInfo Folder;
 public TempFolder(string prefix = "TempFolder")
 {
 var folderName = prefix + new Random().Next(1000000000);
 Folder = Directory.CreateDirectory(Path.Combine(Path.GetTempPath(), folderName));
 }
 public void Dispose()
 {
 Directory.Delete(Folder.FullName, true);
 }
}

• phipsgabler: Here's some alternatives to your approach. It really made me wonder that windows has no alternative to mkdtemp.
• shanif: I wonder if you can avoid this problem by changing the unit tests. I would not call a test that create file a unit test, more like an integration test Would you like to post here one of the tests?
• shanif: Maybe this will help stackoverflow.com/q/1087351/1723352
• Etsitpab Nioliv: @shanif I added a test
• redcalx: It's also worth considering using Path.GetTempPath() instead of generating your own random folder name.

Answer 1

You could easily wind up with collisions on the folder name if the method is called multiple times in very quick succession because of the new Random() being declared for each usage. It should be instantiated once, and since you mentioned it could be called on multiple threads, it should be mutexed onto one at a time. Finally, it's not a good idea to have fields be public - making it a property is luckily pretty easy. So:

public class TempFolder : IDisposable
{
 private static readonly Random _Random = new Random();
 public DirectoryInfo Folder { get; }
 public TempFolder(string prefix = "TempFolder")
 {
 string folderName;
 lock (_Random)
 {
 folderName = prefix + _Random.Next(1000000000);
 }
 Folder = Directory.CreateDirectory(Path.Combine(Path.GetTempPath(), folderName));
 }
 public void Dispose()
 {
 Directory.Delete(Folder.FullName, true);
 }
}

Now, what I don't address here is the issue of constructors or the Dispose() method possibly throwing unexpected exceptions. Just know that's a possibility and you may want to catch them and/or re-raise them depending on your use case.

• nogjam: +1 for the "Are protected members/fields really that bad?" link. I learned something new.