How to Write 3v1L, Untestable Code
Thursday, July 24, 2008
by Miško Hevery, Jonathan Wolter, Russ Ruffer, Brad Cross, and lots of other test infected Googlers
This guide lists principles that will help you write impossible to tests code. Or, avoiding these techniques will help you write code that can be tested.
Java Specific
This guide lists principles that will help you write impossible to tests code. Or, avoiding these techniques will help you write code that can be tested.
- Make Your Own Dependencies - Instantiate objects using new in the middle of methods, don't pass the object in. This is evil because whenever you new up an object inside a block of code and then use it, anyone that wants to test that code is also forced to use that concrete object you new'ed up. They can't "inject" a dummy, fake, or other mock in to simplify the behavior or make assertions about what you do to it.
- Heavy Duty Constructors - Make constructors that do lots of work in them. The more work you do in the constructor, the hard it is to create your object in a test fixture. And if your constructor can construct other things that are hard themselves to construct, that's even better! You want the transitive dependencies of every constructor to be enormous. Enormous is hard to get under test.
- Depend on Concrete Classes - Tie things down to concrete classes - avoid interfaces wherever possible. (They let people substitute the concrete classes you're using for their own classes which would implement the same contract in the interface or abstract class. Testing do-gooders might want to do this to get your code under a test harness - don't let them!)
- Conditional Slalom - Always, always, feel good when writing lengthy
if
branches andswitch
statements. These increase the number of possible execution paths that tests will need to cover when exercising the code under test. The higher the Cyclomatic complexity, the harder it is to test! When someone suggests to use polymorphism instead of conditionals, laugh at their thoughtfulness towards testing. Make the branching both deep and wide: if you're not consistently at least 5 conditionals deep, you're spoon feeding testable code to the TDD zealots. - Depend on Large Context Objects - Pass around ginormous context objects (or small ones with hard to construct contents). These will reduce the clarity of methods [myMethod(Context ctx) is less clear than myMethod(User user, Label label)]. For testing, the context objects will need to be created, populated, and passed around.
- Use Statics - Statics, statics everywhere! They put a great big crunch in testability. They can't be mocked, and are a smell that you've got a method without a home. OO zealots will say that a static method is a sign that one of the parameters should own the method. But you're being 3v1L!
- Use More Statics - Statics are a really powerful tool to bring TDD Infected engineers to their knees. Static methods can't be overridden in a subclass (sometimes subclassing a class and overriding methods is a technique for testing). When you use static methods, they can't be mocked using mocking libraries (nixing another trick up the pro-testing engineer's sleeve).
- Use Global Flags - Why call a method explicitly? Just like L Ron Hubbard, use "mind over matter" to set a flag in one part of your code, in order to cause an effect in a totally different part of your application (it's even more fun when you do it concurrently in different threads!). The testers will go crazy trying to figure out why all of a sudden a conditional that was evaluating true one minute is all of a sudden evaluating to false.
- Use Singletons Everywhere - Why pass in a dependency and make it obvious when you can use a singleton? It's hard to set up a test that requires singletons, and the TDDers will be in for a world of hurt when all their tests depend on each other's state.
- Be Defensive - They're out to Get Your Code! - Defensively assert about the state of parameters passed in methods, constructors, and mid-method. If someone can pass in a null, you've left your guard down. You see, there are testing freaks out there that like to instantiate your object, or call a method under test and pass in nulls! Be aggressive in preventing this: rule your code with an iron fist! (And remember: it's not paranoia if they really are out to get you.)
- Use Primitives Wherever Possible - Instead of using a "cookie object," pass in primitives and do all the parsing you need, every time you need a value. Primitives make people work harder by having to parse and massage them to get the data out -- where objects are mockable (gasp) and nullable, and encapsulate state (who'd want to do that?)
- Look for Everything You Need - By Looking for things you are asserting your objects dominance as the object which knows where everything is. This will make the life of tester hard, since he will have to mimic the environment so that your code can get a hold of what it needs. And don't be afraid of how many objects you need to reach out to to, the more the harder it will be for test to mock them all out in unisin. If you are an InvoiceTaxCalculator, feel free to do things like: invoiceTaxCalculator.getUser().getDbManager().getCaRateTables().getSalesTaxRate(). Cover your ears when some testing weenie tells you about Dependency Injection, Law of Demeter, or not looking for things.
- Use static initializes - Do as much work as possible when your class is loaded. Testing nuts will be so frustrated when they find out just loading your class causes nasty stuff like network or file access.
- Couple functional code directly to the external systems it depends on If your product uses external systems such as a databases, file systems or a network, make sure your business logic is coded to reference as many low level implementation details of these systems as possible. This will prevent others from using your code in ways you don't intend, (like small tests that run in 2 ms instead of 5 minutes).
- Mix Object Lifecycles - Have long lived objects reference short lived objects. This confuses people as the long lived object references the short lived object still after it's no longer valid or alive. This is especially insidious, both bad design, and evil, hard to test.
- Side Effects are the Way to Go Your best bet is to perform a large number of side effect producing operations in your methods. This is especially true for setters. The more non-obvious the side effects better. Peculiar and seemingly irrational side effects are particularly helpful for unit testing. To add another layer of sweet creamy goodness on top, you want to make it possible to initialize your objects in an invalid state, with uninitialized member fields. Once you have achieved this, be sure to make calls on the methods of the uninitialized fields as side effects from your setters in order to cause SEGV's or NPE's, depending on your language's vernacular. Why go to all this trouble? Clean, readable, and testable code that works, that's why! Side effect free functions are for intellectual weaklings that think a function name should give some kind of an indication of what the function does.
- Create Utility Classes and Functions/Methods - For instance, you have a String which is a URL you're passing around (obeying "Use Primitives Wherever Possible"). Create another class with static methods such as isValidUrl(String url). Don't let the OO police tell you to make that a method on a URL object. And if your static utility methods can call to external services as well, that's even better!
- Create Managers and Controllers - all over the place have these Managers and Controllers meddling in the responsibilities of other objects. Don't bother trying to pull that responsibility into other individual objects. Look at a SomeObjectManager class and you have no idea what it's going to do.
- Do Complicated Creation Work in Objects - Whenever someone suggests you to use a Factory to instantiate things, know that you are smarter than them. You're more intelligent than they must be, because your objects can have multiple responsibilities and be thousands of lines long.
- Greenlight if-branches and switch statements - Go ahead, don't feel dirty about nesting if-branches. It's "more readable" that way. OO cowboys will want to have a whole polymorphic soup of collaborating objects. Say no to the OO-ist. When you nest and branch conditionals, all you need to do is read the code from top to bottom. Like a great novel, one simple linear prose of code. With the OO-overboard paradigm, it's like a terrible choose-your-own-adventure kid's book. You're constantly flipping between classes and juggling patterns and so many more complex concepts. Just if-things out and you'll be fine.
- Utils, Utils, Utils! - Code smell? No way - code perfume! Litter about as many util and helper classes as you wish. These folks are helpful, and when you stick them off somewhere, someone else can use them too. That's code reuse, and good for everyone, right? Be forewarned, the OO-police will say that functionality belongs in some object, as that object's responsibility. Forget it, you're way to pragmatic to break things down like they want. You've got a product to ship after all!
- Use "Refactoring" whenever you need to get away with something - This is a word that Test-Driven and OO-goons like. So if you want to do something far reaching, involving new functionality, without tests, just tell them you're "Refactoring." It'll trick them every time. No matter that they think you need to have tests around everything before you can refactor, and that it should never add new functionality. Ignore their hubbub, and do things your own way!
Java Specific
- Final Methods - Use final classes and methods all the time. They can't be overridden for testing (-; But don't bother making fields final, or using value objects (without setters) - just let your objects' state be changed by anything and anyone. No sense in guaranteeing state, it'd make things too easy.
- Handcuff your users to Specific Types - Use
instanceof
as much as possible. This will make Mocking a pain, and show people that you're in control of the objects allowed.
C++ Specific
- Use non-virtual methods - Unless you need to override the method in a deep and scary inheritance hierarchy, just make the method non-virtual. If you make it virtual, a TDD zealot may mock your class! An even nicer trick is to keep your destructor non-virtual - then when the testing freaks try to subclass, whooooaoaaaaaa....
- Never use pure abstract classes - Depending on pure abstract classes is a sure-fire way to let the TDD crazies inject stubs and mocks into your code, making it testable.
- Macros are your friends - Always use
#ifdef PROD
and other compile-time switches to make sure the testies can't get at your really important blocks of code and test them. In fact, this code won't even run: until it gets to production!
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.