6

I've performed the "Replace Method with Method Object" refactoring described by Beck.

Now, I have a class with a "run()" method and a bunch of member functions that decompose the computation into smaller units. How do I test those member functions?

My first idea is that my unit tests be basically copies of the "run()" method (with different initializations), but with assertions between each call to the member functions to check the state of the computation.

(I'm using Python and the unittest module.)

class Train: 

    def __init__(self, options, points): 
        self._options = options 
        self._points = points 
        # other initializations

    def run(self): 
        self._setup_mappings_dict() 
        self._setup_train_and_estimation_sets() 
        if self._options.estimate_method == 'per_class': 
            self._setup_priors() 
        self._estimate_all_mappings() 
        self._save_mappings() 

    def _estimate_all_mappings(): 
        # implementation, calls to methods in this class

    #other method definitions

I definitely have expectations about what the the states of the member attributes should be before and after calls to the the different methods as part of the implementation of the run() method. Should I be making assertions about these "private" attributes? I don't know how else to unittest these methods.

The other option is that I really shouldn't be testing these.

  • Some code or pseudo-code would be helpful in understanding your question. From what you've written, it sounds like you may benefit from the unittest.TestCase.setUp() method. http://docs.python.org/library/unittest.html#unittest.TestCase.setUp – msw May 06 '10 at 21:35
  • I've edited your question with the code you provided. Can you check the indentation is correct, and re-edit if necessary? – ire_and_curses May 06 '10 at 22:14

3 Answers3

10

I'll answer my own question. After a bit of reading and thinking, I believe I shouldn't be unit testing these private methods. I should just test the public interface. If the private methods that do the internal processing are important enough to test independently and are not just coincidences of the current implementation, then perhaps this is a sign that they should be refactored out into a separate class.

  • 1
    +1 for self answer and a pretty good one at that, if it isn't amenable to unit testing, perhaps it isn't a "unit". – msw May 08 '10 at 18:54
  • Strong disagree. There are plenty of reasons to hide methods as private that nevertheless hold significant functionality that is helpful to test independent of the public API. And Python does allow you to test protected/private methods - you just need to know about name mangling. – Nathaniel Ford Mar 24 '22 at 19:19
8

I like your answer, but I disagree.

The situation where you would use this design pattern is one where there is a fairly complex operation going on. As a result, being able to verify the individual components of such an operation, I would say, is highly desirable.

You then have the issue of dependancies on other resources (which may or may not be true in this case).

You need to be able to use some form of Inversion of Control in order to inject some form of mock to isolate the class.

Besides most mocking frameworks will provide you with accessors to get at the private members.

Nathaniel Ford
  • 20,545
  • 20
  • 91
  • 102
John Nicholas
  • 4,778
  • 4
  • 31
  • 50
  • Thank you for this alternative view. I wasn't aware of mocking back when I asked this question. –  Oct 03 '10 at 18:42
  • 1
    I have also changed my mind since to one more like yours. If you need to test private then it probably should be moved out to another class. However I think my idea still holds for things that should be internal - not private. – John Nicholas Sep 18 '12 at 15:02
  • I'd recommend against building your classes, especially in Python, using Inversion-of-Control unless there is a clear use case, such as a decision engine, that you're wrapping in a generic object. It can lead to very tangled code, and if the only reason you're doing that is for testing reasons there are more straightforward ways to go about it (testing private functions or monkey patching function calls). – Nathaniel Ford Mar 24 '22 at 19:29
0

There are two principles at play here. The first is that public methods should be the public API you want to expose. In this case, exposing run() is appropriate, whereas exposing estimate_all_mappings() is not, since you don't want anyone else calling that function.

The second is that a single function should only ever do one thing. In this case run() assembles the results of several other complex actions. estimate_all_mappings() is doing one of those complex actions. It, in turn, might be delegating to some other function estimate_map() that does a single estimation that estimate_all_mappings() aggregates.

Therefore, it is correct to have this sort of delegation of responsibilities. Then all that is required is to know how to test a private method.

The only reason to have another class is if there is some subset of the functionality that composes it's own behavioral unit. You wouldn't, for instance, create some class B that is only ever called/used by a class A, unless there was some unit of state that is easier to pass around as an object.

Nathaniel Ford
  • 20,545
  • 20
  • 91
  • 102