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

Remove necessity of Moq on external methods #110

Open
HuwLittle opened this issue Jul 28, 2021 · 2 comments
Open

Remove necessity of Moq on external methods #110

HuwLittle opened this issue Jul 28, 2021 · 2 comments

Comments

@HuwLittle
Copy link

HuwLittle commented Jul 28, 2021

Hi guys,

I really like the idea of this package, but having a hard dependency on Moq is not something that's palatable for my team.

I'm happy with the internal workings of this package using Moq, but would like this package to enable the end-user to use whatever mocking framework they want when using this package.

It seems like this is partially supported already with the Silo ServiceProvider (allowing an instance or a Mock -

public Mock<T> AddServiceProbe<T>() where T : class
{
var mock = new Mock<T>();
_services.Add(typeof(T), mock.Object);
return mock;
}
public T AddService<T>(T instance)
{
_services.Add(typeof(T), instance);
return instance;
}

But for grain probes, it seems like I can't register an instance of the grain via the AddProbe method on the Silo without using Moq in some way -

public static void AddProbe<T>(this TestKitSilo silo, Func<IGrainIdentity, IMock<T>> factory)
where T : class, IGrain
{
if (silo == null)
{
throw new ArgumentNullException(nameof(silo));
}
silo.GrainFactory.AddProbe(factory);
}

I'm thinking of overloading the GrainProbeExtensions' AddProbe method to allow the user to pass in simply a factory method that returns the interface of the grain rather than a IMock, as it seems like we're only using the instance in the GrainFactory regardless.

private T GetProbe<T>(IGrainIdentity identity, string grainClassNamePrefix)
where T : IGrain
{
var key = GetKey(identity, typeof(T), grainClassNamePrefix);
if (_probes.TryGetValue(key, out var grain))
{
return (T)grain;
}
//If using strict grain probes, throw the exception
if (_options.StrictGrainProbes)
{
throw new Exception($"Probe {identity.IdentityString} does not exist for type {typeof(T).Name}. " +
"Ensure that it is added before the grain is tested.");
}
else
{
IMock<IGrain> mock;
if (_probeFactories.TryGetValue(typeof(T), out var factory))
{
mock = factory(identity);
}
else
{
mock = Activator.CreateInstance(typeof(Mock<>).MakeGenericType(typeof(T))) as IMock<IGrain>;
}
//Save the newly created grain for the next call
grain = mock?.Object;
_probes.Add(key, grain);
}
return (T)grain;
}

Please let me know what you think and if I'm missing something that would make this undoable.

Thanks,

Huw

@seniorquico
Copy link
Collaborator

The idea of replacing Moq with a pluggable factory had come up before... I can't find the GitHub issue, though. Maybe it was a discussion on Gitter? 🤔 Regardless...

I'd be happy to review an API proposal that refactors the mocking framework into a separate dependency. It might make sense to start by listing the public APIs that introduce the Moq dependency. Then for each, we should review alternatives. Should we use a mock factory? Should we switch to a fake implementation?

I think the ideal solution will introduce minimal/no breaking changes for existing consumers. Maybe we refactor the TestKit to have a core library that exposes generic/factory methods and the fake implementations. Then we can introduce separate mocking framework-specific libraries that build on top of the core that expose helper/strongly typed methods. If the signatures in a Moq-specific library didn't change, it could be a really easy transition for existing users.

For context: When we created the TestKit, it was our answer to unit testing using Moq to handle the complexity of introducing test doubles for framework components. Moq was a hard requirement and exposing it through the public API intentional. However, over time several mocks have been replaced with fake implementations. As you point out, there are some parts of the library that don't really depend on Moq anymore. It sounds reasonable to think this trend can continue and the TestKit could become more general purpose.

@HuwLittle
Copy link
Author

Yep completely agree on no breaking changes for existing users. And like this idea of having different adapters for different testing frameworks.

I'm keen on a fake implementation being passed in via an interface input, but will read through the code to see if this is applicable in all the scenarios where Moq is introduced.

I will start going through all externally exposed APIs, but will probably only change a couple of the main ones as I feel most of the value can be achieved with a handful of the signatures. This can then be expanded upon further in future.

Thanks,

Huw

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants