Should unit tests be written for getter and setters?

Question

Are we supposed to write tests for our getters and setters or is it overkill?

Kilburn

I don't think so. You should not write test case for getter/setter. – Quarles

I assume you mean Java? This is a particularly acute question for Java, much less so for more modern languages. – Disgruntle

@Disgruntle What modern languages don't have properties? Sure, languages like Java require them to be full method bodies, but that doesn't make it logical different from say C#. – Obsequent

@Claus: He didn't say properties, he said getters and setters. In java you write those manually, in other languages you get better support. – Disgruntle

One might ask why have getters and setters at all. – Delineator

Answer 1

I would say no.

@Will said you should aim for 100% code coverage, but in my opinion that's a dangerous distraction. You can write unit tests that have 100% coverage, and yet test absolutely nothing.

Unit tests are there to test the behaviour of your code, in an expressive and meaningful way, and getters/setters are only a means to an end. If you tests use the getters/setters to achieve their goal of testing the "real" functionality, then that's good enough.

If, on the other hand, your getters and setters do more than just get and set (i.e. they're properly complex methods), then yes, they should be tested. But don't write a unit test case just to test a getter or setters, that's a waste of time.

Disgruntle

Aiming for 100% code coverage is silly. You'll end up doing coverage of code that's not public exposed, and/or code with no complexity. Such things are best left for autogeneration, but even autogenerated tests are fairly pointless. It's better to shift the focus to important testing, like integration tests. – Obsequent

@Claus: I agree except for the bit about focussing on integration testing. Unit tests are critical to good design, and complement integration tests. – Disgruntle

I think that 100% code coverage when the entire test suite is run is a good aim. Property getters setters and other code is run as part of larger tests. The last pieces to be covered are probably error handling. And if the error handling isn't covered by unit tests it will never be covered. Do you really want to ship a product that contains code that has never ever been run? – Progressist

I would tend to disagree with this advice. Yes, getters and setters are simple, but they are also surprisingly easy to mess up. A few lines of simple tests and you know they are working and continue to work. – Furze

You might be testing unnecessary setters that you (or a tool) created just to have a 'rounded' POJO. You might be per instance using Gson.fromJson to "inflate" POJOS (no setters needed). In this case my choice is to delete the unused setters. – Epiphytotic

You should always delete unused code. Remember, folks, functionally testing your public interface is only one benefit of a unit test. The other benefit is regression testing--that is, testing the integrity of your public interface. You may not already own the code you just wrote, and those getters and setters are vulnerable to some future developer mucking with their implementations. That's basically the gist of why a developer might think it's useless while her boss thinks it's important. – Supporting

I personally also would write unit tests for properties, because we had several times the problem, that someone changed the property to a more complex one (e.g. moving an auto property to a property with a backing field or something else) and made a copy / pase mistake. If you have a unit test, it will immediatly fail - in our case, it failed when we delivered it to the user. Since that time I will always write tests even if it is boring ;-). – Nilotic

This answer begins with an "I would say no" while the context of it is "I would say it depends", poor lazy readers :) – Relational

Also on top of that, adding extra logic within getters and setters are bad practices. These logic should be extracted out to a service layer. So in short, don't add unit tests for getters and setters. – Verjuice

I think you should test them if you wrote custom code inside these methods or you can decide to test them if you set default values in the global values returned by getters. – Shark

Answer 2

Roy Osherove in his famous book 'The Art Of Unit Testing' says:

Properties (getters/setters in Java) are good examples of code that usually doesn’t contain any logic, and doesn’t require testing. But watch out: once you add any check inside the property, you’ll want to make sure that logic is being tested.

Lamanna

Considering how little time it to takes test those and the likelihood of behaviour getting added I don't see why not. I suspect if pressed he might reply "well, I suppose why not". – Impresario

Important early books often have bad advice. I've seen so many cases where people toss in getters and setters quickly and make mistakes because they are cutting and pasting or forget this. or this-> or something like that. They are easy to test and should certainly be tested. – Furze

Answer 3

A resounding YES with TDD

---

Note: This answer keeps getting upvotes, albeit potentially a bad advice. To understand why, have a look at its little sister below.

---

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 engineering¹ - the development process of a system is fragmented into disjointed sub-processes the artifacts of which (documentation, code) are often inconsistent.

