strangeioc / strangeioc

The IoC/Binding Framework for Unity3D and C#
1.6k stars 445 forks source link

Unit Testing a StrangeIoC Project: Model #64

Closed ghost closed 9 years ago

ghost commented 10 years ago

INTRODUCTION

I'm combining my StrangeIoc project from; http://www.rivellomultimediaconsulting.com/unity3d-mvcs-architectures-strangeioc/

With my testing template (from Unity's new nUnit setup); http://www.rivellomultimediaconsulting.com/unity3d-unit-testing-2/

I will put my findings in a new article this week for all to enjoy!

THE SETUP

The project compiles nicely. The first test I want to do is test the model. By coincidence the timing of the test occurs AFTER the root, context, first few commands all fire as part of the non-test project in the hierarchy. All good.

THE PROBLEM

A few simple test work, but the FIRST test I try (customModel.gameList setter) where that setter references an injected variable internally throws an exception;

System.NullReferenceException : Object reference not set to an instance of an object

I am testing the CustomModel as a 'unit'. Its not important to me that the signal 'do' anything or communicate with anything. I just don't want the error. The error, of course makes sense (see test below) since I'm creating a CustomModel instance freshly with the 'new' operator and obviously the injection does not happen that way.

THE SOLUTION?

But my question is what is the best setup to continue the process of testing?

  1. Reference the context (via a GO.Find() and GetComponent), and then test the real, live instance? -- probably possible, but I want to test a fresh instance of the model if possible.
  2. Reference the context (via a GO.Find() and GetComponent), and call something to get an instance of CustomModel FROM the context's binder?
  3. Something else?

Please let me know your thoughts and provide a code snippet for how to help me find the solution.

THE CODE SNIPPETS

PART OF MY CONTEXT MAPBINDINGS

injectionBinder.Bind<ICustomModel>().To<CustomModel>().ToSingleton();

HERE IS MY TEST SO FAR

//com.rmc.projects.strangeioc_template.mvcs.model.CustomModelTest.cs

[Test]
public void testPropertyGameList_SetValid  ()
{

    //1. FIND CONTEXT?

    //2. SETUP
    List<string> list_string = new List<string>();
    list_string.Add ("String0");

    //3. MAKE INSTANCE
    CustomModel customModel = new CustomModel ();

    //4. SETTER, THE ERROR HAPPENS IN HERE 
    //DUE TO REFERENCE TO AN INJECTED SIGNAL
    customModel.gameList = list_string;

    //5. TEST
    Assert.AreEqual (list_string, customModel.gameList);

}
srimarc commented 10 years ago

I have no experience with the new UTT (which is exciting news, to be sure!), but it uses nUnit just like Strange. So for your tests you can look at our test directories (strange/.tests) for examples. These tests include numerous examples of Injections, Context bindings and most everything else that happens in Strange (with some exceptions owing to Unity integration touchpoints).

But a unit test by definition shouldn't rely on the running of your Context. You should only write the required definitions inside the Test class or mocks. For our tests that involve injections, we typically instantiate an InjectionBinder, bind the required classes, then draw them using GetInstance(). If all your bindings are correct, the instance should appear fully-formed with all its injections. For your tests, it's arguable that you should be using injection at all. You're not trying to unit test Strange...we've done that for you. You're testing that your classes work, and that needn't/shouldn't rely on our library.

Your test is therefore best as it is. I.e., written just using setters...not relying on Strange to supply the dependencies.

ghost commented 10 years ago

Great, with your advice I solved it. Here is the complete test method with sections 1 and 3 are updated. It works great. When I get to testing the view I may post here again.

[Test]
public void testPropertyGameList_SetValid  ()
{

    //1. BIND THE CLASS TO BE TESTED
    //   *AND* ANY INJECTIONS WITHIN THE CLASS TO BE TESTED
    IInjectionBinder injectionBinder = new InjectionBinder ();
    injectionBinder.Bind<GameListUpdatedSignal>().ToSingleton();
    injectionBinder.Bind<ICustomModel>().To<CustomModel>().ToSingleton();

    //2. SETUP
    List<string> list_string = new List<string>();
    list_string.Add ("String0");

    //3. GET INSTANCE FROM INJECTOR
    CustomModel customModel = injectionBinder.GetInstance<ICustomModel>() as CustomModel;

    //4. SETTER
    customModel.gameList = list_string;

    //5. TEST
    Assert.AreEqual (list_string, customModel.gameList);

}
tenpn commented 10 years ago

This may be a straw man example, but can I ask what you're testing here? If you're testing the implementation of CustomModel, you can just new that without using the injector. If it's the context binds a CustomModel to satisfy ICustomModel, I'd prefer to mock out the injector, pass it to mapBindings(), and confirm the mapping was made. 

Either way, as a unit test this example is testing too much. It tests both the assumption that the context makes the correct binding, and that gamesList is correctly implemented in CustomModel. An ideal unit test should fail for only one assumption.

On Sun, Dec 22, 2013 at 10:39 AM, Samuel Asher Rivello notifications@github.com wrote:

I agree with all your points. Yes, with your advice I solved it. Here is the complete test method. Works great.

[Test]
public void testPropertyGameList_SetValid  ()
{
  //1. BIND THE CLASS TO BE TESTED
  //   *AND* ANY INJECTIONS WITHIN THE CLASS TO BE TESTED
  IInjectionBinder injectionBinder = new InjectionBinder ();
  injectionBinder.Bind<GameListUpdatedSignal>().ToSingleton();
  injectionBinder.Bind<ICustomModel>().To<CustomModel>().ToSingleton();

  //2. SETUP
  List<string> list_string = new List<string>();
  list_string.Add ("String0");

  //3. GET INSTANCE FROM INJECTOR
  CustomModel customModel = injectionBinder.GetInstance<ICustomModel>() as CustomModel;
  //4. SETTER
  customModel.gameList = list_string;
  //5. TEST
  Assert.AreEqual (list_string, customModel.gameList);

}

Reply to this email directly or view it on GitHub: https://github.com/thirdmotion/strangeioc/issues/64#issuecomment-31082649

tenpn commented 10 years ago

...of course this is all OT. :)

