How to test an anonymous inner class that calls a private method

Question

We have a bunch of classes that listen for events from the server and then respond to them. For example:

class EventManager {
    private Set<Event> cache = new HashSet<Event>();

    private EventListener eventListener = new EventListener() {
        void onEvent(Event e) {
          if (e instanceof MyEvent || e instanceof YourEvent) {
            handleEvent(e);
          }  
        }
    }

    public EventManager(ServerCommunication serverComm) {
        serverComm.addListener(eventListener);
    }

    private handleEvent(Event e) {
        // handle the event...
        // ...
        cache.add(cache);
        // ...
    }
}

Here's a made-up example of the kind of thing we are doing. Here are the problems I see:

1. I'd like to test handleEvent to make sure it's doing what it is supposed to but I can't because it's private.
2. I'd also like to check that something got added to the cache too but that also seems difficult since cache is a private member and I don't want to add a needless getter method.
3. I'd also like to test the code inside the anonymous class's onEvent method.

For now, what I did was move all logic from the anonymous class to the handleEvent method, and I made handleEvent package private (my unit test is in the same package). I'm not checking the contents of the cache although I want to.

Does anyone have any suggestion for a better design that is more testable?

Answer 1

Well, there are two approaches here: black box and white box.

Black box testing suggests you should only test the publicly visible changes. Does this method have any observable effect? (Some things don't - caches being an obvious example where they improve performance but may otherwise be invisible.) If so, test that. If not, test that it isn't having a negative effect - this may well just be a case of beefing up other tests.

White box testing suggests that maybe you could add a package-level method for the sake of testing, e.g.

Cache getCacheForTesting()

By putting "for testing" in the name, you're making it obvious to everyone that they shouldn't call this from production code. You could use an annotation to indicate the same thing, and perhaps even have some build rules to make sure that nothing from production does call such a method.

This ends up being more brittle - more tied to the implementation - but it does make it easier to test the code thoroughly, IMO. Personally I err on the side of white box testing for unit tests, whereas integration tests should definitely be more black box. Others are rather more dogmatic about only testing the public API.

Answer 2

I would probably extract a EventCache component. You can replace this for your test with an implementation that counts the cached events or records whatever is of interest.

I probably would not change the visibility of handleEvent. You could implement a ServerCommunication that just raises the event from the test case.

Answer 3

I assume your EventManager is a singleton, or you have access to the particular instance of the class you're testing.

1 - I suppose you can send events to your class. Your method is private, and nobody else can call it, then sending an event should be enough.

2 - You can access that through reflection, if you really need to. Your test would depend on a particular implementation.

3 - What would you like to test, actually? If you want to be sure that this method is called, you can replace the EventListener with another EventListener object through reflection (and eventually call the onEvent method of the first listener from your new listener). But your question seems to be more about code coverage than actual unit-testing.

Answer 4

Use this opportunity to break the code up into smaller more focussed (default access) classes. A test is just another client for the code.

Note that the anonymous inner class' onEvent method is actually accessible, so calling it should not be a problem.

Answer 5

Sometimes, when coming across private methods that I want to test... they are simply screaming to be public methods on another object.

If you believe that HandleEvent is worth testing in isolation (and not through onEvent processing), one approach would be to expose HandleEvent as a public method on new/different object.