Unit testing tricks: Look ma, no setters!
Here’s a neat trick if you want set an object in a specific state in a unit test, but you don’t want to violate encapsulation:
@Test
public void withdrawShouldReduceBalance() {
Account account = new Account() {{
super.balance = 100;
}};
account.withdraw(10);
assertEquals(90, account.getBalance());
}
@Test(expected=IllegalStateException.class)
public void overdraftShouldThrow() {
Account account = new Account() {{
super.balance = 5;
}};
account.withdraw(10);
}
This seemingly magic code lets me have a protected (but sadly not private) field Account#balance
. I can set this field in each individual test method.
It looks very magic, so it deserves a brief explanation:
- The first set of curlies create an anonymous subclass of Account. That is:
new Account() {}
. - The second set of curlies creates an initializer block, that is: a block of code that is added to the constructor.
- So in effect: Each test method creates a subclass of Account and modifies the account field in the constructor. This is why
Account#balance
must be protected instead of private.
This trick (and it is a trick) lets me set up the object under test to any state I want, without needing a special constructor and without breaking encapsulation by adding a setter. However, I’m required to make the fields I want to initialize protected. Additionally, each test method creates one or more new classes. This could potentially affect performance and/or memory use during compilation and/or running of tests. I’ve only used the trick at a small scale, so I don’t know if I’ll run into these issues in the future.
So, what do you think: How big of a “WTF” is this code?
Comments:
Sergio Bossa - Jun 15, 2009
Hi Johannes,
it’s a neat trick, but I still prefer to separate the domain object interface from its concrete implementation, and than add all required setters/getters to the latter.
By doing so, if you program by interfaces your domain code will only see methods from the interface (so no breaking encapsulation), while you will be able to still use setters/getters in your test code (nothing bad about that, tests are there for testing actual implementations).
Cheers,
Sergio B.
Fredrik Kalseth - Jun 14, 2009
Sure, there are always some cases which are “impossible” :) I don’t think the Account example qualifies though - but hey, it’s a simplistic blog post code example, and thus certainly not representative of a complex case. But since you ask, I would probably prefer acount.Deposit(100) as the setup in this case :)
Assuming the account balance is something that would be persisted aswell, you would anyways need to support the concept of initializing the balance of an Account, in which case that would be an ideal way for the test to set it up (f.ex by mocking said mechanism).
jhannes - Jun 14, 2009
In theory, this is a valid concern. In practice, you can’t really get around it. If you consider the example tests: How can you test this functionality without being tied to the fact that the account has a balance?
Stein Kåre Skytteren - Jun 14, 2009
Walk in the park!! At least if you like to write “prototype closure” code.
Fredrik Kalseth - Jun 14, 2009
Neat trick, but don’t you think doing this makes your tests too knowledgeable about how the SUT is implemented? This will make it more difficult to refactor the code down the line without breaking your tests, which will make your tests much less valuable, imo.
jhannes - Jun 14, 2009
This is indeed a tricky tradeoff. (However, I don’t think I’d set the fields public to support tests!)
We use Account.createAccountWithBalance() for similar cases. But when we found that each test needed to setup about 5 of 10 possible fields (different 5 for different tests), factory methods started to become confusing as well.
The tradeoff between a complex factory method and this trick is that the {{}}-trick is harder to understand the first time, but it’s usually easier to read when a number of fields are being initialized (as opposed to remembering the position of each factory method argument).
Then there’s the inner builder pattern….
jhannes - Jun 14, 2009
account.Deposit is a good option in this case. The trade-offs are that this function may not exist in your system (which is my situation) or that it may have side-effects which may then affect this test. If neither is the case, it’s probably a better approach.
Mocking is a tool I try to stay away from as it, too, can lead to tight coupling between your test and accidental behavior.
I think the {{}}-trick is only appropriate in very specific circumstances.
jhannes - Jun 15, 2009
Yes. But I thought that’d give me as much violation of encapsulation, but with more code. If the setters were to be used by other code, I would’ve gone that way, though.
[Filip van Laenen] - Jun 15, 2009
I think it would give less violation since it would only be a setter (a protected variable is basically a proetcted getter and a setter), and at the same time less confusing.
jhannes - Jun 14, 2009
The impact on the class under test is something that concerns me. In this particular case, we were debating whether to do this or use a inner builder class. They both felt slightly smelly, but at least this approach has the least code.
When the problem starts to scale, I might consider builders, structs, introducing setters or something else. But it seems like a lot of extra code to write for the size of the problem.
There’s a lot of tools in the “object initialization in test” toolbox. I’m still not thrilled with any of them.
Thomas Kjeldahl Nilsson - Jun 14, 2009
Neat solution!
However, I think (and this is just a personal preference) that encurring the resulting WTF moment for all subsequent maintainers of the code can be a steep price to pay just to be able to set a bunch of methods protected instead of public. I guess it depends how much of a OO purist you are vs what sort of people are going to maintain that code.
Related question: would you be violently opposed to adding a function Account.getTestAccountInstance([state representation]) and set the domain object state for tests in that way? Not elegant, adds ugly non-domain-logic to the domain objects API but more readable and less WTF factor - at least to my eyes.
[Kristian Nordal] - Jun 15, 2009
I like the “trick” with anonymous subclass + initialized block, and use it often. It’s great when initializing collections.
I’m not sure about making the fields protected though. I don’t like it when my tests affect/dictate visibility/accessibility.
[vidar] - Jun 14, 2009
Nice. I have used a similar technique creating dummy classes that extends the class under test and override methods instead of using mocking. Next time I will definitely consider your approach.
The “dummy class” technique is described in Robert Martin´s book Working Effectively with Legacy Code.
Johannes Brodwall - Jun 17, 2009
True. The reason they ended up as protected is that that’s what happens if you just do “Ctrl-1” in Eclipse. ;-)
This example requires a no-arg constructor, but if Account only had, say, a constructor “Account(String accountNumber)”, you could just do new Account("12345678901") {{
and the rest would be the same.
Thomas Kjeldahl Nilsson - Jun 14, 2009
Two small additional points:
- I’d also be a little thrown off if I was reading the domain object code and seeing a bunch of fields set to protected instead of private for no apparent reason. Yes, tests are supposed to function as documentation but I don’t always start reading unknown code from that direction.
- Position of arguments in the factory methods wouldn’t be an issue if the desired object state was passed as a single struct / DTO instead. Of course, at this point you start to accumulate some noticeable infrastructure in the domain logic just to support tests, but I’d still prefer one/few factory objects over a lot of ad hoc object initialization all over the test cases. Again, personal preference. :)
[Filip van Laenen] - Jun 15, 2009
Have you considered adding a protected setter instead?
[Baard H. Rehn Johansen] - Jun 16, 2009
If the test-code is in the same package as Account you could use package-private fields instead of protected fields. In your example, Account also requires a no-arg constructor.
jhannes - Jun 17, 2009
True. The reason they ended up as protected is that that’s what happens if you just do “Ctrl-1” in Eclipse. ;-)
This example requires a no-arg constructor, but if Account only had, say, a constructor “Account(String accountNumber)”, you could just do new Account("12345678901") {{
and the rest would be the same.