October 9, 2008

Project Darkstar and Unit Tests

In my previous Project Snowman post, I mentioned that when developing games using Project Darkstar, there is a strong tendency to make gratuitous use of the AppContext class throughout all corners of your game server. Why is this a problem? Let me give you an example. Consider the following code:

public class Monster implements ManagedObject, Serializable {
  private int health;
  ...
  public void attack(int damage) {
    AppContext.getDataManager().markForUpdate(this);
    health -= damage;
  }
  ...
  public int getHealth() {
    return health;
  }
}

Now this is a pretty simple, stripped down class.  A Monster object has a health rating and this rating can be affected by inflicting damage via the attack(..) method.  So let's write a test for this method.

Since this is a unit test we don't want to fire up the whole Project Darkstar kernel; instead we want to just stub or mock out any dependencies this class may have. At first glance, though, it doesn't look like there are any, so this should be pretty easy right? Let's try:

public class TestMonster {
  @Test
  public void testAttack() {
    Monster testMonster = new Monster();
    int damage = 10;
    int finalHealth = testMonster.getHealth() - damage;

    testMonster.attack(damage);
    Assert.assertEquals(testMonster.getHealth(), finalHealth);
  }
}

All that we are doing here is instantiating a test Monster, and verifying that the attack method appropriately modifies the health of the Monster.  Does this test work though?  No.  The problem lies in the fact that there is a hidden dependency on the static AppContext method getDataManager().  When run outside of the context of a running Project Darkstar kernel, the behavior of this method is not defined (in fact it will throw a NullPointerException).  What's worse, since this method is static, it is impossible to mock its behavior to return a dummy implementation of a DataManager.  So what can we do?

For the Project Snowman effort, I devised a mechanism to wrap the AppContext static method calls in an interface called SnowmanAppContext. In fact, the strategy I used was very similar to the pattern described in this Google blog post which I just stumbled across today. This isolated the calls to the static methods in AppContext to within a single implementation of SnowmanAppContext. Unit testing became easy as I could simply replace the usages of SnowmanAppContext with a mocked out implementation.

Should this be done for all Project Darkstar games though? It seems like a built-in generalized solution should be possible. Fundamentally what is required for me to complete my unit test above is the ability to swap in a different context which is used by the AppContext underneath. Currently, the AppContext implementation is tied to the Project Darkstar kernel. If the kernel hasn't booted up, the behavior of AppContext is invalid. Since the AppContext is static, I can't stub it out. I am proposing a slight modification to the Project Darkstar API that would allow users to modify the behavior of AppContext to use whatever implementation that they would like. Further, this would completely decouple the Project Darkstar API classes from the current implementation. Here is my proposal:

First, add an additional interface to the core set of API classes:

public interface ManagerLocator {
  public ChannelManager getChannelManager();
  public DataManager getDataManager();
  public TaskManager getTaskManager();
  public  T getManager(Class type);
}

As you can see, this interface mirrors the set of methods that are exposed through the AppContext. The idea is that the AppContext will use a ManagerLocator underneath in order to retrieve each of the managers provided by the system. In order to set this, a static setter must also be added to the AppContext:

public final class AppContext {
  ...
  public static synchronized void setManagerLocator(ManagerLocator managerLocator) {
    ...
  }
}

Any implementation of the Project Darkstar API now must provide a class that implements the ManagerLocator interface, and set the context using the new AppContext setter method.  If these two minor changes are made, I can now complete the unit test that I originally set out to do using mock objects:

public class TestMonster {
  @Before
  public void swapContext() {
    ManagerLocator dummyLocator = EasyMock.createMock(ManagerLocator.class);
    DataManager dummyDataManager = EasyMock.createNiceMock(DataManager.class);

    EasyMock.expect(dummyLocator.getDataManager()).andReturn(dummyDataManager);
    AppContext.setManagerLocator(dummyLocator);
  }

  @Test
  public void testAttack() {
    Monster testMonster = new Monster();
    int damage = 10;
    int finalHealth = testMonster.getHealth() - damage;

    testMonster.attack(damage);
    Assert.assertEquals(testMonster.getHealth(), finalHealth);
  }
}

Now, this may not be quite how you'd set up your test with a real piece of code, but it demonstrates the point.  If you're not familiar with EasyMock, the code above essentially sets up a dummy DataManager that will be returned by the AppContext by putting it in a dummy ManagerLocator.  When the test is run, the call to AppContext.getDataManager().markForUpdate(this) will simply do nothing, and the test will pass.

I've coded up the necessary changes to the Project Darkstar server codebase for this API modification in a branch. If you're interested in having a look, the branch is located here. Feedback welcome!

3 comments :

  1. I've used easymock in my day job for quite awhile. I've worked around the AppContext static calls issue in my poking around with Darkstar by defining my classes with a method as follows:

    public DataManager obtainDataManager() {
    return AppContext.getDataManager();
    }

    I could then mock this method for the class I am testing with easymock's partial mocking like so

    DataManager mockedDataManager = createMock(DataManager.class);
    Method obtainDataManager = Foo.getDeclaredMethods("obtainDataManager");
    Foo foo = createMock(Foo.class, new Method[] {obtainDataManager});
    expect(foo.obtainDataManager()).andReturn(mockedDataManager);

    I like your solution, though, as it provides a universal approach that doesn't need to be implemented everywhere. I for one will welcome being able to more easily test objects built for darkstar.

    ReplyDelete
  2. Hi Lance,

    Thanks for your feedback! Yes I agree there are ways to work around this issue and I came up with a similar type of solution when contributing to the Project Snowman demo (https://project-snowman.dev.java.net). It's cumbersome, though, and forces you to put funny layers of indirection in your code for the sake of testing.

    If you've been following along on the Project Darkstar developer mailing lists, you may know that this change (actually a slight variation of it) has made it into the trunk (https://sgs-server.dev.java.net). It's also part of the latest unstable release 0.9.7.2 (http://download.java.net/maven/2/com/projectdarkstar/server/sgs-server-dist/0.9.7.2/).

    ReplyDelete
  3. Great Owen, I'll take a look at it. I just signed up for the mailing list yesterday, so I hadn't followed the implementation discussion.

    Thanks for your blog too, its a great resource and has helped me get to the meat of my darkstar investigations quickly.

    ReplyDelete