_{¹Brodie, 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. However, the way to do this is not necessarily by testing each method in isolation, see my other answer for more.

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

Perilune

That's just one side of the coin. Think of the stuff you could have done instead of testing just for the sake of testing. As Kent Beck said, you get paid for working code, not for working tests. – Scoot

@GeorgiiOleinikov Have your read my other answer below? It pretty much in line with your view. – Perilune

Answer 4

tl;dr: Yes you should, and with OpenPojo it's trivial.

1. You should be doing some validation in your getters and setters so you should be testing that. For example, setMom(Person p) should not allow setting anyone younger than themselves as their mother.

2. Even if you aren't doing any of that now, odds are you will in the future, then this will be a good for regression analysis. If you want to allow setting mothers to null you should have a test for that should someone change that later on, this will reinforce your assumptions.

3. A common bug is void setFoo( Object foo ){ foo = foo; } where it should be void setFoo( Object foo ){ this.foo = foo; }. (In the first case the foo that is being written to is the parameter not the foo field on the object).

4. If you are returning an array or collection you should be testing whether or not the getter is going to be performing defensive copies of the data passed into the setter before returning.

5. Otherwise, if you have the most basic setters/getters then unit-testing them will add maybe about 10 minutes at most per-object, so what is the loss? If you add behaviour you already have a skeleton test and you get this regression testing for free. If you are using Java, you have no excuse since there is OpenPojo. There are an existing set of rules you can enable and then scan your entire project with them to make sure they are applied consistently within your code.

From their examples:

final Validator pojoValidator = ValidatorBuilder.create()
        .with(
            new NoPublicFieldsRule  (),
            new NoPrimitivesRule    (),
            new GetterMustExistRule (),
            new SetterMustExistRule ()
        )
        .with(
            new DefaultValuesNullTester (),
            new SetterTester            (),
            new GetterTester            ()
        )
        .build();


pojoValidator.validate(  PojoClassFactory.getPojoClasses( "net.initech.app", new FilterPackageInfo() )  );

Impresario

I know this is old-ish now, but: SHOULD you be doing validation in your getters and setters? I was under the impression that setMom should do what it says. If it's validating, then shouldn't it be validateAndSetMom? Or better, shouldn't the validation code exist ELSEWHERE than a simple object? What am I missing, here? – Geometric

Yes, you should always be validating your inputs. If you aren't, then why not just use a public variable? It becomes the same thing. The whole benefit of using setters versus variables is that it allows you to ensure your object is never invalid, such as age being a negative number. If you don't do this in the object you'll be doing it elsewhere (like a "service" god object) and that's not really OOP at that point. – Impresario

Answer 5

Yes, but not always in isolation

Allow me elaborate:

What is a unit test?

From Working effectively with legacy code¹:

The term unit test has a long history in software development. Common to most conceptions of unit tests is the idea that they are tests in isolation of individual components of software. What are components? The definition varies, but in unit testing, we are usually concerned with the most atomic behavioral units of a system. In procedural code, the units are often functions. In object oriented code, the units are classes.

Note that with OOP, where you find getters and setters, the unit is the class, not necessarily individual methods.

What is a good test?

All requirements and tests follow the form of Hoare logic:

{P} C {Q}

Where:

• {P} is the precondition (given)
• C is the trigger condition (when)
• {Q} is the postcondition (then)

Then comes the maxim:

Test behaviour, not implementation

This means that you shouldn't test how C achieves the post-condition, you should check that {Q} is the result of C.

When it comes to OOP, C is a class. So you shouldn't test internal effects, only external effects.

Why not test bean getters and setters in isolation

Getters and setters may involve some logic, but so long this logic does not have external effect - making them bean accessors²) a test will have to look inside the object and by that not only violate encapsulation but also test for implementation.

So you shouldn't test bean getters and setters in isolation. This is bad:

Describe 'LineItem class'

    Describe 'setVAT()'

        it 'should store the VAT rate'

            lineItem = new LineItem()
            lineItem.setVAT( 0.5 )
            expect( lineItem.vat ).toBe( 0.5 )

Although if setVAT would throw an exception, a corresponding test would be appropriate since now there is an external effect.

How should you test getters and setters?

There is virtually no point changing the internal state of an object if such change has no effect on the outside, even if such effect comes later on.

So a test for setters and getters should be concerned with the external effect of these methods, not the internal ones.

For example:

Describe 'LineItem class'

    Describe 'getGross()'

        it 'should return the net time the VAT'

            lineItem = new LineItem()
            lineItem.setNet( 100 )
            lineItem.setVAT( 0.5 )
            expect( lineItem.getGross() ).toBe( 150 )

You may think to yourself:

Wait a sec, we are testing getGross() here not setVAT().

But if setVAT() malfunction that test should fail all the same.

_{¹Feathers, M., 2004. Working effectively with legacy code. Prentice Hall Professional.}

_{²Martin, R.C., 2009. Clean code: a handbook of agile software craftsmanship. Pearson Education.}

Perilune

Thanks!! In my opinion, that's the best answer to this question :) You helped me with my unit tests with getters and setters which I was struggling with for quite some time. Thanks once again :) – Solstice

Answer 6

If the cyclomatic complexity of the getter and/or setter is 1 (which they usually are), then the answer is no, you shouldn't.

So unless you have a SLA that requires 100% code-coverage, don't bother, and focus on testing the important aspect of your software.

P.S. Remember to differentiate getters and setters, even in languages like C# where properties might seem like the same thing. The setter complexity can be higher than the getter, and thus validate a unit-test.

