search

urasandesu / Prig

Prig is a lightweight framework for test indirections in .NET Framework.
Other
117 stars 21 forks source link

MsTest: Test runner will crash when stubbing a shared member that shouldn't matter #91

Closed urasandesu closed 7 years ago

urasandesu commented 7 years ago

When creating a test like the following example, MsTest will crash:

Product Code

public class Foo
{
    public static void Do()
    {
        throw new InvalidOperationException("We shouldn't get here!!");
    }
}

Indirection Stub Setting for Product Code

<!-- 
    PFoo.Do().Body = 
        () => 
        {   
            throw new NotImplementedException();
        };
-->
<add name="Do" alias="Do">
  <RuntimeMethodInfo xmlns:i="http://www.w3.org/2001/XMLSchema-instance" xmlns:x="http://www.w3.org/2001/XMLSchema" z:Id="1" z:FactoryType="MemberInfoSerializationHolder" z:Type="System.Reflection.MemberInfoSerializationHolder" z:Assembly="0" xmlns:z="http://schemas.microsoft.com/2003/10/Serialization/" xmlns="http://schemas.datacontract.org/2004/07/System.Reflection">
    <Name z:Id="2" z:Type="System.String" z:Assembly="0" xmlns="">Do</Name>
    <AssemblyName z:Id="3" z:Type="System.String" z:Assembly="0" xmlns="">MsTest_MSCorLibAndAnother, Version=1.0.0.0, Culture=neutral, PublicKeyToken=null</AssemblyName>
    <ClassName z:Id="4" z:Type="System.String" z:Assembly="0" xmlns="">MsTest_MSCorLibAndAnother.Foo</ClassName>
    <Signature z:Id="5" z:Type="System.String" z:Assembly="0" xmlns="">Void Do()</Signature>
    <Signature2 z:Id="6" z:Type="System.String" z:Assembly="0" xmlns="">System.Void Do()</Signature2>
    <MemberType z:Id="7" z:Type="System.Int32" z:Assembly="0" xmlns="">8</MemberType>
    <GenericArguments i:nil="true" xmlns="" />
  </RuntimeMethodInfo>
</add>

Indirection Stub Setting for a shared member with MsTest that shouldn't matter In this example, we selected List<T>.Add(T). There is Environment.CurrentDirectory, but this means for to output crash dump easily.

<!-- 
    PList<T>.AddT().Body = 
        (@this, item) => 
        {   
            throw new NotImplementedException();
        };
-->
<add name="AddT" alias="AddT">
  <RuntimeMethodInfo xmlns:i="http://www.w3.org/2001/XMLSchema-instance" xmlns:x="http://www.w3.org/2001/XMLSchema" z:Id="1" z:FactoryType="MemberInfoSerializationHolder" z:Type="System.Reflection.MemberInfoSerializationHolder" z:Assembly="0" xmlns:z="http://schemas.microsoft.com/2003/10/Serialization/" xmlns="http://schemas.datacontract.org/2004/07/System.Reflection">
    <Name z:Id="2" z:Type="System.String" z:Assembly="0" xmlns="">Add</Name>
    <AssemblyName z:Id="3" z:Type="System.String" z:Assembly="0" xmlns="">mscorlib, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089</AssemblyName>
    <ClassName z:Id="4" z:Type="System.String" z:Assembly="0" xmlns="">System.Collections.Generic.List`1</ClassName>
    <Signature z:Id="5" z:Type="System.String" z:Assembly="0" xmlns="">Void Add(T)</Signature>
    <Signature2 z:Id="6" z:Type="System.String" z:Assembly="0" xmlns="">System.Void Add(!T)</Signature2>
    <MemberType z:Id="7" z:Type="System.Int32" z:Assembly="0" xmlns="">8</MemberType>
    <GenericArguments i:nil="true" xmlns="" />
  </RuntimeMethodInfo>
</add>

<!-- 
    PEnvironment.CurrentDirectorySetString().Body = 
        value => 
        {   
            throw new NotImplementedException();
        };
-->
<add name="CurrentDirectorySetString" alias="CurrentDirectorySetString">
  <RuntimeMethodInfo xmlns:i="http://www.w3.org/2001/XMLSchema-instance" xmlns:x="http://www.w3.org/2001/XMLSchema" z:Id="1" z:FactoryType="MemberInfoSerializationHolder" z:Type="System.Reflection.MemberInfoSerializationHolder" z:Assembly="0" xmlns:z="http://schemas.microsoft.com/2003/10/Serialization/" xmlns="http://schemas.datacontract.org/2004/07/System.Reflection">
    <Name z:Id="2" z:Type="System.String" z:Assembly="0" xmlns="">set_CurrentDirectory</Name>
    <AssemblyName z:Id="3" z:Type="System.String" z:Assembly="0" xmlns="">mscorlib, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089</AssemblyName>
    <ClassName z:Id="4" z:Type="System.String" z:Assembly="0" xmlns="">System.Environment</ClassName>
    <Signature z:Id="5" z:Type="System.String" z:Assembly="0" xmlns="">Void set_CurrentDirectory(System.String)</Signature>
    <Signature2 z:Id="6" z:Type="System.String" z:Assembly="0" xmlns="">System.Void set_CurrentDirectory(System.String)</Signature2>
    <MemberType z:Id="7" z:Type="System.Int32" z:Assembly="0" xmlns="">8</MemberType>
    <GenericArguments i:nil="true" xmlns="" />
  </RuntimeMethodInfo>
</add>

Test Code The point is that the above shared member is not used anywhere in our test code.

[TestClass]
public class UnitTest1
{
    [TestMethod]
    public void TestMethod1()
    {
        using (new IndirectionsContext())
        {
            PFoo.Do().Body = () => { };
        }
    }
}

Crash Dump vstest.executionengine.exe.11312.txt

urasandesu commented 7 years ago

As a result of our investigation, we found that this problem cause is same as the reason that we are merging NUnit Test Adapter as Vendor Branch.

Indirection Stub is unintendedly crossed plural AppDomains by MsTest.

Against NUnit Test Adapter, we modified to fix that -- always running with DomainUsage=None. Currently, we can't respond by same way because MsTest is closed source. However, Microsoft plans to open source MsTest (see Anand Kamat's comment). We might be able to fix same way if it will be achieved in the future.

andy250 commented 7 years ago

Does this mean that we can try to use NUnit instead of MSTest and that might be a workaround for #89?

urasandesu commented 7 years ago

I think that NUnit will not be a complete workarround.

I guess that the test crash won't be to occur. But note that Prig couldn't be executed in the combination for NUnit and ReSharper when I tried.

urasandesu commented 7 years ago

MEMO I found another way to fix. To suppress unintendedly crossing plural AppDomains, we should strengthen the inspection that is performing by native code (JITed code). It seems that AppDomainID can be used for that. For example,

  1. In the managed code, we save AppDomainID and function pointer map (this is now using) to new map when we set Indirection Stub at first.
  2. In the native code (JITed code), we firstly check the new map by AppDomainID.

Thereby, Indirection Stub should never call different from AppDomain that the stub is set. However, we should be careful about the behavior of public interface, or this way will break backward-compatibility easily.

urasandesu commented 7 years ago

MEMO To avoid FileLoadException, it turned out that we have to register framework assemblies (Urasandesu.NAnonym, Urasandesu.Prig.Framework) to GAC. So we should add the processing to our installer after this.

urasandesu commented 7 years ago

MEMO Some problems still remain if executing in multi-thread. We are going to release next version when they are resolved.

urasandesu commented 7 years ago

MEMO We have completed correcting all issues! :bowtie:

Remaining works are updating samples to next version and reviewing documents. We probably release next version within two weeks if there aren't any problems.

urasandesu commented 7 years ago

MEMO It turned out that a deadlock has been detected when running some tests from console runner.