Is this Unit Test implemented correctly? [closed]

Question

I have a unit test which is intended to check the access right and also make sure there is enough access right to file system before constructing a class. But, I think my unit test is violating the unit testing principle (FIRST) such as: Isolated/Independent and Thorough which are talking about building an independent unit test and also considering several aspects of failure scenarios. I also believe that this unit test in the way that is implemented is unit testing the .Net framework not the functionality of the code written by me because the underlying class is not mocked. But I would like to know your opinions as well and see how wrong or right I am.

Explanation of the Unit Test:

The unit test is trying to make sure that the constructor of EventFeedFile is executing a method which is checking the access rights. That method called VerifyWritable(). Note that the EventFeedFile is a separate class and the VerifyWritable() is seating in another class.

[Test]
public void DirectoryNotWritableTest()
{
    var tempPath = Path.Combine(Path.GetTempPath(), "EventFileDirectoryWriteDeniedTest");
    int pageSize = 10;

    try
    {
        if (Directory.Exists(tempPath))
            Directory.Delete(tempPath, true);

        var directoryInfo = Directory.CreateDirectory(tempPath);
        var securityRules = directoryInfo.GetAccessControl();
        var writeDeniedRule =
            new FileSystemAccessRule("Everyone", FileSystemRights.CreateFiles, AccessControlType.Deny);
        securityRules.AddAccessRule(writeDeniedRule);
        directoryInfo.SetAccessControl(securityRules);

        // ReSharper disable once ObjectCreationAsStatement
        Assert.Throws<UnauthorizedAccessException>(() => new EventFeedFile(new PersistenceDirectory(tempPath),
            pageSize));
    }
    finally
    {
        if (Directory.Exists(tempPath))
            Directory.Delete(tempPath, true);
    }
}

/// <param name="directory">The directory containing the persistence file.</param>
/// <param name="pageSize">The size of event feed page.</param>
public EventFeedFile(PersistenceDirectory directory, int pageSize)
{
    directory.EnsureExists();
    directory.VerifyWritable();
    mParentDirectory = directory.TargetPath;
    mFullName = Path.Combine(mParentDirectory, Constants.FILE_NAME_EVENTPERSISTENCE);
    mPageSize = pageSize;
}

public void VerifyWritable()
{
    using (File.Create(
        Path.Combine(TargetPath, Path.GetRandomFileName()),
        1024,
        FileOptions.DeleteOnClose))
    {
    }
}

Answer 1

As you said, You are testing EventFeedFile
Also you said VerifyWritable is in a different module.

Then it is pretty obvious VerifyWritable() should be mocked to return or throw respectively to the test intention.
instead of doing Two bad things:

1. Testing the VerifyWritable Code when you didn't want to.
   in case it is faulty, your test will fail on out of scope code (or worse, will pass on faulty code).

2. You are actually Writing files, and Access rules which for my opinion bad practice, will scale badly, and might be changing when switching FileSystems/OperatingSystems (or versions)

Note: The test doesn't actually tests the .NET Framework... it just utilizing the Real Environment, instead of creating a Mocked (Virtual environment) for the Test use.

Answer 2

I don't think there's a clear yes/no answer here.

A unit test should not access external data like a file system. But okay, at some point you have to test it and if the means to do that automatically is a unit testing project in VS then so be it. Leave the naming to the purists, I'm happy with automated tests.

Yes, you are right, the code under test is plain .NET Framework. So yes, it's testing the framework. But it's also testing if somebody actually used the framework to do this job. If the requirement is to throw that exception on construction and not later in the process, then this is a valid test. If someone messes with the code and introduces an error, this test will find it.

So I'd say this is a good test. It's better to have it then to delete it. It makes sure the requirement of checking the accessibility is implemented correctly.