3
\$\begingroup\$

I am currently working on a C# WPF project following the MVVM design pattern. I am using Autofac and Prism as core frameworks in the design. For unit testing I am using Xunit and Moq. I am also using the Automock Moq extensions in unit testing to help ease the creation of my models.

For a prism event, I am to running a series of complicated verification steps and actions based on specific results. These verification and actions I have split up into separate classes to make the code clearer and easier to test.

The problem I am having is related to testing the viewmodel that I have subscribed to the event in. The tests related to the viewmodel is starting to take 20-30 lines of code to setup all of the mocks to be returning the correct conditions. Also to mock the extracted classes, my class constructor is becoming rather complex passing in a number functions to generate the business logic on demand.

Right now my constructors for my business logic only take one argument, but I know they will need to expand as add more features (trying to solve this issue early).

Thoughts I am having:

  1. I could take an IContainer (from Autofac) and resolve the the business logic functions inside of my classes instead of taking in functions to create the instances. I saw some tutorials using the function style I user hear and I thought it was slick until now.

  2. Use property parameters as well or instead of constructor parameters. These items are required which is why I like them more in the constructor, I also felt properties were optional. Also then I would have to define their requirement in my tests as well correct?

  3. Make my business logic implement interfaces of themselves. I have never really understood why you do this if you know there will never be another instance of that class. Is it really just to make the mocking of objects easier for testing?

  4. This is just a normal thing and I am being paranoid.

Here is my view model that I am trying to test:

public class KitIssueControllerViewModel : ViewModelBase
{
    private readonly IInComtekClient _incomtekClient;
    private readonly Func<string, ILotNumberNotification> _notificationGenerator;
    private readonly  Func<StockCodeBusinessLogic> _stockCodeBusinessLogic;
    private readonly Func<LotNumberVerificationLogic> _lotVerificationBlGenerator;
    internal IDxf Dxf;

    public KitIssueControllerViewModel(IEventAggregator eventAggregator,
        IInComtekClient incomtekClient,
        Func<string, ILotNumberNotification> notificationGenerator,
        Func<StockCodeBusinessLogic> stockCodeBlGenerator,
        Func<LotNumberVerificationLogic> lotVerificationBlGenerator) : 
        base(eventAggregator)
    {
        Ensure.ArgumentNotNull(incomtekClient, nameof(incomtekClient));
        Ensure.ArgumentNotNull(notificationGenerator, nameof(notificationGenerator));
        Ensure.ArgumentNotNull(stockCodeBlGenerator, nameof(stockCodeBlGenerator));
        Ensure.ArgumentNotNull(lotVerificationBlGenerator, nameof(lotVerificationBlGenerator));

        _incomtekClient = incomtekClient;
        _notificationGenerator = notificationGenerator;
        _stockCodeBusinessLogic = stockCodeBlGenerator;
        _lotVerificationBlGenerator = lotVerificationBlGenerator;

        EventAggregator.GetEvent<NewLayerSetEnabledEvent>()
                       .Subscribe(NewLayersetEnabledAction);
    }

    private void NewLayersetEnabledAction(IEnumerable<ILayer> layerEnumerable)
    {
        if (string.IsNullOrEmpty(workOrder)) return;

        var businessLogic = _stockCodeBusinessLogic();
        var stockCode = businessLogic.ExtractStockCodeFromLayer(Dxf, layerEnumerable);
        if (string.IsNullOrEmpty(stockCode)) return;

        var generatedNotification = _notificationGenerator(stockCode);
        generatedNotification.VerificationAction = LotNumberVerification;

        RaiseNotification(generatedNotification, LotNumberHandler);           
    }
}

Below are the constructor signatures for the extracted logic:

public LotNumberVerificationLogic(IInComtekClient inComtekClient)
public StockCodeBusinessLogic(IInComtekClient inComtekClient)

And an example of one of my unit tests that I am worried about:

