Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Mocking framework architecture & discussion #4670

Closed
bclothier opened this issue Dec 23, 2018 · 1 comment
Closed

Mocking framework architecture & discussion #4670

bclothier opened this issue Dec 23, 2018 · 1 comment
Labels
discussion stale Issue is no longer relevant, or was fixed and forgotten. If closed but still relevant, open a new.

Comments

@bclothier
Copy link
Contributor

We now have a working proof of concept for mocking a COM type:
image

At the moment, this only work on any COM types and only the SetupProperty is implemented. This works by basically grabbing the ITypeInfo for the given COM type, convert it into Type (e.g. using Marshal.GetTypeForITypeInfo. With a Type, we can then build a Moq.Mock around it.

In theory, it would work for VBA classes, too, since we have the ITypeInfo; however there is an issue with the TypeLibConverter class, which will throw a null reference object deep within mscorlib (specifically somewhere inside this method). For now, I'm attributing it to the fact that VBA's TypeLib/TypeInfo may not be fully compatible, and we need to find a workaround for that.

Even so, it should not prevent us from being able to mock the rest of COM types, which would enable us to write unit tests that take those as parameters. For example, a function that take Excel.Range can be now directly unit tested using this technique.

However, the code required to set up a Moq Mock with a type specified at runtime is quite more complicated. Contrast the code:

Typical Mock setup with compile-time generics:

var mock = new Mock<Excel._Application>();
mock.Setup(x => x.Name).Returns("Stop mocking me!")

To the code required to handle a type that's specified at the runtime:

  1. First we must construct the lambda x => x.Name (Name is a user-defined parameter that's a string containing Name as the value):
var typeExpression = Expression.Parameter(_type, "x");
var memberInfo = _type.GetProperty(Name);

if (memberInfo == null)
{
    throw new ArgumentOutOfRangeException(Name, $"Not found on the interface '{_type.Name}'");
}

var memberAccessExpression = Expression.MakeMemberAccess(typeExpression, memberInfo);
var lambdaTypes = Expression.GetFuncType(_type, memberInfo.PropertyType);
var func = Expression.Lambda(lambdaTypes, memberAccessExpression, typeExpression);
  1. We must then use the above expression to then make the setup with the Setup(expression).Returns(value):
var mockType = _mock.GetType();
var asMemberInfo = mockType.GetMethod("As")?.MakeGenericMethod(_type);
            
var setupMemberInfos = mockType.GetMember("Setup");
var setupMemberInfo = ((MethodInfo)setupMemberInfos.Last()).MakeGenericMethod(targetType);

var returnsType = setupMemberInfo.ReturnType.GetInterface("IReturns`2");
var returnsMemberInfos = returnsType.GetMember("Returns");

//TODO: find a better way to get the correct method
var returnsMemberInfo = (MethodInfo) returnsMemberInfos.First();

var mockParameterExpression = Expression.Parameter(mockType, "mock");
var valueParameterExpression = Expression.Parameter(targetType, "value");
var asCallExpression = Expression.Call(mockParameterExpression, asMemberInfo);
var setupCallExpression = Expression.Call(asCallExpression, setupMemberInfo, expression);
var castReturnExpression = Expression.Convert(setupCallExpression, returnsType);
var returnsCallExpression = Expression.Call(castReturnExpression, returnsMemberInfo, valueParameterExpression);
var lambda = Expression.Lambda(returnsCallExpression, mockParameterExpression, valueParameterExpression);
lambda.Compile().DynamicInvoke(_mock, value);

We cannot just pass the expression directly to the Mock.Setup for following reasons:

  • The Setup is exposed only on the Mock<> generic, not on the Mock class.
  • To get Returns, the expression must be of Expression<Func <T, TResult>>, and we do not know either T or TResult at the compile time.
  • We could create a generic ComMock<T> but that won't help us because of the TResult not being known until runtime and we'd have to somehow dynamically call the closed generic method.

Thus for those reasons, both the lambda expressions and the Mock's setup code must be all built with expression trees, which requires us to know a bit more about the Moq's internals. It should be observed that if there's a breaking API change, the impact is amplified because of several statements to construct an equivalent expression.

OTOH, because it's literally a Moq wrapper, that means we can piggyback the Moq's investment in developing and testing the mocking; our concerns are limited to setting up the mock and translating the user's code into an equivalent expression tree.

Assuming this is deemed viable, I'm considering doing this piecemeal, perhaps starting out with only basic support for simple scenario and enabling the more complex scenarios that Moq can support (e.g. raising events for instance). We could mark this with the [Experimental] attribute and ship with only the simple Setup scenario enabled. What do you think?

NB: for some historical discussion: #1550

@bclothier
Copy link
Contributor Author

There is one more further consideration we need to put to properly implement the mocking framework. It all lies with the fact that the type equivalence is fundamentally broken. See this report for details. The expectation is that it will not be fixed anytime soon so we must work around it.

The tenative idea was to create a CachedTypeService class which would statically cache all the System.Type that got generated from calls to Type.GetTypeFromProgId or Marshal.TypeForITypeInfo methods (or any other method where a type may be created, actually). See, if the method is allowed to be invoked more than once, the System.Type it returns will be a new type, considered distinct from the first invocation. Because the Type.IsEquvialentTo returns false, we cannot safely cast an instance of first invocation type to second (or vice versa).

To avoid runtime errors due to invalid cast/ type mismatch, etc. we must ensure that there is only one type created for each COM interface. The plan was to use a static cache so that COM interfaces defined outside the VBA project would be always mapped to the same type that was created from a single invocation of the aforementioned methods.

However, this will not work for VBA's types. Once PR #4706 is merged, we will be able to create System.Type based on the user-defined code which are obviously more volatile. The solution would be to cache only for a single unit testing session and dispose of it.

We also have 2nd problem most COM coclasses are in fact composed of several interfaces. To ensure that the type casting succeeds when using the implemented interfaces, we must proactively create all System.Types for all the implemented interfaces. @comintern made the suggestion that we leverage ComProject to help with providing all the data. Instead of reflecting on a single COM coclass and getting its interface, we should reflect on the entire type library, which simplifies several operations such as finding the implementing coclass, finding the ProgID, and probably few more.

We may also need the changes introduced in the PR #4535 to enable access to entire VBA project and thus enabling mocking of those types.

To that end, we would need to be able to do the following:

  1. Convert a ProgID into a ITypeLib which can be then provided to a ComProject
  2. Cache the ComProject created from multiple opened projects named VBProject will break the fully-qualified method calls #1 or from a prior parsing run
  3. Provide a mechanism for creating System.Type based on the ITypeInfo returned by the members of ComProject
  4. Cache those created System.Type and ensure that future invocations re-use the same instance for euqality.
  5. Provide a mechanism for invalidating the cache only for the user-defined objects. It should not be necessary to invalidate the types for the referenced objects since those will not change at runtime like a VBA type could. However we may need to consider the changes in the references invalidating the cache.

So in end, we are looking at needing a caching service for ComProjects and generated System.Types separately, in addition to some querying services to support some metadata extraction such as discovering the ProgID.

Thoughts/suggestions welcome.

@bclothier bclothier added the stale Issue is no longer relevant, or was fixed and forgotten. If closed but still relevant, open a new. label Sep 1, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discussion stale Issue is no longer relevant, or was fixed and forgotten. If closed but still relevant, open a new.
Projects
None yet
Development

No branches or pull requests

2 participants