Be Defensive - They're out to Get Your Code! I thought that writing defensive code was a good thing. I understand that this can be bad for testing, but can you specify a bit more when it's good do go defensive and when not.
While the content of this post was excellent, the disingenuous, sarcastic, say-the-opposite-of-what-I-mean style of it was a complete turn-off.
I hope you consider re-posting this information in a positive, "what-to-do" style, so that people don't have to parse through all of your negations just to get some good tips.
Defensive programong is usefull if you don't trust third parties. I.e. Public APIs. but for internal APIs (APIs for your own consumption defensive programong should be replaced with good test coverage. I.e.it works not because I am defensive but because it is well tested.
I feel like a crappy developer when I read this. There's a lot of bad things in my software such as overuse of singletons, overuse of utility classes, interfaces are rarely used, *manager-classes and so on. I could probably say "check" next to every one of these items.
On the other hand, almost everything (everything except for the user interface) is automatically tested after every new build and has been since I started to write the software 5 years ago. I add new test cases for every code I write and whenever someone reports a bug I add a automated test case for that as well before I fix it. I find it extremely easy to add new automated test cases.
So my conclusion is that you can code which is easily testable even if you follow these principles. If your software has other things, such as good APIs and other kinds of open interfaces, it may not be 'untestable'. :)
Great post. It reminded me of the 'How to get bad database performance' videos: http://tkyte.blogspot.com/2008/02/word-pathetic-never-sounded-so-good.html
Though I do have the same doubts as Thomas on being defensive. I see that it makes testing more difficult, but that doesn't mean it's without merit. The benefits of the 'Crash Early' principle are laid out pretty well in the book 'The Pragmatic Programmer'. Though this is can be a fault if taken to an extreme (like anything else).
I am also curious what you would define as a 'public api'. I'd say that any function that can be called by say 'Dave in the nyc office' should be considered public as you can't guarantee that Dave won't accidentally pass in some value that is out of range or his level of test coverage. Maybe Dave's team isn't that big on testing themselves.
I was reading through the list and was going like... uh huh... uh huh... yes this is precisely why we need more referential transparent programming (purely functional programming if you prefer). if any functional programmer is reading this, feel free to use this entry as an advertisement!
Testability Explorer is designed to catch many of these. I always have ideas on how to extend it to catch a lot more of these, but can't seem to find time.
At its core the TestabilityExplorer computes the smallest number of conditionals that a given method must execute from a test. (i.e. these conditionals can not be mocked out in any way) The higher the number the less testable to code.
Many of the 3v1L things listed here make it so that a method has very few if any places for seems therefore the number of un-mockable conditionals will be very high.
"Favor polymorphism over conditionals" - good advice if you never think through the OO alternatives. I've seen (and have at times myself written) a lot of code that bends over backwards to avoid the odd if-else or small switch in the name of "OO". Sometimes the if-else is just clearer and more honest.
"Create Managers and Controllers" - Good basic advice that can be taken too far. There are some places - "controllers" are great examples - that are a lot more like transaction scripts - a small number of lines of arbitrary statements - and just don't make sense to cover over with more abstraction - even a Command is often a waste of time. Gotta dispatch somewhere, just make it as clear as possible, whatever that means to your team.
Many of these are good ideas whether you're writing tests or not. The last real entrenched "best practice" still out there (even in more agile environments) that TDD'ers have to contend with is defensive coding. I wish the Pragmatic Programmers would come out and disown (in a post-come-to-Jesus/TDD kind of way) that advice - it's the only fault in that book.
Many of the things mentioned here such as static methods, final classes and making your own dependencies etc can actually be unit tested in Java. It’s just that the most common mock frameworks fail to do so. We have developed an open source framework to deal with many of these issues called PowerMock that is just an extension to EasyMock. What’s important to understand is that good design and testable design is not always the same thing.
If you're interested please have a look at http://www.powermock.org.
I understand the motivation behind PowerMock. but I wonder what coding habits the library promotes ? I work with the code deodorized with more than half of the 3v1L goodies and I happy original devs don't know PowerMock. I would neither understand their tests nor to have a chance to change the code when writing tests.
Sorry for the very late response, I just didn't realize that someone had replied here.
We don't promote the use of static methods or anything of that kind. What we mean is that there's a distinction between good design and testable code. It should be up to you as a developer to choose the design, not the technical limitation of a mocking framework.
I agree that putting PowerMock in the hands of novice programmers can do more harm than good in many cases. But I've experienced time and again that there are definitely use cases where PowerMock or similar frameworks really can help out. Especially if you're interacting with third-party frameworks or standard API's.
But you should be careful when using PowerMock if you're used to having a mock-framework guide you towards better design, there's no doubt about that.
This article has been extremely helpful to me. I've been having a hard time wrapping my brain around unit testing and test-driven design; I think I see now that what I consider to be good practices are bad practices from a testing standpoint. Fascinating!
One issue in particular that surprises me is the notion that it's bad to do "new" inside of an object's implementation. I write a lot of C++ code; I don't actually call new, but I certainly embed instances of other classes in a class I'm writing and then construct those instances. I do this because code re-use is good. Passing in objects that are used by the internal implementation of an object feels like exposing the implementation of the class in a bad way. Not to mention the fact that if you need to later change the class's implementation, you'll have to change the constructor and all the code calling that constructor in all places in the code...
I want to throw $.02 in support of what Igor is saying. I also agree with much of the points made in this post, but I will concede that after learning how to effectively apply a framework like EasyMock, many of these issues go away, b/c the code is testable using the framework. This isn't to promote crap code, but some of the things you end up doing in your design "just to make the code testable" can go away with a proper mock framework. EasyMock is *not* hard to learn, either, by the way.
Overall a very good article on how to write difficult to test code.
However, I disagree with some of your assertions
Make Your Own Dependencies At some point (at least in Java) you're going to have to call new in a method. This is often less of a dependency and more of a fact of life, not everything is caller specifable.
Be Defensive - They're out to Get Your Code!
Well, they are aren't they? Asserting that things that should definitely not be null are in fact, not null is a very good idea. If the tester is passing in a parameter as null that should not be, what are they testing for?
It's a rock and a hard place here, either you check (and the tester complains that they can't pass in null), or you don't (and the tester complains about an NPE or other bad behavior).
I'm surprised private isn't mentioned here. Before everyone starts forming a lynch mob, I am NOT trying to say that private should not be used! But from a pure testability perspective, private creates some challenges:
1. It can be difficult to observe the effects of running a piece of code. 2. It can be difficult to run a piece of code that you wish to test.
In C++, you can use friend to get around these issues, but that's generally looked down upon.
Another technique that I see people use to get around these issues is to add methods and comment them as "for test only". Like friend, this makes me feel dirty.
You could argue that these problems result from poor design. While I'd probably agree with you, that does not help if I have some existing code that I want to test. To fix that problem, I'd have to go back in time, and tell the original author to do it differently. That's just not possible. What I need is a way to make incremental changes to that code to make it more testable.
Be Defensive - They're out to Get Your Code!
ReplyDeleteI thought that writing defensive code was a good thing. I understand that
this can be bad for testing, but can you specify a bit more when it's good do go defensive and when not.
Otherwise I agree on most things, good post!
While the content of this post was excellent, the disingenuous, sarcastic, say-the-opposite-of-what-I-mean style of it was a complete turn-off.
ReplyDeleteI hope you consider re-posting this information in a positive, "what-to-do" style, so that people don't have to parse through all of your negations just to get some good tips.
Defensive programong is usefull if you don't trust third parties. I.e. Public APIs. but for internal APIs (APIs for your own consumption defensive programong should be replaced with good test coverage. I.e.it works not because I am defensive but because it is well tested.
ReplyDeleteYes, we will repost this list as a "Top 10 things to do to make your code testable with detailed explsanations"
ReplyDeleteI feel like a crappy developer when I read this. There's a lot of bad things in my software such as overuse of singletons, overuse of utility classes, interfaces are rarely used, *manager-classes and so on. I could probably say "check" next to every one of these items.
ReplyDeleteOn the other hand, almost everything (everything except for the user interface) is automatically tested after every new build and has been since I started to write the software 5 years ago. I add new test cases for every code I write and whenever someone reports a bug I add a automated test case for that as well before I fix it. I find it extremely easy to add new automated test cases.
So my conclusion is that you can code which is easily testable even if you follow these principles. If your software has other things, such as good APIs and other kinds of open interfaces, it may not be 'untestable'. :)
I'd add to this list of how to make your code untestable: Don't bother to ever read the Google Testing Blog :-)
ReplyDeleteI like the sarcasm, it's similar to the must-read 'How to Write Unmaintainable Code':
ReplyDeletehttp://mindprod.com/jgloss/unmain.html
Great post. It reminded me of the 'How to get bad database performance' videos:
ReplyDeletehttp://tkyte.blogspot.com/2008/02/word-pathetic-never-sounded-so-good.html
Though I do have the same doubts as Thomas on being defensive. I see that it makes testing more difficult, but that doesn't mean it's without merit. The benefits of the 'Crash Early' principle are laid out pretty well in the book 'The Pragmatic Programmer'. Though this is can be a fault if taken to an extreme (like anything else).
I am also curious what you would define as a 'public api'. I'd say that any function that can be called by say 'Dave in the nyc office' should be considered public as you can't guarantee that Dave won't accidentally pass in some value that is out of range or his level of test coverage. Maybe Dave's team isn't that big on testing themselves.
Writing testable code is harmful.
ReplyDelete@misko
ReplyDeleteWill the Testability explorer catch these lovely tips and tricks?
I was reading through the list and was going like... uh huh... uh huh... yes this is precisely why we need more referential transparent programming (purely functional programming if you prefer). if any functional programmer is reading this, feel free to use this entry as an advertisement!
ReplyDeleteYeah, the sarcasm was "great".
ReplyDelete@redsolo
ReplyDeleteTestability Explorer is designed to catch many of these. I always have ideas on how to extend it to catch a lot more of these, but can't seem to find time.
At its core the TestabilityExplorer computes the smallest number of conditionals that a given method must execute from a test. (i.e. these conditionals can not be mocked out in any way) The higher the number the less testable to code.
Many of the 3v1L things listed here make it so that a method has very few if any places for seems therefore the number of un-mockable conditionals will be very high.
"Code smell? No way - code perfume!"
ReplyDeleteAwesome! That one made me laugh. Tre bien.
-Kristian
Hmm, does it mean that google engineers don’t write their own unittests?...
ReplyDelete3v1L fails the spellchecker unit test
ReplyDeleteOn the whole great advice.
ReplyDeletesmall beefs:
"Favor polymorphism over conditionals" - good advice if you never think through the OO alternatives. I've seen (and have at times myself written) a lot of code that bends over backwards to avoid the odd if-else or small switch in the name of "OO". Sometimes the if-else is just clearer and more honest.
"Create Managers and Controllers" - Good basic advice that can be taken too far. There are some places - "controllers" are great examples - that are a lot more like transaction scripts - a small number of lines of arbitrary statements - and just don't make sense to cover over with more abstraction - even a Command is often a waste of time. Gotta dispatch somewhere, just make it as clear as possible, whatever that means to your team.
Many of these are good ideas whether you're writing tests or not. The last real entrenched "best practice" still out there (even in more agile environments) that TDD'ers have to contend with is defensive coding. I wish the Pragmatic Programmers would come out and disown (in a post-come-to-Jesus/TDD kind of way) that advice - it's the only fault in that book.
Indeed this is a good article, lot can be learnt from this. Thanks
ReplyDeleteMany of the things mentioned here such as static methods, final classes and making your own dependencies etc can actually be unit tested in Java. It’s just that the most common mock frameworks fail to do so. We have developed an open source framework to deal with many of these issues called PowerMock that is just an extension to EasyMock. What’s important to understand is that good design and testable design is not always the same thing.
ReplyDeleteIf you're interested please have a look at http://www.powermock.org.
Johan,
ReplyDeleteI understand the motivation behind PowerMock. but I wonder what coding habits the library promotes ?
I work with the code deodorized with more than half of the 3v1L goodies and I happy original devs don't know PowerMock. I would neither understand their tests nor to have a chance to change the code when writing tests.
Igor,
ReplyDeleteSorry for the very late response, I just didn't realize that someone had replied here.
We don't promote the use of static methods or anything of that kind. What we mean is that there's a distinction between good design and testable code. It should be up to you as a developer to choose the design, not the technical limitation of a mocking framework.
I agree that putting PowerMock in the hands of novice programmers can do more harm than good in many cases. But I've experienced time and again that there are definitely use cases where PowerMock or similar frameworks really can help out. Especially if you're interacting with third-party frameworks or standard API's.
But you should be careful when using PowerMock if you're used to having a mock-framework guide you towards better design, there's no doubt about that.
/Johan
This comment has been removed by a blog administrator.
ReplyDeleteThis comment has been removed by a blog administrator.
ReplyDeleteThis article has been extremely helpful to me. I've been having a hard time wrapping my brain around unit testing and test-driven design; I think I see now that what I consider to be good practices are bad practices from a testing standpoint. Fascinating!
ReplyDeleteOne issue in particular that surprises me is the notion that it's bad to do "new" inside of an object's implementation. I write a lot of C++ code; I don't actually call new, but I certainly embed instances of other classes in a class I'm writing and then construct those instances. I do this because code re-use is good. Passing in objects that are used by the internal implementation of an object feels like exposing the implementation of the class in a bad way. Not to mention the fact that if you need to later change the class's implementation, you'll have to change the constructor and all the code calling that constructor in all places in the code...
Perhaps I under-use factory functions?
I want to throw $.02 in support of what Igor is saying. I also agree with much of the points made in this post, but I will concede that after learning how to effectively apply a framework like EasyMock, many of these issues go away, b/c the code is testable using the framework. This isn't to promote crap code, but some of the things you end up doing in your design "just to make the code testable" can go away with a proper mock framework. EasyMock is *not* hard to learn, either, by the way.
ReplyDeleteA lot of this is specific to Java.
ReplyDeleteFor example, "avoid concrete classes use interfaces everywhere". In dynamic languages one doesn't need to play such games to mock things.
Same goes for static methods - much easier to mock in a dynamic language. Although the OO design arguments against them may still hold.
You missed the big one: Multi-threaded code!
ReplyDeleteOverall a very good article on how to write difficult to test code.
ReplyDeleteHowever, I disagree with some of your assertions
Make Your Own Dependencies
At some point (at least in Java) you're going to have to call new in a method. This is often less of a dependency and more of a fact of life, not everything is caller specifable.
Be Defensive - They're out to Get Your Code!
Well, they are aren't they? Asserting that things that should definitely not be null are in fact, not null is a very good idea. If the tester is passing in a parameter as null that should not be, what are they testing for?
It's a rock and a hard place here, either you check (and the tester complains that they can't pass in null), or you don't (and the tester complains about an NPE or other bad behavior).
Guess you don't do a great deal of Perl at Google then or you would have had this.
ReplyDelete1) Loads of classes with 'beige' methods such as 'run', 'process' and 'execute'
2) Write code like this...
#!/usr/bin/env perl
use URI;
use Number::Fraction;
sub beige {
my ($x, $y, $z) = @_;
$x->new($y)->$z();
}
print beige('URI', 'http://www.google.com', 'scheme');
print beige('Number::Fraction', '1/2', 'to_num');
Mwahaha
unsarcastic positive repost++
ReplyDeleteI'm surprised private isn't mentioned here. Before everyone starts forming a lynch mob, I am NOT trying to say that private should not be used! But from a pure testability perspective, private creates some challenges:
ReplyDelete1. It can be difficult to observe the effects of running a piece of code.
2. It can be difficult to run a piece of code that you wish to test.
In C++, you can use friend to get around these issues, but that's generally looked down upon.
Another technique that I see people use to get around these issues is to add methods and comment them as "for test only". Like friend, this makes me feel dirty.
You could argue that these problems result from poor design. While I'd probably agree with you, that does not help if I have some existing code that I want to test. To fix that problem, I'd have to go back in time, and tell the original author to do it differently. That's just not possible. What I need is a way to make incremental changes to that code to make it more testable.