6
\$\begingroup\$

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);
    }
}

\$\endgroup\$
6
  • \$\begingroup\$ Here's some alternatives to your approach. It really made me wonder that windows has no alternative to mkdtemp. \$\endgroup\$ Commented Apr 23, 2020 at 6:30
  • \$\begingroup\$ 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? \$\endgroup\$
    – shanif
    Commented Apr 23, 2020 at 11:47
  • \$\begingroup\$ Maybe this will help stackoverflow.com/q/1087351/1723352 \$\endgroup\$
    – shanif
    Commented Apr 23, 2020 at 11:49
  • \$\begingroup\$ @shanif I added a test \$\endgroup\$ Commented Apr 24, 2020 at 17:29
  • 1
    \$\begingroup\$ It's also worth considering using Path.GetTempPath() instead of generating your own random folder name. \$\endgroup\$
    – redcalx
    Commented Jun 27, 2021 at 20:18

1 Answer 1

10
\$\begingroup\$

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.

\$\endgroup\$
1

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.