SUT - Testing Internal Behaviour TDD

Question

I have a question about testing requirements.

Let's take example:

Class Article {

    public void deactive() {//some behaviour, which deactives article}

}

I have in my requirements, that Article can be deactived. I will implement it as part of my Article class.

Test will call deactive method, but what now? I need to check at the end of the test, if requirement was fulfilled so I need to implement method isDeactived.
And here is my question, if I really should do it? This method will be only used by my test case, nowhere else. So I am complicating my class interface, just to see assert, if is really deactived.

What is the right way to implement it?

Answer 1

It's generally considered ok to add test hooks to a class. It's better to have a slightly more cluttered interface and know your class works than it is to make it untestable. But there are some other solutions.

If you're ok making your method protected or package-private, you might be able to use something like Guava's @VisibleForTesting annotation. Languages other than Java might have other similar libraries.

You could also inherit from Article to get access to private fields.

class ArticleTest extends Article {
    @Test
    public void deactiveTest() {
        this.deactive();
        assertTrue(this.isDeactive);    
    }
}

This all assumes you have some field you're using to mark whether the object is active or not.

It might be the case that you're causing some side effects, like calling the database, and a couple of services to say you're deactivating that article. If so, you should mock the collaborators that you're using to make the side effects and verifying that you're calling them correctly.

For example (in java/mockito like pseudocode):

@Test
public void deactiveTest() {
    Article underTest = new Article(databaseMock, httpMock); //or use dependency injection framework of your choice...
    underTest.deactive();
    verify(databaseMock).calledOnce().withParams(expectedParams);
    verify(httpMock).calledOnce().withParams(expectedParams);
}

A final possibility, if that field affects the behavior of some other method or function, you could try (again in pseudocode):

article.deactive()
result = article.post() // maybe returns true if active, else false?
assertFalse(result)

This way, you're testing the resulting behavior, not just checking the internal state.

Answer 2

It sounds like you are writing a test along the lines of:

assertThatCallingDeactiveMarksArticleAsDeactivated

With an isDeactivated method, this test becomes trivial, however as you've said, your Article class doesn't contain this method. So, the question becomes should it have that method. The answer really depends on what it really means for the Article class to be deactive.

I would expect and active Article to behave differently in some way from a deactive Article. Otherwise, it seems like the state change doesn't have a reason / there is nothing to test.

To give a practical example, looking at it from the perspective of a client of the Article class. Something triggers the call to deactive. It may be something as simple as the user clicking on a Deactivate button/link on the user interface, which calls through to the Article class. After this call, I'd expect the user interface to reflect in some way that the Article was deactive (for example by greying out the button/link), but for it to do that, the UI needs to be able to read the state of the Article, which brings us back to the question how does it do that and/or why isn't the isDeactivated method needed by the current code?

Without knowing more about the implementation of the deactive method (does it simply set a flag, or does it call out to other code in an observable way) and how the state change effects the behaviour of Article and it clients it's hard to give a more concrete response.

Answer 3

Ideally you don't want to test the internals of a method or class, as this makes tests brittle and tightly coupled. If you refactor the production code, your test has a higher change of also needing to be refactored, thus going the benefit of the test. You want to try and test the classes behaviour as a whole (i.e what does calling deactivate do)

Check out Kent Beck's 4 rules for simple design (in priority order).

1) All Tests Pass

2) Expresses Intent

3) Eliminates Duplication

4) Fewest Elements

The last rule is that a system shouldn't be any larger than it needs to be, which is where your question falls. Given this is the least important element and that its better to 1) pass a test and 2) express intent, it would be acceptable (in my view) to simply add an isActive() method.

This also makes the class more useful and generic, as is you deactivate something, it seems logical to be able to verify its state.

Also as previously mentioned, there must be an effect of calling deactivate which in itself should be tested, so try and test this - It maybe an Integration Test is better placed, or you have to mock or stub another class.