Take the 2-minute tour ×
Programmers Stack Exchange is a question and answer site for professional programmers interested in conceptual questions about software development. It's 100% free, no registration required.

So today I had a talk with my teammate about unit testing. The whole thing started when he asked me "hey, where are the tests for that class, I see only one?". The whole class was a manager (or a service if you prefer to call it like that) and almost all the methods were simply delegating stuff to a DAO so it was similar to:

SomeClass getSomething(parameters) {
    return myDao.findSomethingBySomething(parameters);
}

A kind of boilerplate with no logic (or at least I do not consider such simple delegation as logic) but a useful boilerplate in most cases (layer separation etc.). And we had a rather lengthy discussion whether or not I should unit test it (I think that it is worth mentioning that I did fully unit test the DAO). His main arguments being that it was not TDD (obviously) and that someone might want to see the test to check what this method does (I do not know how it could be more obvious) or that in the future someone might want to change the implementation and add new (or more like "any") logic to it (in which case I guess someone should simply test that logic).

This made me think, though. Should we strive for the highest test coverage %? Or is it simply an art for art's sake then? I simply do not see any reason behind testing things like:

  • getters and setters (unless they actually have some logic in them)
  • "boilerplate" code

Obviously a test for such a method (with mocks) would take me less than a minute but I guess that is still time wasted and a millisecond longer for every CI.

Are there any rational/not "flammable" reasons to why one should test every single (or as many as he can) line of code?

share|improve this question
1  
I'm still making up my mind on this question, but here's a talk of someone who has decided the answer is "no". Ian Cooper: TDD, where did it all go wrong To summarize this great talk, you should test outside-in and test new behaviors not new methods. –  tieTYT Mar 27 at 17:21
    
This its really a great talk, a must see, a eye-opener talk for a lot of people, i love it. But i think that the answer its not "no". Its "yes, but indirectly". Ian cooper talks about hexagonal architecture and test features/behaviors mocking/stubbing the ports. In this case this ports are the DAO's and this "manager/service" its tested not with an individual unit test only for this class but with a "unit test" (unit in Ian Cooper definition that i agree completely with) that test some feature in your domain that use this manager/service. –  AlfredoCasado Mar 28 at 2:33

9 Answers 9

up vote 21 down vote accepted

I go by Kent Beck's rule of thumb:

Test everything that could possibly break.

Of course, that is subjective to some extent. To me, trivial getters/setters and one-liners like yours above usually aren't worth it. But then again, I spend most of my time writing unit tests for legacy code, only dreaming about a nice greenfield TDD project... On such projects, the rules are different. With legacy code, the main aim is to cover as much ground with as little effort as possible, so unit tests tend to be higher level and more complex, more like integration tests if one is pedantic about terminology. And when you are struggling to get overall code coverage up from 0%, or just managed to bump it over 25%, unit testing getters and setters is the least of your worries.

OTOH in a greenfield TDD project, it may be more matter-of-fact to write tests even for such methods. Especially as you have already written the test before you get the chance of starting to wonder "is this one line worth a dedicated test?". And at least these tests are trivial to write and fast to run, so it's not a big deal either way.

share|improve this answer
    
Ah I totally forgot that quote! Guess I will use it as my main argument because frankly - what can break here? Not really much. Only thing that can break is the method invocation and if that happens than it means something really bad happened. Thanks! –  Zenzen Jan 19 '12 at 21:35
    
@Zenzen: "what can break here? Not really much." - So it can break. Just a small typo. Or somebody adds some code. Or messes up the dependency. I really think that Beck would claim that your main example qualifies as breakable. Getters and setters, less so, although I have caught myself out in a copy/paste error, even then. The real question is, if it's too trivial to write a test, why does it even exist? –  pdr Jan 19 '12 at 21:44
    
