dotnet / orleans

Cloud Native application framework for .NET
https://docs.microsoft.com/dotnet/orleans
MIT License
10.04k stars 2.02k forks source link

Mocking for Unit Tests #3086

Open dandago opened 7 years ago

dandago commented 7 years ago

This is a question, a suggestion to add more detail to the docs with regards to unit testing, and also a call to consider possible API changes that would facilitate mocking grain dependencies for unit tests.

(Context: I am talking about real unit tests, not test cluster.)

There are at least 3 things in grains which need to be mocked, lest they be problematic when it comes to unit tests (if you can think of others, please mention them):

  1. Getting a reference to another grain via GrainFactory
  2. Any of the methods that return the current grain's primary key
  3. Persistence methods (e.g. WriteStateAsync())

Below are suggestions on tackling the above. If you have a better idea, please share it!

  1. GrainFactory already uses the IGrainFactory interface. This can be set up in DI and passed into the grain's constructor. Thus it is easy to mock.
  2. Primary key methods are extension methods and cannot be mocked. The best workaround I could think of was to create a service that wraps those extension methods, and which could thus be set up in DI and thus mocked (sample code further below from memory).
  3. Still thinking about this.
public class GrainIdService : GrainIdService
{
    public string GetPrimaryKeyString(IAddressable grain)
    {
        return grain.GetPrimaryKeyString();
    }
}
DixonDs commented 7 years ago

@dandago

GrainFactory already uses the IGrainFactory interface. This can be set up in DI and passed into the grain's constructor. Thus it is easy to mock.

It already works like that - IGrainFactory is injected by the runtime.

centur commented 7 years ago

with regards to persistence methods - you can mock them on IStorage instance.

protected ProjectGrain(IGrainIdentity identity, IGrainRuntime runtime, ProjectState state, IStorage storage)
: base(identity, runtime, state, storage) { }

Agree on mocking primary key retrievals - we need them too

dandago commented 7 years ago

@centur the approach you suggested seems to require the use of a grain base class, which brings in a lot more concerns than it solves. Your constructor would need to accept all the parameters you specified.

I did notice that storage providers implement IStorageProvider, however since storage is configuration-based, it can't just be hooked into DI.

Are there any additional suggestions on how storage mocking can be solved?

centur commented 7 years ago

@dandago umm, aren't you inheriting from Orleans.Grain<TGrainState> ? This base constructor is part of Orleans abstract class: orleans/src/Orleans/Core/Grain.cs. Is it possible to build grain classes without inheriting from Grain or Grain<TGrainState> in latest versions ? Also keen to hear what concerns you have about base grain classes .

We are mocking it using AutoSubstitute container so it works for us just fine. Yes, you can't inject IStorageProvider in your runtime scenario from DI-container, but why would you need exactly that semantic ? For unit tests you're testing a unit of code and it's expected behaviour, not how it's exactly constructed... We haven't started using DI for all the grains (our runtime ctors are parameter-less ones), but besides aestetical side of the code - what is not right with the case when your testing ctor (internal) accepts 4 more arguments to fully isolate the unit of code you're testing ?

dandago commented 7 years ago

@centur I only meant that I am not using that constructor with 4 parameters. I am trying to figure out how to mock storage but that constructor brings in even more stuff that I don't know how to fill in, and shouldn't have to because it's not part of my production code.

centur commented 7 years ago

Testing something that is tightly coupled with the code out of your control is always dirty. But out of 4 parameters, 3 of them you will need for various aspects: State and IStorageProvider - to set the initial grain state and verify state modifications and writes\reads IGrainRuntime is what we are using (before modern DI was added. And we still are as we haven't migrated yet) to set mocks for IGrainFactory and IStreamFactory and provide your mocked calls for GetGrain<IOtherGrain> and mocked IOtherGrain.

It's not the cleanest possible code, really, we have bunch of test helpers, mockers and god objects. But it allowed us (with some help of handy libraries) to run unit tests of grains for 2 years of using Orleans. So if you want to test your grains - this would work. If you already can successfully test your grains and looking for some better practice - this is fair point to seek for different options.

PS: IGrainIdentity is presumably something that would allow you to mock Grain keys, but we never managed to work with it correctly, don't remember why.

chris-eaton commented 7 years ago

Hey... Hopefully adding to this rather than hijacking here but I am currently having issues regarding Timers. After registering a timer I would like to test that it got fired. I believe this to be currently impossible. It would be nice if this could be mocked or tested someway

DixonDs commented 7 years ago

So far, my experience is as follows: 1) Other grains calls - use autoinjected IGrainFactory in the grain code 2) Reminders - use autoinjected IReminderRegistry 3) Timers - use ITimerRegistry 4) Streams - use IStreamProviderManager 5) Storage - you don't have other choice as to use Grain<T> ctor accepting IStorage even though it is not used by the runtime (some refactoring needs to be done regarding this in Orleans core - for starters not to throw NullReferenceException, it is never a good thing) 6) Logging - GetLogger throws if there is no IGrainRuntime (I wonder why it does that instead of just returning a null logger, but it is how it is). We use our custom logging and hence a separate abstraction. For metrics, we extracted underlying code and call it instead of GetLogger().TrackMetric:

public static class MetricHelper
{
    public static void TrackMetric(string name, double value, IDictionary<string, string> properties = null)
    {
        foreach (var telemetryConsumer in LogManager.TelemetryConsumers.OfType<IMetricTelemetryConsumer>())
        {
            telemetryConsumer.TrackMetric(name, value, properties);
        }
    }
}
centur commented 7 years ago

By auto-injected you mean 1.4 injection into ctor ? Also, can you please add few examples for 3. and 4. Keen to learn how to get references to those interfaces in tests

DixonDs commented 7 years ago

@centur Yes, I mean that it is registered in DI by the runtime so you can accept it in ctor.

Keen to learn how to get references to those interfaces in tests

Not sure I've got that. You just mock those interfaces in your unit tests. Just be sure that the grain code uses those interfaces instead of calling inherited methods from Grain which rely on IGrainRuntime to be present.

jdom commented 7 years ago

Have you guys looked at #2856? it's supposed to help with a lot of these services that are really scoped to a particular grain activation. Since you are fighting these issues already, I would very much appreciate some feedback on the PR, and validate if it is helpful. It's been de-prioritized to after 1.5 since we didn't hear much feedback from others actually fighting against our current DI

centur commented 7 years ago

@DixonDs I thought that those interface instances are somehow hidden, found them on IGrainRuntime so it can be mocked along with IGrainRuntime. Does Orleans registers those interfaces (ITimerRegistry, IReminderRegistry) in DI and resolves\autoinjects them in ctor along with others or it's a special case and one have to mock them through IGrainRuntime.

@jdom haven't really checked it. Will take a look

DixonDs commented 7 years ago

@centur Yes, Orleans registers them in DI: https://github.com/dotnet/orleans/blob/5fc9368ccaa492c7640f0dc83a6f1b521e947e08/src/OrleansRuntime/Silo/Silo.cs#L229