tonerdo / pose

Replace any .NET method (including static and non-virtual) with a delegate
MIT License
1.08k stars 74 forks source link

NullReferenceException in Pose.Helpers.StubHelper.GetMatchingShimIndex #3

Open tjrobinson opened 7 years ago

tjrobinson commented 7 years ago

Using Pose in a fairly complex test I've been getting this exception. It seems to be trying to find a shim for method(s) that haven't been shimmed. Unfortunately I've not been able to reproduce this in a simple example and can't post the full code from my product.

The test project is using an in-memory ASP.NET Core server (https://docs.microsoft.com/en-us/aspnet/core/testing/integration-testing) and the method I'm shimming is called from the Controller, not directly from the unit test itself. I expect this is the underlying problem as I'm calling PoseContext.Isolate from the "client" and expecting it to maintain that isolation on the "server" side.

Anyway, I'm posting this anyway in case you have any suggestions.

Unit test:

#if !NET462
        [Fact]
        public void When_person_exists_in_db()
        {
            var sqlMapperShim = Shim.Replace(() => SqlMapper.QueryAsync<Applicant>(Is.A<IDbConnection>(), Is.A<CommandDefinition>()))
                .With(
                    delegate (IDbConnection dbConnection, CommandDefinition commandDefinition)
                    {
                        return Task.FromResult(SqlMapper.Query<Applicant>(dbConnection, commandDefinition));
                    });

            Default<ApplicantListViewModel> response = null;

            PoseContext.Isolate(() =>
            {
                var httpResponse = this.Read($"organisations/{TestConstants.OrganisationIdentifier}/applicants");
                response = httpResponse.Content.ReadAsAsync<Default<ApplicantListViewModel>>().Result;
            },
            sqlMapperShim);

            // Assert
            response.ShouldNotBe(null);
        }
#endif

Test results:

Test run for C:\Code\MyCompany\MyProduct\test\MyCompany.MyProduct.Specs\bin\Debug\netcoreapp2.0\MyCompany.MyProduct.Specs.dll(.NETCoreApp,Version=v2.0)
Microsoft (R) Test Execution Command Line Tool Version 15.3.0-preview-20170628-02
Copyright (c) Microsoft Corporation.  All rights reserved.

Starting test execution, please wait...
[xUnit.net 00:00:01.4649449]   Discovering: MyCompany.MyProduct.Specs
[xUnit.net 00:00:01.5546231]   Discovered:  MyCompany.MyProduct.Specs
[xUnit.net 00:00:01.5617069]   Starting:    MyCompany.MyProduct.Specs
[xUnit.net 00:00:53.7768297]     MyCompany.MyProduct.Specs.ApplicantsSpecs.ApplicantListSpecs.When_person_exists_in_db2 [FAIL]
[xUnit.net 00:00:53.7786980]       System.NullReferenceException : Object reference not set to an instance of an object.
[xUnit.net 00:00:53.7800539]       Stack Trace:
[xUnit.net 00:00:53.7811621]         C:\Code\MyCompany\MyProduct\test\MyCompany.MyProduct.Specs\Pose\IL\MethodRewriter.cs(27,0): at Pose.IL.MethodRewriter.Rewrite()
[xUnit.net 00:00:53.7813452]            at stub_System.Net.Http.Headers.HttpHeaders_ParseRawHeaderValues(HttpHeaders , String , HeaderStoreItemInfo , Boolean , RuntimeMethodHandle , RuntimeTypeHandle )
[xUnit.net 00:00:53.7814211]            at dynamic_System.Net.Http.Headers.HttpHeaders_AddHeaders(HttpHeaders , HttpHeaders )
[xUnit.net 00:00:53.7814574]            at stub_System.Net.Http.Headers.HttpHeaders_AddHeaders(HttpHeaders , HttpHeaders , RuntimeMethodHandle , RuntimeTypeHandle )
[xUnit.net 00:00:53.7814902]            at dynamic_System.Net.Http.Headers.HttpRequestHeaders_AddHeaders(HttpRequestHeaders , HttpHeaders )
[xUnit.net 00:00:53.7815276]            at stub_System.Net.Http.Headers.HttpHeaders_AddHeaders(HttpHeaders , HttpHeaders , RuntimeMethodHandle , RuntimeTypeHandle )
[xUnit.net 00:00:53.7815669]            at stub_System.Net.Http.HttpClient_PrepareRequestMessage(HttpClient , HttpRequestMessage , RuntimeMethodHandle , RuntimeTypeHandle )
[xUnit.net 00:00:53.7816352]            at dynamic_System.Net.Http.HttpClient_SendAsync(HttpClient , HttpRequestMessage , HttpCompletionOption , CancellationToken )
[xUnit.net 00:00:53.7816805]            at stub_System.Net.Http.HttpClient_SendAsync(HttpClient , HttpRequestMessage , HttpCompletionOption , CancellationToken , RuntimeMethodHandle , RuntimeTypeHandle )
[xUnit.net 00:00:53.7817409]            at dynamic_System.Net.Http.HttpClient_GetAsync(HttpClient , Uri , HttpCompletionOption , CancellationToken )
[xUnit.net 00:00:53.7817783]            at stub_System.Net.Http.HttpClient_GetAsync(HttpClient , Uri , HttpCompletionOption , CancellationToken , RuntimeMethodHandle , RuntimeTypeHandle )
[xUnit.net 00:00:53.7818927]            at dynamic_System.Net.Http.HttpClient_GetAsync(HttpClient , Uri , HttpCompletionOption )
[xUnit.net 00:00:53.7819275]            at stub_System.Net.Http.HttpClient_GetAsync(HttpClient , Uri , HttpCompletionOption , RuntimeMethodHandle , RuntimeTypeHandle )
[xUnit.net 00:00:53.7820015]            at dynamic_System.Net.Http.HttpClient_GetAsync(HttpClient , Uri )
[xUnit.net 00:00:53.7820570]            at stub_System.Net.Http.HttpClient_GetAsync(HttpClient , Uri , RuntimeMethodHandle , RuntimeTypeHandle )
[xUnit.net 00:00:53.7821121]            at stub_System.Net.Http.HttpClient_GetAsync(HttpClient , String , RuntimeMethodHandle , RuntimeTypeHandle )
[xUnit.net 00:00:53.7821604]            at dynamic_MyCompany.MyProduct.Specs.BehavesLikeApi_Read(BehavesLikeApi , String , Dictionary`2 , Dictionary`2 )
[xUnit.net 00:00:53.7822409]            at stub_MyCompany.MyProduct.Specs.BehavesLikeApi_Read(BehavesLikeApi , String , Dictionary`2 , Dictionary`2 , RuntimeMethodHandle , RuntimeTypeHandle )
[xUnit.net 00:00:53.7823764]            at dynamic_MyCompany.MyProduct.Specs.ApplicantsSpecs.ApplicantListSpecs+<>c__DisplayClass3_0_<When_person_exists_in_db2>b__1(<>c__DisplayClass3_0 )
Failed   MyCompany.MyProduct.Specs.ApplicantsSpecs.ApplicantListSpecs.When_person_exists_in_db2
Error Message:
 System.NullReferenceException : Object reference not set to an instance of an object.
Stack Trace:
   at Pose.IL.MethodRewriter.Rewrite() in C:\Code\MyCompany\MyProduct\test\MyCompany.MyProduct.Specs\Pose\IL\MethodRewriter.cs:line 27
   at stub_System.Net.Http.Headers.HttpHeaders_ParseRawHeaderValues(HttpHeaders , String , HeaderStoreItemInfo , Boolean , RuntimeMethodHandle , RuntimeTypeHandle )
   at dynamic_System.Net.Http.Headers.HttpHeaders_AddHeaders(HttpHeaders , HttpHeaders )
   at stub_System.Net.Http.Headers.HttpHeaders_AddHeaders(HttpHeaders , HttpHeaders , RuntimeMethodHandle , RuntimeTypeHandle )
   at dynamic_System.Net.Http.Headers.HttpRequestHeaders_AddHeaders(HttpRequestHeaders , HttpHeaders )
   at stub_System.Net.Http.Headers.HttpHeaders_AddHeaders(HttpHeaders , HttpHeaders , RuntimeMethodHandle , RuntimeTypeHandle )
   at stub_System.Net.Http.HttpClient_PrepareRequestMessage(HttpClient , HttpRequestMessage , RuntimeMethodHandle , RuntimeTypeHandle )
   at dynamic_System.Net.Http.HttpClient_SendAsync(HttpClient , HttpRequestMessage , HttpCompletionOption , CancellationToken )
   at stub_System.Net.Http.HttpClient_SendAsync(HttpClient , HttpRequestMessage , HttpCompletionOption , CancellationToken , RuntimeMethodHandle , RuntimeTypeHandle )
   at dynamic_System.Net.Http.HttpClient_GetAsync(HttpClient , Uri , HttpCompletionOption , CancellationToken )
   at stub_System.Net.Http.HttpClient_GetAsync(HttpClient , Uri , HttpCompletionOption , CancellationToken , RuntimeMethodHandle , RuntimeTypeHandle )
   at dynamic_System.Net.Http.HttpClient_GetAsync(HttpClient , Uri , HttpCompletionOption )
   at stub_System.Net.Http.HttpClient_GetAsync(HttpClient , Uri , HttpCompletionOption , RuntimeMethodHandle , RuntimeTypeHandle )
   at dynamic_System.Net.Http.HttpClient_GetAsync(HttpClient , Uri )
   at stub_System.Net.Http.HttpClient_GetAsync(HttpClient , Uri , RuntimeMethodHandle , RuntimeTypeHandle )
   at stub_System.Net.Http.HttpClient_GetAsync(HttpClient , String , RuntimeMethodHandle , RuntimeTypeHandle )
   at dynamic_MyCompany.MyProduct.Specs.BehavesLikeApi_Read(BehavesLikeApi , String , Dictionary`2 , Dictionary`2 )
   at stub_MyCompany.MyProduct.Specs.BehavesLikeApi_Read(BehavesLikeApi , String , Dictionary`2 , Dictionary`2 , RuntimeMethodHandle , RuntimeTypeHandle )
   at dynamic_MyCompany.MyProduct.Specs.ApplicantsSpecs.ApplicantListSpecs+<>c__DisplayClass3_0_<When_person_exists_in_db2>b__1(<>c__DisplayClass3_0 )
[xUnit.net 00:01:10.9337665]   Finished:    MyCompany.MyProduct.Specs

Total tests: 47. Passed: 46. Failed: 1. Skipped: 0.
Test Run Failed.
tonerdo commented 7 years ago

@tjrobinson if the client test actually calls the code in the controller instead of making an actual http request you should be fine. I'm looking into this as well as the other issues you raised. Have you been able to get some other tests to work with Pose?

tonerdo commented 7 years ago

Also what's happening here is that a stub is passing a null method to be rewritten.

tjrobinson commented 7 years ago

if the client test actually calls the code in the controller instead of making an actual http request you should be fine

Yep, I think it would be. Unfortunately these are deliberate integration tests to check the API layer. I have other tests for the MediatR commands/queries that the controllers use.

I haven't got any other tests working with Pose yet as I've not tried yet.

tonerdo commented 7 years ago

Alrighty. Do try if/when you can and let me know if you run into anymore problems. Thanks

tonerdo commented 7 years ago

@tjrobinson I'm curious, what made you think the error was from Pose.Helpers.StubHelper.GetMatchingShimIndex? The exception stack trace doesn't include it. Also I notice you're using Pose directly in source code form, you can update your local with this branch https://github.com/tonerdo/pose/tree/stub-enhancements and try the code again. Paste the exception output here, I added a few things to make it easier to track the origin of exceptions like this

tjrobinson commented 7 years ago

@tonerdo GetMatchingShimIndex was coming up in the stack trace when I was trying a few things out earlier on though as you noticed, it's not in the one above. I think it's a different trace because I tried to simplify the failing scenario and ended up with a variation on the problem. Sorry I don't have the earlier stack trace anymore.

I can't try the stub-enhancements branch at the moment but will if I get time at some other point .

jonkeda commented 6 years ago

@tonerdo I have the same Null Reference Exception popping up. And tracked it down to GetRuntimeMethodForVirtual

The following code throws the null reference exception.

using System;
using Microsoft.VisualStudio.TestTools.UnitTesting;
namespace Pose.Tests.Fails
{
    [TestClass]
    public class ShimFails
    {
        public class MyClass
        {
            public int MyProperty { get; set; }
        }

        [TestMethod]
        [ExpectedException(typeof(NullReferenceException))]
        public void ShimConstructor()
        {
            Shim ctorShim = Shim.Replace(() => new MyClass())
                .With(() => new MyClass { MyProperty = 10 });

            PoseContext.Isolate(() =>
            {
                // this line breaks the Shim
                Assert.AreEqual(10, 10);

            }, ctorShim);
        }

    }
}

It was trying to find: RuntimeType.GetTypeInfo but the actual method name was System.Reflection.TypeInfo.System.Reflection.IReflectableType.GetTypeInfo(). I tried to return that but it didn't seem to help.

After that I was getting confused.

Hope this helps.

tonerdo commented 6 years ago

I've noticed that Assert statements tend to break Pose. I'm currently looking into it, but in the meantime you can do this instead:

using System;
using Microsoft.VisualStudio.TestTools.UnitTesting;
namespace Pose.Tests.Fails
{
    [TestClass]
    public class ShimFails
    {
        public class MyClass
        {
            public int MyProperty { get; set; }
        }

        [TestMethod]
        [ExpectedException(typeof(NullReferenceException))]
        public void ShimConstructor()
        {
            Shim ctorShim = Shim.Replace(() => new MyClass())
                .With(() => new MyClass { MyProperty = 10 });

            int prop = 0;

            PoseContext.Isolate(() =>
            {
                prop = new MyClass().MyProperty;
            }, ctorShim);

            // this line breaks the Shim
            Assert.AreEqual(10, prop);
        }
    }
}
jonkeda commented 6 years ago

I tried looking into the issue. Unfortunately I haven't found a solution yet. I tried the following:

  1. The RuntimeMethodForVirtual returns null after which GetIndexOfMatchingShim will fail. In the Assert case Type is System.RunTimeType and methodInfo is GetTypeInfo
    
        public static MethodInfo GetRuntimeMethodForVirtual(Type type, MethodInfo methodInfo)
        {
            BindingFlags bindingFlags = BindingFlags.Instance | (methodInfo.IsPublic ? BindingFlags.Public : BindingFlags.NonPublic);
            Type[] types = methodInfo.GetParameters().Select(p => p.ParameterType).ToArray();
            return type.GetMethod(methodInfo.Name, bindingFlags, null, types, null);
        }

2. One solution I tried was to find the method in anotherway. ie:
    public static MethodInfo GetRuntimeMethodForVirtual(Type type, MethodInfo methodInfo)
    {
        BindingFlags bindingFlags = BindingFlags.Instance | (methodInfo.IsPublic ? BindingFlags.Public : BindingFlags.NonPublic);
        Type[] types = methodInfo.GetParameters().Select(p => p.ParameterType).ToArray();
        MethodInfo foundMethod = type.GetMethod(methodInfo.Name, bindingFlags, null, types, null);
        if (foundMethod == null)
        {
            MethodInfo[] methods = type.GetMethods(BindingFlags.Instance | BindingFlags.Public | BindingFlags.NonPublic);
            foundMethod = methods.FirstOrDefault(i => i.Name.EndsWith(methodInfo.Name));
            return foundMethod;
        }
        return foundMethod;
    }


The name of the method it find is: "System.Reflection.IReflectableType.GetTypeInfo" (this is not the `FullName`, but the normal `Name`.

But running this results in crashing the debugger.

3. Next I tried to simply run the original method whenever `RuntimeMethodForVirtual` returned NULL but I that resulted in a invalid program.
Daxaker commented 6 years ago

@jonkeda Problem with Assert.AreEqual caused by explicit implementation of IReflectableType.GetTypeInfo() in

class System.Reflection.IntrospectionExtensions
{
  public static TypeInfo GetTypeInfo(this Type type)
  {
    if (type == (Type) null)
      throw new ArgumentNullException(nameof (type));
    return ((IReflectableType) type).GetTypeInfo();
   }
}

this is the reason why public static MethodInfo GetRuntimeMethodForVirtual(Type type, MethodInfo methodInfo) returns null

there is a case to reproduce this issue

interface IMyTest
{
   int ExplicitMethod();
}

class MyTest : IMyTest
{
   int IMyTest.ExplicitMethod() => 1;
}

[TestMethod]
public void TestExplicit()
{
    Shim shim = Shim.Replace(() => Is.A<IMyTest>().ExplicitMethod()).With((IMyTest t) => 42);
    var r =  new MyTest();
    PoseContext.Isolate(() => { var foo = ((IMyTest)r).ExplicitMethod(); }, shim);
 }

I haven't solve this problem yet but if your know an elegant and easy way to get explicitly implemented members, i would like to hear about it

jonkeda commented 6 years ago

@Daxaker It isn't anywere near elegant, but I do have a working 'solution'.

  1. if the method isn't found then look if a method exists which name ends with the method to find. (brrr)

        public static MethodInfo GetRuntimeMethodForVirtual(Type type, MethodInfo methodInfo)
        {
            BindingFlags bindingFlags = BindingFlags.Instance | (methodInfo.IsPublic ? BindingFlags.Public : BindingFlags.NonPublic);
            Type[] types = methodInfo.GetParameters().Select(p => p.ParameterType).ToArray();
            var method = type.GetMethod(methodInfo.Name, bindingFlags, null, types, null);
            if (method == null)
            {
                MethodInfo[] methods = type.GetMethods(BindingFlags.Instance | BindingFlags.Public | BindingFlags.NonPublic);
                MethodInfo foundMethod = methods.FirstOrDefault(i => i.Name.EndsWith(methodInfo.Name));
                return foundMethod;
            }
            return method;
        }
  2. Expand the SignatureEquals to also look for interfaces.

    private static bool SignatureEquals(Shim shim, Type type, MethodBase method)
        {
            if (shim.Type == null || type == shim.Type)
                return $"{shim.Type}::{shim.Original.ToString()}" == $"{type}::{method.ToString()}";

            if (type.IsSubclassOf(shim.Type))
            {
                if ((shim.Original.IsAbstract || !shim.Original.IsVirtual)
                        || (shim.Original.IsVirtual && !method.IsOverride()))
                {
                    return $"{shim.Original.ToString()}" == $"{method.ToString()}";
                }
            }

            if (shim.Type.IsAssignableFrom(type))
            {
                if ((shim.Original.IsAbstract || !shim.Original.IsVirtual)
                    || (shim.Original.IsVirtual && !method.IsOverride()))
                {
                    return $"{shim.Original.ToString()}" == $"{method.ToString()}"
                        || method.Name.EndsWith(shim.Original.Name) ;
                }
            }

            return false;
        }

This code works for your testcase. But isn't very elegant or fail safe.

tonerdo commented 6 years ago

@Daxaker many thanks for this repro. Will take a look and work on a fix

Daxaker commented 6 years ago

@tonerdo Glad to be helpful

brian-pickens commented 5 years ago

My issue indeed had to do with calling xUnit Assert() method within the Isolate() function. Moving my assert methods out fixed the error for me.

StarkShang commented 4 years ago

Has this problem fixed in a new distribution? Or is it a normative way to act in isolate context and to assert outside?