1) I would not test the dependency because it would be an integration test. I would use a mock. 2a) if somebody adds some code then he adds actual logic WHICH should be tested then. 2b) Setters don't qualify then? Why? What if someone adds some code to them? Same thing. 3) Why is it there? Just to adhere to the common layered architecture of the application. Following that logic everything can break ;) –  Zenzen Jan 19 '12 at 21:48
    
The amount of time you spent thinking about it already you could have written the test. i say write the test, don't leave when not to write a test as a grey area, more broken windows will appear. –  kett_chup Jan 20 '12 at 0:11
1  
Blindly testing everything that could break doesn't make sense. There needs to be a strategy where high risk components get tested first. –  CodeART Mar 27 at 23:50

Here's good way to think about quality of your software:

  1. type checking is handling part of the problem.
  2. testing will handle the rest

For boilerplate and trivial functions, you can rely on type checking doing it's job, and for the rest, you do need test cases.

share|improve this answer

There are few types of unit testing:

  • State based. You act and then assert against state of the object. E.g. I make a deposit. I then check to see if balance has increased.
  • Return value based. You act and assert against return value.
  • Interaction based. You verify that your object called another object. This seems to be what you are doing in your example.

If you were to write your test first, than it would make more sense - as you would expect to call a data access layer. Test would fail initially. You would then write production code to make the test pass.

Ideally you should be testing logical code, but interactions (objects calling other objects) are equally important. In your case, I would

  • Check that I called data access layer with exact parameter that has been passed in.
  • Check that it's been called only once.
  • Check that I return exactly what's been given to me by data access layer. Otherwise I might as well return null.

Currently there is no logic there, but it won't always be the case.

However, if you are confident that there will be no logic in this method and it's likely to stay the same, than I would consider calling data access layer directly from consumer. I would do this only if the rest of the team are on the same page. You don't want to send a wrong message to the team by saying "Hey guys, it's fine to ignore domain layer, just call the data access layer directly".

I would also concentrate on testing other components if there was an integration test for this method. I'm yet to see a company with solid integration tests though.

Having said all this - I wouldn't blindly test everything. I would establish the hot spots (components with high complexity and high risk of breaking). I would then concentrate on these components. There is no point in having a codebase where 90% of codebase is quite straightforward and it's covered by unit tests, when remaining 10% represent core logic of the system and they are not covered by unit tests due to their complexity.

Finally, what's the benefit of testing this method? What are the implications if this doesn't work? Are they catastrophic? Don't strive to gain high code coverage. Code coverage should be a by product of good suite of unit tests. For example you may write one test that will walk the tree and give you 100% coverage of this method, or you can write three unit tests that will also give you 100% coverage. Difference is that by writing three tests you test edge cases, as opposed to just walk the tree.

share|improve this answer

In my opinion cyclomatic complexity is a parameter. If a method is not complex enough (like getters and setters). No unit testing is needed. McCabe's Cyclomatic Complexity level should be more than 1. Another word there should be minimum 1 block statement.

share|improve this answer
    
Remember some getters or setters have side effects (although it is discouraged and considered bad practice in most cases), so change in your source code may also affect it. –  Andrzej Bobak Oct 20 '12 at 21:41

When faced with a philosophical quesion, drop back to the driving requirements.

Is your goal to produce reasonably reliable software at a competitive cost?

Or is it to produce software of the highest possible reliability almost regardless of cost?

Up to a point, the two goals of quality and development speed/cost align: you spend less time writing tests than fixing defects.

But beyond that point, they don't. It's not that hard to get to, say, one reported bug per developer per month. Halving that to one per two months only releases a budget of perhaps a day or two, and that much extra testing probably won't halve your defect rate. So it is no longer a simple win/win; you have to justify it based on the defect cost to the customer.

This cost will vary (and, if you want to be evil, so will their ability to enforce those costs back to you, whether through the market or a lawsuit). You don't want to be evil, so you count those costs back in full; sometimes some tests still globally make the world poorer by their existence.

In short, if you try to blindly apply the same standards to an inhouse website as passenger airliner flight software, you will end up either out of business, or in jail.

