Strategy for tests that use local files

[tintoy]

Relates to problems found in dotnet#16657.

Maybe a base class for tests that creates a subdirectory named after the test class with methods child tests can call to create files. Those files created in subdirectories of test subdirectory. Auto deleted when disposed (or when test class is disposed).

Sorry typing on phone so sound like telegram. What do you think, @anthonylangsworth?

[anthonylangsworth]

I don't have a problem with the idea. I am just wondering whether we can do better ... and its late and my brain is fried after stepping through the internals of System.Xml.

[peterwurzinger]

Well to me that sounds like a plan. Maybe concat the name of the directory with a Guid.NewGuid() to ensure not to run into some exceptions after a dirty/incomplete test run.

[tintoy]

I'll look into this today or tomorrow. Should be pretty simple.

[tintoy]

Something like:

class TestWithFilesBase
    : IDisposable
{
    private DirectoryInfo BaseDirectory { get; } = new DirectoryInfo(
        Path.Combine(Directory.GetCurrentDirectory(), "TestFiles", GetType().Name)
    );
    private DirectoryInfo TestDirectory(string testName) => new DirectoryInfo(
        Path.Combine(BaseDirectory.FullName, testName)
    );

    public void Dispose()
    {
        foreach (TempFile tempFile in TempFiles)
            tempFile.Dispose();
    }

    private List<TempFile> TempFiles { get; } = new List<TempFile>();

    protected IDisposable CreateTestFile(string content, string extension, [CallerMemberName] string testName = null)
    {
        return CreateTestFileCore(testName + extension, content);
    }

    IDisposable CreateTestFileCore(string fileName, string content)
    {
        var file = new TempFile(
            Path.Combine(Directory.GetCurrentDirectory(), fileName)
        );

        TempFiles.Add(file);

        File.WriteAllText(file.Path, content);

        return file;
    }
}

How does that sound?

[tintoy]

@anthonylangsworth @peterwurzinger what do you think of this first attempt (707e9d3)?

Directory per test class, and then directory per test method. IIRC, Xunit won't run tests from the same class in parallel, so thread-safety should not be an issue.

[tintoy]

Example usage:

[Fact]
public void SomeTest()
{
    FileInfo tempFile1 = CreateTempFile("doc.dtd", content: "<!-- presence required, not content -->");
    FileInfo tempFile2 = CreateTempFile("world.txt", content: "world");
    using (EnterTempDirectory())
    {
        // Able to use "doc.dtd" and "world.txt" without qualified path names.
    }
}

[tintoy]

I guess my only question then is, if running tests in parallel, how is current directory affected? Are they all in the same process? That'd be painful.

[peterwurzinger]

Looks good to me! I only could review it on my smartphone though.

[tintoy]

If so, then we'll need to get rid of the EnterTempDirectory and modify samples that require temp files to use fully-qualified paths.

For many of the tests, we can use an XmlResolver variant such as XmlPreloadedResolver which means we can modify them to not need temp files at all, but a couple of them are specifically named to indicate that they're designed to run with no resolver configured (e.g. C14NSpecExample1_WithoutResolver).

For the rest, it's as simple as:

XmlPreloadedResolver resolver = new XmlPreloadedResolver();
resolver.Add("doc.dtd", "<!-- presence required, rather than content -->");
resolver.Add("world.txt", "world");

CC: @bartonjs

[peterwurzinger]

Whew, that's a good question. My shot in the dark would be yes, they're running in the same process. Left to prove ofc.

[tintoy]

Checked the xUnit docs:

Tests within the same test class will not run in parallel against each other

So I think we're good :)

[peterwurzinger]

Hm. This sounds surprising to me, considering how many problems I already had to face in the past because tests were running parallel. Could have changed ofc.
If this still causes problems, I remember that there is a switch in app.config to disable parallel execution at all.

[tintoy]

Of course, there is another much simpler way to deal with all this - if we put all tests that use the doc.dtd file in the same test collection (named, for example, "Uses doc.dtd") then xUnit will guarantee that those tests are never run in parallel. Then we can create a collection-level fixture for doc.dtd. Ditto for world.txt.

@bartonjs - I think this option is a lot simpler but I'm not sure if there's a preferred approach. Any input would be welcome :)