3

I have a class DataImport with only two public methods

public void fromStream(InputStream inputStream);
public ImportCommand getImportCommand();

When I test it I send some test data to fromStream method

dataImport.fromStream(some stream with data...)

Only way how to test the result of this method is to get the ImportCommand from getImportCommand and test if this object is in the state according to the input I provided.

But what if ImportCommand has only one method? 

public ImportResult execute();

I could test ImportResult (after invoking execute() which would involve some mocks and additional setup) because it has some state exposed. But this way I am testing ImportCommand and the DataImport class in one unit test which does not feel right to me.

But exposing inner state of the classes I am testing or accessing their private fields/methods also is not good thing.

Does this mean that my design is not ok (how should I change it) or is there some way how to solve this situation?

Related: TDD anti-patterns on SO

Improve this question
3
  • 1
    the way you describe getImportCommand sounds like a poor man Callable to me. I'd probably redesign the interface to single method like public Callable<ImportResult> fromStream(InputStream inputStream)
    – gnat
    Apr 26, 2014 at 8:35
  • As you can see in the answer i ended with slightly different design. But you've got good point. Two methods were not neccessary because the getCommand was dependent on the fromStream method and returned results from it.
    – DominikM
    Apr 28, 2014 at 7:34
  • I see, your new approach makes good sense
    – gnat
    Apr 28, 2014 at 7:54

3 Answers 3

Reset to default
3

Don't do any processing in the fromStream() method, just call another method - something like procesStreamResult() that get called whenever something comes in from the stream. This way, you can test procesStreamResult() by calling it with mock objects as arguments and also implement proper error handling and unit testing.

What I'm basically trying to say is: decouple the actual outside dependency (the stream, in your case) from the internal workings of your own code and only test the code you need to test.

Improve this answer
1
  • 1
    I think your answer suits my case the best. I think i am mixing responsibilities of parsing the stream and creating the command. The whole class now seems redundant and i ended up with parser and command which consumes result of a parser.
    – DominikM
    Apr 28, 2014 at 7:29
1

In unit tests you should always test behavior of your objects not the state. TDD is helpful when you design you APIs, in this case you already decided how the API will look and you have problems how to test it.

IMO DataImport is unnecessary abstraction. It can be one method somewhere else.

Improve this answer
0

Having a restriction on the interface such as,

... class DataImport with only two public methods

is valid and in no way implies that your design is not okay. In fact it would be bad to clutter the interface with methods intended for testing only.

Instead, you can use interface segregation to increase the testability of the class and present separate views.

The interface-segregation principle (ISP) states that no client should be forced to depend on methods it does not use. ISP splits interfaces which are very large into smaller and more specific ones so that clients will only have to know about the methods that are of interest to them. Such shrunken interfaces are also called role interfaces. ISP is intended to keep a system decoupled and thus easier to refactor, change, and redeploy. ISP is one of the five SOLID principles of Object-Oriented Design

Practically this means that you will create two interfaces, for example DataImportIf and DataImportIfInternal where the other interface will expose the relevant private methods or fields, and will be used either by the tester only or by classes which themselves constitute part of the DataImport state.

Improve this answer

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

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