share|improve this answer

It's a tricky question.

     Strictly speaking I would say that it is not necessary. You are better off writing BDD style unit and system level tests that insure business requirements function as intended in positive and negative scenarios.

     That said if your method is not covered by these test cases then you have to question why it exists in the first place and if it is needed, or if there are hidden requirements in the code that are not reflected in your documentation or user stories which should be encoded in a BDD style test case.

     Personally I like to keep coverage by line at around 85-95% and gate check-ins to mainline to insure existing unit test coverage per line hits this level for all code files and that no files are uncovered.

  Assuming best testing practices are being followed this gives plenty of coverage without forcing developers to waste time trying to figure out how to get additional coverage on hard to exercise code or trivial code simply for the sake of coverage.

share|improve this answer

Your answer on this depends on your philosophy (believe it is Chicago vs London? I'm sure someone will look it up). The jury is still out on this on the most time-effective approach (because, after all that's biggest driver of this- less time spent on fixes).

Some approaches say test only the public interface, others say test the order of each function call in every function. Plenty of holy wars have been fought. My advice is to try both approaches. Pick a unit of code and do it like X, and another like Y. After a few months of test and integration go back and see which one fit your needs better.

share|improve this answer

The problem its the question itself, you don't need to test all the "methdos" or all the "classes" you need to test all the features of your systems.

Its key thinking of terms of features/behaviors instead of thinking in terms of methods and classes. Of course a method its here for providing support for one or more features, at the end all your code its tested, at least all the code matters in your code base.

In your scenario, probably this "manager" class its redundant or unnecessary (like all the classes with a name that contains the word "manager"), or perhaps not, but seems like an implementation detail, probably this class do not deserve a unit test because this class has not any relevant business logic. Probably you need this class for some feature to work, the test for this feature covers this class, this way you can refactor this class and have test that verify that the thing that matters, your features, still works after the refactor.

Think in features/behaviors not in method classes, i cannot repeat this enough times.

share|improve this answer

This made me think, though. Should we strive for the highest test coverage %?

Yes, ideally 100%, but some things are not unit testable.

getters and setters (unless they actually have some logic in them)

Getters/Setters are stupid - just don't use them. Instead, put your member variable to public section.

"boilerplate" code

Get common code out, and unit test it. That should be as simple as that.

Are there any rational/not "flammable" reasons to why one should test every single (or as many as he can) line of code?

By not doing so, you might miss some very obvious bugs. Unit tests are like a safe net to catch certain kind of bugs, and you should use it as much as possible.

And the last thing : I am on a project where people didn't want to waste their time writing unit tests for some "simple code", but later they decided not to write at all. At the end, parts of the code turned into big ball of mud.

share|improve this answer
    
Well lets get one thing straight: I didn't mean I do not use TDD/write tests. Quite the opposite. I know that tests might find bug that I didn't think about, but what is there to test here? I just simply think that such method is one of the "not unit testable" ones. As Péter Török said (quoting Kent Beck) you should test stuff that can break. What could possibly break here? Not really much (there is only a simple delegation in this method). I CAN write a unit test but it will simply have a mock of the DAO and an assert, not much testing. As for getters/setters some frameworks require them. –  Zenzen Jan 19 '12 at 21:28
    
Also, since I didn't notice it "Get common code out, and unit test it. That should be as simple as that.". What do you mean by that? It is a service class (in a service layer between the GUI and the DAO), it is common to the whole app. Can't really make it more generic (since it accepts some parameters and calls a certain method in the DAO). The only reason it is there is to adhere to the layered architecture of the application so the GUI won't call the DAO directly. –  Zenzen Jan 19 '12 at 21:33
16  
-1 for "Getters/Setters are stupid - just don't use them. Instead, put your member variable to public section." - Very wrong. This has been discussed several times on SO. Using public fields everywhere is actually worse even than using getters and setters everywhere. –  Péter Török Jan 19 '12 at 21:38

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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