public class NewLayerSetEnabledEventTests
{
    private void MockRepoSetup(AutoMock mock)
    {
        var lotNumberViewModel = new Mock<LotNumberNotificationViewModel>("stockCode");
        mock.Provide(lotNumberViewModel.Object);

        mock.CreateAndRegisterEvent<NewLayerSetEnabledEvent, IEnumerable<ILayer>>();
        mock.CreateAndRegisterEvent<NewNotificationEvent, IDialogNotification>();               
    }


    [Fact]
    public void OnNewLayerSetEnabledEventIfNewLayerSetShouldQueryForLotNumber()
    {
        using (var mock = AutoMock.GetLoose())
        {
            MockRepoSetup(mock);
            mock.RegisterRealEvent<NewLayerSetEnabledEvent, IEnumerable<ILayer>>(new NewLayerSetEnabledEvent());
            Func<string, ILotNumberNotification> notificationGenerator =
                (stockCode) => new Mock<ILotNumberNotification>().Object;

            var mockBl = new Mock<StockCodeBusinessLogic>(mock.Create<IInComtekClient>());
            mockBl.Setup(bl => bl.ExtractStockCodeFromLayer(It.IsAny<IDxf>(), It.IsAny<IEnumerable<ILayer>>())).Returns("test");
            mockBl.Setup(bl => bl.VerifyStockCodeExist("test")).Returns(Task.FromResult(true));
            Func<StockCodeBusinessLogic> blGen = () => mockBl.Object;

            mock.Mock<IDxf>()
                .Setup(ii => ii.GetDistinctTextOnLayers(It.IsAny<IEnumerable<ILayer>>()))
                .Returns(new List<string>());
            mock.Mock<IInComtekClient>()
                .Setup(ii => ii.GetUrl<InventoryQuery>(It.IsAny<string>()))
                .Returns(Task.FromResult(new InventoryQuery { StockItem = { new StockItem { StockCode = "Test" } } }));

            var viewModel = mock.Create<KitIssueControllerViewModel>(new NamedParameter("notificationGenerator", notificationGenerator), new NamedParameter("stockCodeBlGenerator", blGen));

            mock.Mock<NewNotificationEvent>().Setup(ii => ii.Publish(It.IsAny<ILotNumberNotification>()));

            viewModel.Dxf = mock.Mock<IDxf>().Object;

            var mockLayer = new Mock<ILayer>();
            mockLayer.Setup(ii => ii.Name).Returns("Test");
            var layerList = new List<ILayer> { mockLayer.Object };
            mock.Create<NewLayerSetEnabledEvent>().Publish(layerList);

            mock.Mock<NewNotificationEvent>().Verify(ii => ii.Publish(It.IsAny<ILotNumberNotification>()), Times.Once);
        }
    }
}

I also use a couple custom extension methods to ease some of the repetitive setup tasks I have to do:

public static class AutoMockExtensions
{
    public static void CreateAndRegisterEvent<T>(this AutoMock container) where T : PubSubEvent, new()
    {
        var mockEvent = new Mock<T>();
        mockEvent.Setup(ii => ii.Publish());
        container.Provide(mockEvent.Object);

        container.Mock<IEventAggregator>().Setup(ii => ii.GetEvent<T>()).Returns(container.Create<T>());
    }

    public static void RegisterRealEvent<T>(this AutoMock container, T eventObject) where T : PubSubEvent, new()
    {
        container.Provide(eventObject);
        container.Mock<IEventAggregator>().Setup(ii => ii.GetEvent<T>()).Returns(eventObject);
    }
}
\$\endgroup\$
  • \$\begingroup\$ Please provide a more specific title based on the code's purpose. \$\endgroup\$ – Jamal Sep 11 '16 at 21:59
  • \$\begingroup\$ Why you should not inject container itself (your first thought): stackoverflow.com/a/11390520/1386995 \$\endgroup\$ – Nikita B Sep 12 '16 at 14:19
  • \$\begingroup\$ Thanks for that link. I thought it was a terrible idea and glad I avoided it. \$\endgroup\$ – Cameron Hurst Sep 12 '16 at 15:54