Shamekashameless

Although one should test for defensive copying on getters – Impresario

Answer 7

While there are justified reasons for Properties, there's a common Object Oriented Design belief that exposing member state via Properties is bad design. Robert Martin's article on the Open Closed Principle expands upon this by stating that Properties encourage coupling and therefore limit the ability to close a class from modification -- if you modify the property, all consumers of the class will need to change as well. He qualifies that exposing member variables isn't necessarily bad design, it might just be poor style. However, if properties are read-only, there's less chance of abuse and side-effects.

The best approach I can provide for unit testing (and this may seem odd) is to make as many properties as possible protected or internal. This will prevent coupling while discouraging writing silly tests for getters and setters.

There are obvious reasons where read/write Properties should be used, such as ViewModel properties that are bound to input fields, etc.

More practically, unit tests should drive functionality through public methods. If the code you're testing happens to use those properties, you get code-coverage for free. If it turns out that these properties never get highlighted by code-coverage there's a very strong possibility that:

1. You are missing tests that indirectly use the properties
2. The properties are unused

If you write tests for getters and setters, you get a false sense of coverage and will not be able to determine if the properties are actually used by functional behavior.

Supporter

Answer 8

A humorous, yet wise take: The Way of Testivus

"Write the test you can today"

Testing getters/setters may be overkill if you're an experienced tester and this is a small project. However, if you're just getting started learning how to unit test or these getters/setters may contain logic (like @ArtB's setMom() example) then it would be a good idea to write tests.

Medin

Answer 9

This has actually been a recent topic between my team and I. We shoot for 80% code coverage. My team argues that getters and setters are auto implemented, and the compiler is generating some basic code behind the scenes. In this case, given the code generated is non-intrusive, it doesn't really make sense to test code the compiler creates for you. We also had this discussion about async methods and in that case the compiler generates a whole bunch of code behind the scenes. This is a different case and something we DO test. Long answer short, bring it up with your team and decide for yourselves if its worth testing.

Also, if you are using the code coverage report like us, one thing you can do is add the [ExcludeFromCodeCoverage] attribute. Our solution has been to use this for models that just have properties using getters and setters, or on the property itself. That way it won't affect the total code coverage % when the code coverage report is run, assuming that is what you are using to calculate your code coverage percentages. Happy testing!

Destefano

Answer 10

I did a little analysis of the coverage achieved in the JUnit code itself.

One category of uncovered code is "too simple to test". This includes simple getters and setters, which the developers of JUnit do not test.

On the other hand, JUnit doesn't have any (non-deprecated) method longer than 3 lines that is not covered by any test.

Thorson

Answer 11

In my opinion code coverage is a good way to see if you missed any functionality that you should cover.

When you inspect the coverage manually by it's pretty coloring then it can be argued that plain getters and setters do not need to be tested (although I always do).

When you only check the code coverage percentage on your project, then a test coverage percentage like 80% is meaningless. You can test all the none logical parts and forget some crucial parts. In this case only 100% means that you have tested all you vital code (and all the non-logical code as well). As soon as it is 99.9% you know that have forgotten something.

By the way: Code coverage is the final check to see if you have fully (unit) tested a class. But 100% code coverage not necessarily means that you have actually tested all the functionality of the class. So unit tests should always be implemented following the logic of the class. In the end you run coverage to see if you forgot anything. When you did it right, you hit 100% the first time.

One more thing: While recently working at a large bank in The Netherlands I noticed that Sonar indicated 100% code coverage. However, I knew something was missing. Inspecting the code coverage percentages per file it indicated a file at a lower percentage. The whole code base percentage was that large that the one file did not make the percentage be displayed as 99.9%. So you might want to look out for this...

Nernst

Answer 12

I would say: YES Errors in getter/setter methods can silently sneak in and cause some ugly bugs.

I have written a lib to make this and some other tests easier. The only thing you have to write in your JUnit tests is this:

        assertTrue(executor.execute(*TheClassIWantToTest.class*, Arrays.asList( new DefensiveCopyingCheck(),
            new EmptyCollectionCheck(), new GetterIsSetterCheck(),
            new HashcodeAndEqualsCheck(), new PublicVariableCheck())));

-> https://github.com/Mixermachine/base-test

Legitimize

Answer 13

Yes, especially if the item to get is an object of a class subclassed from an abstract class. Your IDE may or may not alert you that a certain property has not been initialized.

And then some apparently unrelated test crashes with a NullPointerException and it takes you a while to figure out that a gettable property is actually not there to get in the first place.

Though that still wouldn't be anywhere as bad as discovering the problem in production.

It might be a good idea to make sure all your abstract classes have constructors. If not, the test of a getter might alert you to a problem there.

As for getters and setters of primitives, the question might be: Am I testing my program or am I testing the JVM or CLR? Generally speaking, the JVM does not need to be tested.

Laurentium