On Sun, Dec 22, 2013 at 5:40 PM, Andrew Fray andrew.fray@gmail.com wrote:

This may be a straw man example, but can I ask what you're testing here? If you're testing the implementation of CustomModel, you can just new that without using the injector. If it's the context binds a CustomModel to satisfy ICustomModel, I'd prefer to mock out the injector, pass it to mapBindings(), and confirm the mapping was made.  Either way, as a unit test this example is testing too much. It tests both the assumption that the context makes the correct binding, and that gamesList is correctly implemented in CustomModel. An ideal unit test should fail for only one assumption. On Sun, Dec 22, 2013 at 10:39 AM, Samuel Asher Rivello notifications@github.com wrote:

I agree with all your points. Yes, with your advice I solved it. Here is the complete test method. Works great.

[Test]
public void testPropertyGameList_SetValid  ()
{
 //1. BIND THE CLASS TO BE TESTED
 //   *AND* ANY INJECTIONS WITHIN THE CLASS TO BE TESTED
 IInjectionBinder injectionBinder = new InjectionBinder ();
 injectionBinder.Bind<GameListUpdatedSignal>().ToSingleton();
 injectionBinder.Bind<ICustomModel>().To<CustomModel>().ToSingleton();

 //2. SETUP
 List<string> list_string = new List<string>();
 list_string.Add ("String0");

 //3. GET INSTANCE FROM INJECTOR
 CustomModel customModel = injectionBinder.GetInstance<ICustomModel>() as CustomModel;
 //4. SETTER
 customModel.gameList = list_string;
 //5. TEST
 Assert.AreEqual (list_string, customModel.gameList);

}

Reply to this email directly or view it on GitHub: https://github.com/thirdmotion/strangeioc/issues/64#issuecomment-31082649

ghost commented 10 years ago

Without step # 1 above an error is thrown and an error thrown forces the related test to fail which prevents the process of testing. For me the binding is not the test, the binding is to allow me to test. If you have a complete example of how you test a model with a injection inside, please do provide it. I'd love to see your technique too.

srimarc commented 10 years ago

@srivello It seems as if we're possibly talking at cross purposes. What several of us have indicated is that Strange needn't be involved in the unit testing at all. Indeed, it shouldn't be involved if you can help it. So a Context isn't necessary. An injection binder isn't necessary. All your Model's/Command's/Service's dependencies are, for the purpose of the unit test, just setters, as if the [Inject] tags didn't exist.

So your test could look like this:

[Test]
public void testPropertyGameList_SetValid  ()
{
    //0. Setup (possibly this should occur in a TestSetup method)
    List<string> list_string = new List<string>();
    list_string.Add ("String0");

    //1. Instantiate the instance
    CustomModel model = new CustomModel();

    //2. Provide dependencies
    model.gameListUpdatedSignal = new GameListUpdatedSignal();

    //3. Call post-construct if applicable.
    model.PostConstruct();

    //4. Call test method
    customModel.gameList = list_string;

    //5. Assert outcomes
    Assert.AreEqual (list_string, customModel.gameList);
}