1
\$\begingroup\$

I am going to re-use an old answer.

While the context is slightly different, it is basically the same question.

UnitTesting with Mocking and Dependency Injection using Ninject


As for the other main question:

re: lots of constructor arguments.

you are right, this is a code smell. a clear indicator of complexity and shows too much of the guts of the internals.

..unfortunately it is the smell of cooking with IOC. which is somewhat unavoidable.

that being said, it is an example of precisely why constructional design patterns are needed.

you have a complex object with a number of components. but you only need a single entity, preconfigured.

the solution is a combination of constructional patterns and configuration.

Take all of those constructor parameters, figure out which are instance-specific vs domain specific. then make a factory to create those objects. e.g:

var vmConfig = new ViewModelConfig
{
   NotificationGenerator = ...,
   EventAgreggator = ...,
}// in real terms this object could be added to the resolvable ioc container and injected into the factory. It is just a composite object


var factory = new ViewModelFactory(vmConfig);

var kitIssueConfig = {
    StockBusinessLogic = ...,
    LotVerificationBLGenerator = ...
}


var kitIssue = factory.createKitIssueViewModel(kitIssueConfig);

The above isn't perfect yb any means but it seperates concerns relaevant to the current VM vs relevant to all system VMs.

It makes new instances composible without a huge constructor collection by just swapping the config object,

and finally if the factory is backed by an interface contract you can inject the factory directly and the consuming classes are only directly dependent on a factory NOT all the sub elements. AND that factory can be mocked, as can each config object passed in to a concrete factory to test THOSE!

\$\endgroup\$
  • \$\begingroup\$ I have read over the question, and trying to draw the parallels. I agree that if A -> B ->C evaluates properly does not matter, but if the return of B dictates whether C or D is called, should that not be tested? \$\endgroup\$ – Cameron Hurst Sep 12 '16 at 15:03
  • \$\begingroup\$ but it IS tested. if B returning true = C, and false = D. that test is complete. now test the wrapper, when handed true, does it call C. two separate entities with separate tests. in this instance that wrapper is prism. you didn't write it. so: The other lesson from above is don't test what you can't control. if your components are all tested and an error still occurs, write a sanity check that checks the library/framework, there is no need to pre-test external frameworks. if they couldn't be trusted you wouldn't use them. \$\endgroup\$ – apieceoffruit Sep 12 '16 at 15:13
  • \$\begingroup\$ ok I feel there is some communicate issue. That is what I am doing when I said C and D i meant inside of my event handler function. I am assuming it is called correctly and just making sure the correct code is called based on return results determined inside of the handler with the payload passed by the event. Currently the only way i can access the handler is by calling an event, are you saying that I should make the member access internal so I don't test that when the event is fired it run this code and that the code just works? \$\endgroup\$ – Cameron Hurst Sep 12 '16 at 15:50
  • \$\begingroup\$ yes exactly. lets move away from prism for one second. the actual thing you are testing is indeed an "event". how the event fires is immaterial. just like the way mvvm uses relaycommands to decouple events so too should you and your logic. a class somewhere should have a DoTheThingOnEventFire() which the prism logic fires. something you can test in its entirety. something fully encapsulated and mockable. the answer lies in the sentence "the only way i can access that logic" ... then move the logic. \$\endgroup\$ – apieceoffruit Sep 12 '16 at 15:54
  • \$\begingroup\$ alright, makes sense. I can add an internal modifier to make the function testable without having to invoke a prism event. Wasn't on my list of problems, but I am happy to have a new way to look at that anyways. If you have any insight to any of the other questions your input is appreciated. thanks again. \$\endgroup\$ – Cameron Hurst Sep 12 '16 at 16:01

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.