Improve Tests

[TrabacchinLuigi]

Unit tests seem to be working as integration tests: mounting a disk and using that disk. Because of that moq is being used to verify calls from the driver instead of creating mocks to be fed to the system under test. Also, it is impossible for now to use moq with Spans https://github.com/moq/moq4/issues/1049 (and i belive we should not be held back by that)

I think we should try to use the marshal class to tests if the class / struct we wrote can be marshalled https://docs.microsoft.com/en-us/dotnet/api/system.runtime.interopservices.marshal.copy

[LTRData]

Not sure really what you are trying to do here. Structures can be directly pinned and used by unmanaged code through pinned pointer if the structure is blittable. You can test that using the unmanaged type constraint on a generic type definition for example. Other kind of structs need marshalling. So effectively, if the struct in unmanaged in this way we could instead use MemoryMarshal class to directly get references to elements of arrays, copy from and to unmanaged memory etc. https://docs.microsoft.com/en-us/dotnet/api/system.runtime.interopservices.memorymarshal

[TrabacchinLuigi]

To be honest i haven't already been through all the tests, so i'm not 100% sure of what need to be done. My goal is to keep the spans in my fork, and propose also some tests to ensure no mistakes where made which the compiler can't check (for instance when I've changed the DokanFileInfo into a struct it was compiling right away, but during runtime it was complaining about the no more valid attribute MarshalAs(UnmanagedType.LPStruct)

Also now i'm receiving contexts that seem to have lost their context, probably because i need to pass them down by reference, but the compiler won't say anything

[LTRData]

Where did you get the error about MarshalAs attribute? I think you could compare your code to my fork. I turned DokanFileInfo to a struct some time ago and it has been used a lot in some applications since that change. I just checked in latest changes with some code cleanups such as auto-properties etc. https://github.com/LTRData/dokan-dotnet/blob/LTRData.DokanNet-initial/DokanNet/DokanFileInfo.cs

[TrabacchinLuigi]

Yes, i went to have a look, it was on the DokanOperationProxy, i'm trying not to watch too often or i won't understand what i'm doing 🤣 i end up doing the same thing, remove the marshalas attribute at all

[LTRData]

Okay, MarshalAs attribute is really only needed for selecting a specific method for marshalling. There is no reason to use that attribute when passing blittable structures by reference since that is by default done simply using a pointer to the structure instance, without any marshalling or memory copies.

[TrabacchinLuigi]

I've figured out I can do something like this (the examples are for my branch or @LTRData branches which uses DokanFileInfo as a structure)

[TestClass]
public class MarshallingTests
{
    [TestMethod]
    public void DokanOperations_should_be_marshallable()
    {
        var sut = new DOKAN_OPERATIONS();
        _ = new NativeStructWrapper<DOKAN_OPERATIONS>(sut);
    }

    [TestMethod]
    public void DOKAN_OPERATIONS_ZwCreateFile_should_be_marshallable()
    {
        static NtStatus CreateFile(
            string rawFileName,
            IntPtr securityContext,
            uint rawDesiredAccess,
            uint rawFileAttributes,
            uint rawShareAccess,
            uint rawCreateDisposition,
            uint rawCreateOptions,
            ref DokanFileInfo dokanFileInfo) { return NtStatus.Unsuccessful; }

        var sut = new DOKAN_OPERATIONS()
        {
            ZwCreateFile = new DokanOperationProxy.ZwCreateFileDelegate(CreateFile)
        };
        _ = new NativeStructWrapper<DOKAN_OPERATIONS>(sut);
    }

    [TestMethod]
    public void DOKAN_OPERATIONS_CleanUp_should_be_marshallable()
    {
        static void CleanUp(string rawFileName, ref DokanFileInfo rawFileInfo) { }

        var sut = new DOKAN_OPERATIONS()
        {
            Cleanup = new DokanOperationProxy.CleanupDelegate(CleanUp)
        };
        _ = new NativeStructWrapper<DOKAN_OPERATIONS>(sut);
    }
}

Then if i go to the delegates and sabotage them like this

public delegate NtStatus ZwCreateFileDelegate(
  [MarshalAs(UnmanagedType.LPWStr)] string rawFileName,
  IntPtr securityContext,
  uint rawDesiredAccess,
  uint rawFileAttributes,
  uint rawShareAccess,
  uint rawCreateDisposition,
  uint rawCreateOptions,
  [MarshalAs(UnmanagedType.LPStruct), In, Out] ref DokanFileInfo dokanFileInfo);

The second test method will fail

[TrabacchinLuigi]

@Liryna I've updated my branch, what do you think we need to be tested ? i could move the integration tests to a dedicated project, but i feel this is a more sensible way to tests what dokan.net is doing, Dokany should have it's own tests

[Liryna]

So the Mock test looks to be a good idea but in reality it is way harder to maintain it (env changes that issue new IO from a third party make the test fail) than it actually detect issues.

We have tools that generates IO like winfstest that are more adapted https://github.com/dokan-dev/dokan-dotnet/blob/master/appveyor.yml#L91 We are missing fsx https://github.com/dokan-dev/dokany/blob/master/samples/memfs_test.ps1#L151-L153 which should be added.

What I suggest is that dokan-dotnet test ensure the code outside the simple native code wrapping be tested. Like the BufferPool / DokanHelper ... If we have a native wrapping issue, we should have the previous tools detect it. Hope that make sense.