Testing Untesting Code (or, Google Blows Goats)

scrottie on 2008-09-15T05:58:14

Best practices have a special place for me -- I hate them. Google, ditto. I've joined the "Google is evil" camp. So when a big steaming pile of Google Best Practices came to my attention, I knew I had to take a hose to it.

http://googletesting.blogspot.com/2008/07/how-to-write-3v1l-untestable-code.html but I've cut and paste all of the points in below so there's no reason to go there.

On second thought, I've written about Code::Splice and testing difficult code before, and you all know about testing in Perl. I was going to write rebuttals to each one but I realized pretty quickly that I'd just be talking to hear myself speak. So let's do this instead:

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.

Doesn't apply to Perl or other languages with "monkey patching".

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.

"The more work you do in the constructor, the hard it is"? The more spooge you write, the harder it is for me to read it. So, don't create factories because they're hard to test? Or just don't write big ones? So this whole point is redundant with "don't write huge methods"?

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!)

Alright, this much I agree with. It's bad to speak in terms of specific implementations of a class rather than an interface or some code name for the functionality you want. That's like asking for a Kleenex rather than a tissue and then actually refusing to accept an off-brand tissue for want of a real Kleenex. Not much code in Perl is strongly typed (but could be!), but Perl programmers kind of tackled this one anyway. There's a lot of code out there that does my $foo = shift; $foo->isa("Foo") or die, and there's plenty of code out there that does sub isa { 1 }. UNIVERSAL::isa() is a bit trickier but you can always monkey-patch in an inheritance. Continuity fakes HTTP::Request objects that are actually wrappers around FastCGI or HTTP::Server requests.

Conditional Slalom - Always, always, feel good when writing lengthy if branches and switch 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.

Alright, well, I basically agree with this one too. Using classes rather than if/else chains or switch lets you not scatter code all over for the various options. Each scenario has its own file -- a class. But a little evil with Code::Splice and you can dictate the code path the if/else chain takes (ie, which block runs).

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.

Wait, so, value objects are bad because you don't want to have to create them to test with? And the alternative is... passing in a whole bunch of parameters? So basically this is redundant with "too much crap lumped together in an object is bad". Thanks for that brilliant insight, Google.

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!

Oooh, you poor, poor creature. You have no idea what true evil is.

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).

So, "don't use methods that can't be overridden in a subclass because they can't be overridden in a subclass"? No arguing with that logic. There is snickering at it though. Anyway, monkey patching. Most Perl programmers who write SomeClass::method($blargh) usually have a good reason for doing so and if you want to replace it in testing, just localize the glob and assign in another function: local *SomeClass::method = sub { "now you die! I alone am best!" };.

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.

Okay, I thought I was going to disagree with you on every point, but then you started saying stuff like "water is wet!". So, basically, global variables are bad because they're variables that are global? Gotcha. You annoy me regardless of whether I agree or disagree with you, I'm finding.

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.

... in this case, a singleton just being a final class that's referenced implicitly, hence this point not only being redundant with everything that's completely obvious (when you address things globally, you aren't passing them in! and final classes can't be extended!), you're also being redundant with yourself and complaining about two things you've already complained about. Oh, heck, and now so I am I... being redundant and complaining... ahh, heck.

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.)

We have a bored test engineer here. I think I'm of the opinion that testing is good, but hiring people just to test other people's code is bad. Maybe it's your primary hat, but there's no healthy relationship possible there. I'm wondering about the job dynamics where a tester is complaining someone has placed asserts in their code protecting it from illegal values. Does the tester really think that has anything to do with them? Who is paranoid here -- the guy worried about the bad data or the guy who sees if(!$foo) and thinks it's defense against *him* and *his job*?

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?)

I'm pretty sure the programmer would have only parsed the thing once if he had known that the value would be needed parsed elsewhere. But, heck, I guess you could make an object out of each and every value, even if you use it just once, just in case someone else later might want it. Nevermind "don't code it, you aren't going to need it" and all 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.

"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. " Okay this actually makes it harder for me to make fun of you. Obviously this was a quick, unpolished stream of consciousness type rant, not a proper essay. This is a toughie. When there are complex and long chains of relationships between objects, there will be long, complex paths walked through them. Pass in as little as you need to, divide up the work of navigating the structure, and do whatever else. But in the end, in testing, you'll find that you have to mock these elaborate structures. Them's the beans.

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.

Oh, gesh, on about static stuff again. Could be interesting to see a testing project that required using a Safe compartment to keep obnoxious code from actively doing the obnoxious things it wants to as it loads.

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).

When I read a run-on sentence like that, my subconscious interprets it as something so unlikely it's absurd, like Weird Al's song Albuquerque. Well, *I* *like* sour kraut, so if it takes you forever to tell me about something you don't like, you better be damn sure I don't like it either or I'm going to be severely annoyed. More likely, I just won't bother to listen to you whine. (Hey, why are *you* still here?)

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.

I'm starting to see where this is going -- you aren't a good test engineer. If you were, you'd probably also be a good programmer and wouldn't be a test engineer. I don't think the programmer has any control over the life-cycle of the object. He's not going to keep files and connections open forever just so that your precious object never goes away. That isn't how it works in the real world.

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.

Yeah, I think Code Complete had something to say about functions with lots of side effects. 20 years ago.

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!

Wait, okay, so you have a URL object, and it has a method, isValidUrl()? So then you have an object which is sometimes invalid? Or does it always return 1? I'm confused. Your code doesn't even exist and it's confusing me.

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.

My guess is delegate ;)

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.

Now you're just repeating yourself. Again. Allow me to take a moment to not only taunt you but also everyone who thought that this was a good essay.

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.

You already did polymorphism vs if/switch chains. Kind of ironic. if spreads out scenarios all over the code (maybe 20 different places will do if(case == 1) { ... }). You're spreading out the same arguments all over your essay. Maybe you should aggregate your arguments into classes, or paragraphs, or something.

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!

Wait, did I botch my cut and paste and paste this article in twice? If so (I'm serious here) I apologize for not noticing. This thing is running together in my head.

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!

Yes, the programmers are out to trick you, the poor test engineer. And worse, they're making you, the poor test engineer, test it. It's so unfair.

-scott