79

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?

5
  • 2
    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. Commented Mar 27, 2014 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. Commented Mar 28, 2014 at 2:33
  • see also: When is it appropriate to not unit test?
    – gnat
    Commented Nov 18, 2015 at 12:28
  • It will depend on your system to some extent, if you are developing a system with a moderate to hight level of safety certification you will need to cover all methods regardless of triviallity
    – jk.
    Commented Nov 18, 2015 at 15:35

10 Answers 10

63

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.

7
  • 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
    Commented Jan 19, 2012 at 21:35
  • 7
    @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
    Commented Jan 19, 2012 at 21:44
  • 1
    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
    Commented Jan 20, 2012 at 0:11
  • 1
    I'll add that my general experience is that testing getters and setters is somewhat valuable in the long-term, but low-priority. The reason why is because why it has "zero" chance of finding a bug now, you can't guarantee that another developer won't add something in three months ("just a simple if statement") that will have a chance of breaking. Having a unit test in place guards against that. At the same time, it isn't really overly high priority, because you aren't going to find anything soon that way.
    – dclements
    Commented Jan 20, 2012 at 4:00
  • 8
    Blindly testing everything that could break doesn't make sense. There needs to be a strategy where high risk components get tested first.
    – CodeART
    Commented Mar 27, 2014 at 23:50
15

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.

1
  • Why would you check that your DAL has only been called once? Commented Mar 16, 2015 at 20:43
14

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.

1
  • Of course type checking only works if you're using specific types in your code, and you're either working a compiled language or you otherwise ensure that a static analysis check is run frequently, e.g. as part of CI.
    – bdsl
    Commented Aug 10, 2018 at 20:59
9

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.

1
  • 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. Commented Oct 20, 2012 at 21:41
6

A resounding YES with TDD (and with a few exceptions)

Controversial alright, but I'd argue that anyone who answers 'no' to this question is missing a fundamental concept of TDD.

For me, the answer is a resounding yes if you follow TDD. If you aren't then no is a plausible answer.

The DDD in TDD

TDD is often quoted as having thee main benefits.

  • Defence
    • Ensuring the code may change but not its behaviour.
    • This allows the ever so important practice of refactoring.
    • You gain this TDD or not.
  • Design
    • You specify what something should do, how it should behaves before implementing it.
    • This often means more informed implementation decisions.
  • Documentation
    • The test suite should serve as the specification (requirements) documentation.
    • Using tests for such purpose mean that the documentation and implementation are always in consistent state - a change to one means a change to other. Compare with keeping requirements and design on separate word document.

Separate responsibility from implementation

As programmers, it is terribly tempting to think of attributes as something of significance and getters and setter as some sort of overhead.

But attributes are an implementation detail, while setters and getters are the contractual interface that actually make programs work.

It is far more important to spell that an object should:

Allow its clients to change its state

and

Allow its clients to query its state

then how this state is actually stored (for which an attribute is the most common, but not the only way).

A test such as

(The Painter class) should store the provided colour

is important for the documentation part of TDD.

The fact that the eventual implementation is trivial (attribute) and carries no defence benefit should be unknown to you when you write the test.

The lack of round-trip engineering...

One of the key problems in the system development world is the lack of round-trip engineering1 - the development process of a system is fragmented into disjointed sub-processes the artifacts of which (documentation, code) are often inconsistent.

1Brodie, Michael L. "John Mylopoulos: sewing seeds of conceptual modelling." Conceptual Modeling: Foundations and Applications. Springer Berlin Heidelberg, 2009. 1-9.

...and how TDD solves it

It is the documentation part of TDD that ensures that the specifications of the system and its code are always consistent.

Design first, implement later

Within TDD we write failing acceptance test first, only then write the code that let them pass.

Within the higher-level BDD, we write scenarios first, then make them pass.

Why should you exclude setters and getter?

In theory, it is perfectly possible within TDD for one person to write the test, and another one to implement the code that makes it pass.

So ask yourself:

Should the person writing the tests for a class mention getters and setter.

Since getters and setters are a public interface to a class, the answer is obviously yes, or there will be no way to set or query the state of an object.

Obviously, if you write the code first, the answer may not be so clearcut.

Exceptions

There are some obvious exceptions to this rule - functions that are clearcut implementation detail and clearly not part of the design of the system.

For instance, a the local method 'B()':

function A() {

    // B() will be called here    

    function B() {
        ...
    }
} 

Or the private function square() here:

class Something {
private:
    square() {...}
public:
    addAndSquare() {...}
    substractAndSquare() {...}
}

Or any other function that is not part of a public interface that needs spelling in the design of the system component.

2

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.

1

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.

0

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.

-1

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.

-4

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.

3
  • 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
    Commented Jan 19, 2012 at 21:28
  • 1
    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
    Commented Jan 19, 2012 at 21:33
  • 22
    -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. Commented Jan 19, 2012 at 21:38

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