Notice how the only Strange-related piece involved in the test is the Signal...and that only on the presumption that it's important for the test. Testing your Commands should happen in much the same fashion: treat the injectables as setters (mocking where necessary), call Execute(), assert the outcomes.

In short: how the injections get provided to your instance are not the province of your tests. They may be coming via Injection, via setter, or via carrier pigeon. Strange is UT'd to assure that they do in fact arrive. If they don't, you file a bug report with me. If they do, all your test needs to know is that arrive they did...magically...and you can get on with testing the code you actually wrote.

HTH!

srimarc commented 10 years ago

As with #65 I believe this has been resolved? Please let me know if you have any objection to my closing this ticket.

ghost commented 10 years ago

Ok. Your code sample was very helpful.

It sounds like the bounds for what you recommend we developers do not test is any commands. That I can wrap my mind around. It sounds like it is recommended not to test any class which includes any injections. For me my model, view, controller, service tiers of code all may include injections. I have done that in PureMVC and Robotlegs and had test harnesses setup there. So that was the nature of my question. But it sounds like I'm looking to test things that aren't recommended. Thanks for the advice.

@srimarc. Sure thing. Close as desired.

srimarc commented 10 years ago

"It sounds like the bounds for what you recommend we developers do not test is any commands. That I can wrap my mind around. It sounds like it is recommended not to test any class which includes any injections."

I'm not saying anything of the sort. Absolutely you should test commands and things with injections. You just don't utilize the injector or the Context to do it. You supply the dependencies (including injections) manually, run your tests, then assert the results.

ghost commented 10 years ago

Ok. I misunderstood "Strange needn't be involved in the unit testing at all. Indeed, it shouldn't be involved if you can help it. " to mean we would not test anything that extends/references a Strange class such as Commands. Your most recent comment clear that up.

I also didn't think that a manually 'set' property would stop the injection error from happening. That is helpful.

Thanks.

srimarc commented 10 years ago

Awesome! Glad we've worked that out. :)

Since I'm working on some UTs today for StrangeRocks anyway, I thought it might be helpful to provide this explicit example. Here's the Command I'm testing:

//Executes when the user clears a level. Increments the game level.

using System;
using strange.extensions.command.impl;

namespace strange.examples.strangerocks.game
{
public class LevelEndCommand : Command
{
    [Inject]
    public IGameModel gameModel{ get; set; }

    [Inject]
    public UpdateLevelSignal updateLevelSignal{get;set;}

    public override void Execute ()
    {
        if (gameModel.levelInProgress)
        {
            gameModel.levelInProgress = false;
            gameModel.level++;
            updateLevelSignal.Dispatch (gameModel.level);
        }
    }
}
}

And here's the associated tests:

using System;
using NUnit.Framework;
using strange.examples.strangerocks.game;

namespace strange.examples.strangerocks.test
{
public class TestLevelEndCommand
{
    UpdateLevelSignal updateLevelSignal = new UpdateLevelSignal ();
    //Arguably could use a Mock...but GameModel is just a simple data holder anyway.
    GameModel gameModel = new GameModel ();

    [SetUp]
    public void Setup()
    {
        gameModel.level = 0;
    }

    [TearDown]
    public void Teardown()
    {
        gameModel.level = 0;
    }

    [Test]
    public void TestLevelEndCommandLevelInProgress()
    {
        gameModel.levelInProgress = true;
        LevelEndCommand command = new LevelEndCommand ();
        command.gameModel = gameModel;
        command.updateLevelSignal = updateLevelSignal;

        TestDelegate testDelegate = delegate()
        {
            command.Execute();
            Assert.AreEqual(1, gameModel.level);
            Assert.IsFalse(gameModel.levelInProgress);
        };

        updateLevelSignal.AddOnce (onLevelUpdatedLevelInProgress);
        Assert.DoesNotThrow (testDelegate);
    }

    [Test]
    public void TestLevelEndCommandLevelNotInProgress()
    {
        gameModel.levelInProgress = false;
        LevelEndCommand command = new LevelEndCommand ();
        command.gameModel = gameModel;
        command.updateLevelSignal = updateLevelSignal;

        TestDelegate testDelegate = delegate()
        {
            command.Execute();
            Assert.AreEqual(0, gameModel.level);
        };

        updateLevelSignal.AddOnce (onLevelUpdatedLevelNotInProgress);
        Assert.DoesNotThrow (testDelegate);
    }

    private void onLevelUpdatedLevelInProgress(int newLevelValue)
    {
        Assert.AreEqual(1, newLevelValue);
    }

    private void onLevelUpdatedLevelNotInProgress(int newLevelValue)
    {
        Assert.AreEqual(0, newLevelValue);
    }
}
}
ghost commented 10 years ago

Very helpful. Thanks.