In the Spotlight - Jason Rudolph

As we’ve seen over the last several weeks, it’s remarkably easy for code to earn the badge of 100% test coverage without necessarily having a strong test suite. In each of those examples, the coverage analysis tool performed its task flawlessly: it reported exactly which portions of our code were executed as a result of running the tests. The all-green coverage report showed us that the tests indeed touched all of our code, but it was up to us to acknowledge that simply touching a line of code doesn’t mean that you’ve exercised and verified that line of code in a meaningful way. Some folks interpret this acknowledgement to mean that coverage analysis is meaningless, but that unfortunate conclusion overlooks the real benefit of a coverage report: it’s not about getting to 100% test coverage and assuming victory, it’s about highlighting any areas of our codebase that we’ve forgotten to test entirely.

As with any tool, to make effective use of coverage analysis, we need to understand its purpose, its capabilities, and its limitations. In all of the previous examples, we looked at code that was already in the coverage report. In other words, the coverage tool knew about this code and was able to watch the code and assess its coverage upon completion of the test suite. But if we’re using the coverage report to help us find untested code, how do we deal with code that the coverage tool might not be aware of in the first place?

Let’s start with a sample Rails app that represents the beginnings of an online store. The project currently contains the following files (as well as some others that I’ve omitted for the sake of brevity).

[store]$ tree
...
|-- app
|   |-- controllers
|   |   |-- application.rb
|   |   `-- products_controller.rb
|   |-- helpers
|   |   |-- application_helper.rb
|   |   `-- products_helper.rb
|   |-- models
|   |   `-- product.rb
|   `-- views
|       |-- layouts
|       |   `-- products.html.erb
|       `-- products
|           |-- edit.html.erb
|           |-- index.html.erb
|           |-- new.html.erb
|           `-- show.html.erb
|-- config
|   |-- boot.rb
|   |-- database.yml
|   |-- environment.rb
|   |-- environments
|   |   ...
|   |-- initializers
|   |   |-- inflections.rb
|   |   |-- mime_types.rb
|   |   `-- new_rails_defaults.rb
|   `-- routes.rb
|-- db
|   |-- migrate
|   |   `-- 20080810220638_create_products.rb
|   `-- schema.rb
...
|-- lib
|   |-- product_ftp_importer.rb
|   `-- tasks
|   |   |-- data_load.rake
...
|-- test
|   |-- fixtures
|   |   `-- products.yml
|   |-- functional
|   |   `-- products_controller_test.rb
|   |-- integration
|   |-- test_helper.rb
|   `-- unit
|       `-- product_test.rb
...

37 directories, 63 files

After installing the rails_rcov plugin, we can easily produce a coverage report to see where we currently stand.

According to the coverage report, we’re not aware of any code that isn’t touched by at least one test. But is that really the whole story? The number of test-related files sure accounts for a small proportion of the overall app. We can see that we have test/unit/product_test.rb and test/functional/products_controller_test.rb, but do those two files really encompass all the developer testing needed for this application?

Out of Sight, Out of Mind?

What about that mysterious file hanging out in the lib directory?

[store]$ tree
...
|-- lib
|   |-- product_ftp_importer.rb
...

Just judging by the name of the file, that sure sounds like functionality that deserves testing, but for some reason it’s not listed in the coverage report. And if we take another look at the test files listed above, there are no test files that obviously correlate to product_ftp_importer.rb. So, how is it that we have 100% coverage?

In order for code to show up in a coverage report, we need to instruct the coverage tool to assess that file. The approach for doing so tends to vary from tool to tool. With rcov, we first have to tell it which files constitute our test suite (i.e., the files we want rcov to run). But that alone is not sufficient; we also have to ensure that application files (i.e., the files whose test coverage we want to measure) get loaded as part of the test suite. Adding this require statement anywhere in our test suite is enough to shed some light on the elusive code in product_ftp_importer.rb.

It’s hard to feel good about 45 lines of untested FTP-processing voodoo, so how can we unearth this invisible code as soon as it tries to sneak its way into our app?

1. Ensure that your coverage task knows where to find your tests, always place new tests where the coverage task can find them, and use a consistent naming scheme so that a simple file-matching pattern can distinguish test files from non-test files.
2. If you’re using rcov, add a quick script that will crawl your project tree and require all application files. Adding individual require statements one-by-one is not a reasonable solution. If someone’s not going to write the test in the first place, they’re sure as heck not gonna take the time to tell the coverage report about that misdeed. So, walk the tree and require any Ruby file that you encounter in places where application code is likely to turn up. At the very least, in a Rails app you should include all subdirectories under app (being aggressive enough to catch any new directories that might get added there) and the lib directory.

(Not So) Scenic View Ahead

View templates have long been a favorite dumping ground for misplaced application logic. This problem can often go undetected, because view templates fly under the radar of the coverage report. Most developers know they should minimize the application logic included in the view, but when a deadline’s looming, the lure of throwing some code in the view “just this once” is often hard to resist. For example, what’s so wrong with having the view decide whether to display a particular product in the list?

Well, how exactly will we verify that the view is indeed displaying the right products and suppressing the others? We could manually test each scenario by visually inspecting the resulting UI, and that might be good enough for us to have confidence that the app is doing the right thing as of this moment. But code has a life of its own, and it will grow and change over time, and we want automated tests to make sure that this page continues to display the correct data even after those inevitable changes.

So we decide to write tests to verify that we’re displaying only the right products. And since this logic is inside our view template, we need to write tests that will render our view template and then dissect the resulting HTML to verify that it contains the products that should be present and that it does not contain the products that should not be present. But in order to render the HTML, we need to invoke some controller action. And because that lone if statement needs at least four different test cases to check the various conditions, we get the joy of doing all that setup and dissection at least four times. That’s a big enough pain that it quickly becomes very tempting to let this bit of logic remain untested, remain out of sight of the coverage report, and remain “good enough.”

We can do better than that. When something’s too hard to test, we should refactor it until it’s easy to test. [1]

We’re going to need this logic outside of the view anyway, so the sooner we get it into the model the better. Sure, the view will only display those products that are available for purchase, but we need that logic for server-side validation as well. Before we process an order, we need to make sure that we still have the product in stock. If we leave the logic in the view as is, then we’ll be forced to duplicate that logic elsewhere inside our order-processing code. There’s clearly no justification at all for leaving this logic in the view.

When we encapsulate this logic in the Product class itself, we can test that logic in isolation, without any dependencies on controllers, and without the need for fragile HTML-parsing to verify the result. Once we perform this refactoring, unit testing the #available_for_purchase? method becomes trivial, and we can refer to that method wherever necessary without unnecessary duplication.

Better still, if we know that we only want to display the products that are available for purchase, we can ensure that our controller provides only those products to the view in the first place. With this approach, our view then enjoys the pleasant simplicity of just displaying the list of products.

The coverage report isn’t going to alert us to business logic lurking in our view templates. It’s up to us to keep our views from becoming too smart for their own good, and it’s up to peer code reviews to keep us honest.

Raking for Buried Treasure

While it’s tempting to let our views acquire too much business logic, there’s usually an obvious place to move that logic once we realize the error of our ways. (In Rails, you’ll typically relocate that logic to a model class or to a helper, either of which are easily tested in isolation.) But what about the other parts of our application where untested code tends to hide out and germinate?

Perhaps we have some code that only needs to run at application start-up. In Rails, we’re talking about code in environment.rb or config/initializers. In Grails, BootStrap.groovy is home to this logic. In either case (or in most any other framework), we’re not likely to see those start-up “scripts” included in the coverage report, nor is there a natural and obvious place for testing any complex code that we may need to include in the start-up process. We’re used to testing models and controllers and helpers and mailers, but where does this start-up logic fit into the mix?

Data migration suffers from a similar problem. Rails migrations are great for creating and dropping tables, adding and removing columns, etc., but sometimes we need to do more than just alter the schema; sometimes we want to push data around as well. Schema transformations are essentially declarative code, and really don’t warrant anything beyond visual verification of the results. But when it comes time to migrate 10 million records from some legacy database into our hip new application, chances are we’re not just talking about simple declarations anymore. What’s the worst that could happen though? This code only has to run once. And who wants to write a bunch of tests for code that we’re only gonna run once and then throw away? And once again, there’s no obvious place for us to add tests for this kind of data conversion functionality in the first place. Surely a simple Rake task will suffice.

Indeed, a simple Rake task will suffice, but that’s certainly not what we’re looking at above. While we could write tests for this logic in its current state, doing so is unnecessarily difficult. We’d be restricted to solely black box tests. To test each individual decision point, we’re forced to also construct a new file holding the appropriate dataset, run the Rake task, and then inspect the state of the data in the database. For every decision point.

Scripts Can Be Classy Too

We shouldn’t have to also test the ability to read a file (i.e., line 7) just so that we can test the ability to populate a vendor based on a given vendor number (i.e., line 21). For sure, we want one good end-to-end test to verify that all the cogs are working together correctly. But if that’s our sole testing strategy, then we’ve made testing just painful enough that it probably won’t happen at all.

Whether we’re talking about hard-to-test code in start-up scripts, hard-to-test code in migration scripts, or hard-to-test code hiding out in the handful of other custom scripts that an application tends to accumulate over time, the answer’s the same in each case. Just because the coverage report doesn’t see this hidden code doesn’t mean that it’s not worth testing. And just because our framework-of-choice might not provide a convention for testing this logic, that doesn’t mean that we should just punt.

When something’s too hard to test, we should refactor it until it’s easy to test.

In the case of this Rake task, and in each of the cases discussed above, by simply moving the logic out of the script and into a proper class (or module), the testing strategy goes from clumsy at best to downright obvious. We no longer need to invoke the whole script in order to verify the particular unit of functionality that we want to test. Instead, we test that functionality in isolation, and allow the script to resume its trivial role of merely calling our well-tested class.

Use It Wisely

In order to make effective use of coverage analysis, it’s important for us to understand what a coverage report is telling us and what it’s incapable of telling us. Tools are imperfect, but we can adopt strategies to make sure we’re reaping the maximum benefit from the tools we choose to employ. With good naming conventions and an agreed-upon application structure, we can easily configure an intelligent solution that allows the coverage tool to automatically pick up any new source files that we want included in the report. With a commitment to testing all application logic - regardless of whether it’s needed in a model, a view, a script, etc. - we’ll extract the code that would otherwise be buried in a dark corner of our app. We’ll benefit from the ability to test it in isolation, and we’ll allow the coverage tool to assess that code, giving us a more realistic and complete view of our codebase.

Invisible code is hidden technical debt, but the sooner you expose it, the sooner you can start to pay it down.

Notes

[1] In past posts in this series, I’ve advocated test-driven development (TDD) as means for combatting the various testing anti-patterns. Invisible code is no exception. While this post is geared more toward uncovering invisible code so that we can give it the testing it deserves, developing test-first is the best bet for preventing invisible code in the first place.

This series is taken from the How To Fail With 100% Test Coverage talk. Check the schedule for a talk near you.

–

Thanks to Muness Alrubaie, Justin Gehtland, and Greg Vaughn for reading drafts of this post.

When you’re able to write a test, nay, a spec, that not only verifies your code’s functionality, but also clearly communicates its intent, you’ve got a real win on your hands. It’s a win when you’re first writing out your test cases as you’re TDD’ing your way to the solution. It’s a win to the person providing the peer review of your code. It’s a win for you (again) when you have to revisit (and relearn) the code six weeks from now. And it’s a win for the guy that has to touch this code yet again when you’re long gone. [1] But when you occasionally find yourself staring at a spec that looks exactly like the code under test, there’s surprisingly little win left to enjoy.

Consider, for example, the following Ruby Struct tasked with building a friendly string representation of a user’s name and e-mail address.

I ran across code similar to this example during a recent code review, and when I got to the corresponding test …

… I couldn’t help but feel like I was staring right back at the very code that was supposedly being tested.

[2]

This ugly mirror of the production code leaves much to be desired. Sure, thanks to the name of the test, we get the general idea that to_s will output some combination of the user’s name and e-mail address. But when we look at the assertion on line 6, we’re forced to mentally reverse engineer the code in order to see through to the underlying requirement. When it comes to quickly and clearly communicating the intent of the underlying code, this test falls far short of its potential. As it’s implemented above, we’re probably better off reviewing the production code itself than bothering to look at the test at all.

Of course, it doesn’t have to be this way, but it’s not unusual to stumble across this kind of test. In fact, it’s remarkably common to see this kind of test when the test is written after the production code is implemented. It’s as if once a developer has written the code to perform a task, the guts of the code can’t be unseen, and as a consequence, the tests often end up reflecting those inner workings instead of the desired end result.

No Mental Juggling Necessary

On the other hand, when we’re developing test-first, we start out with the end-user requirements in mind, and it’s easy to make sure our tests communicate those requirements.

When we improve the test to focus on the end result, we can look at the test and instantly see the requirements (and so can all the people that will live with our code long into the future). Instead of reflecting the ingredients, the test now reflects the end product. And this allows us to have greater confidence that our code is doing the right thing as well.

Whether it’s strings, dates, timestamps, or numeric calculations, any time we can assert on a literal value, our test will be better off because of it. [3] The less logic that’s in our assertion, the fewer chances we have for that logic to be wrong, and the less logic we have to dig through to grok what’s being tested in the first place.

Notes

[1] And if you’re writing a library or framework, the tests may be a win for the users (or potential users) of your code as well. My colleague Muness Alrubaie is often seen skipping right over the docs and heading straight for the tests when he wants to check out some new open source code. (After all, assuming they pass, the tests don’t lie.) And if there are no tests, or if the tests fail to cleanly express the underlying functionality, you can rest assured that he won’t be looking at that project for long.

[2] Image courtesy of Pablo Baslini (flickr.com/98621082@N00)

This series is taken from the How To Fail With 100% Test Coverage talk. Check the schedule for a talk near you.

–

Thanks to Greg Vaughn for reading drafts of this post.

• Mixins, meet Groovy. Groovy, say “Hello” to Mixins.

• I kindly request that all job postings henceforth forgo the usual mumbo jumbo, replacing it instead with Paul Graham’s one metric to rule them all: the income-to-boringness ratio.

• Snippets and scripts deserve first-class version control too.

• If you haven’t disabled “new mail” notifications by the time you finish reading this sentence, I’ll be forced to hold you personally responsible for the world’s economic misfortunes. Pull, Folks. Pull.

• Remember the good ol’ days of integrating Grails with a legacy database? You know, that long walk … uphill … in the snow … both ways. Times sure have changed.

As of 8:55 EDT, there’s no direct link to the store just yet, but you can “hack” your way in. Just search the iTunes store for the free iTunes Remote app named simply, “Remote”. Click Get App, wait for it to download, and voilà, there’s the App Store in the sidebar.

Sweet!

Last week we discussed the perils of overspecification, and while we saw that it’s clearly possible for a test suite to do too much, it’s far more common for it to do too little.

Green Architecture

Suppose we’re building an application for an online retailer, and they decide that they want to provide free shipping on all orders with a minimum price of $25.00. (Where do they come up with this stuff?!) Armed with these requirements, we set off to develop the logic to determine whether a given order qualifies for this new offer. (We’ll pick on some half-baked Ruby code for this example, but we could certainly extrapolate the same ideas to any language we might choose.)

It’s a straightforward request, so we’ll crank out a quick method to encapsulate the logic …

… and add a few simple tests to make sure we’re getting the desired results (all the while being careful to avoid any overspecification, of course).

And just like that, we have a passing test suite …

… and 100% code coverage …

… so there’s certainly the temptation to declare victory.

But what about the customer that just spent the last 20 minutes concocting the perfect order? You know, the one that’s exactly $25.00 and not a penny more. (Sure, he needs professional help, but do you really want to get on the wrong side of someone with that kind of determination and time on their hands?) If we’re looking for our test suite to tell us how the application should respond to a $25.00 order, we’re out of luck. This underspecification means that our test suite not only fails to communicate how the application should behave in this scenario, our test suite also fails to give us any confidence that the application will do the right thing (whatever that may be).

Edge Cases Matter

Since we’re calling it a “minimum” price, it sure sounds like a $25.00 order should qualify for free shipping, so we should add a test to specify that behavior.

And when we run this test, we should get some good insight into whether we’re likely to have that rather obsessive customer hunting us down in his copious free time.

Ouch! As is often the case, the code fails on a boundary condition. Of course, the problem (once identified) is easily fixed by replacing the incorrect operator (>) with the correct one (>=). It’s a trivial change, but under what circumstances would we identify this problem in the first place? Why not call it a day as soon as the first two tests are passing? We already had 100% test coverage [1], and we certainly don’t get any extra credit from our coverage analysis tool for adding this new test case. Geoffrey Wiseman sums up this situation nicely in his commentary on making effective use of coverage analysis:

Coverage [reports] are great at telling you when you don’t have enough tests. They’re terrible at telling you that you have enough, that they’re good enough, etc.

The standard test-driven development approach serves us well: write a test, watch it fail, and then write the code to make it pass. But there’s a next step as well: we need to ask which tests are still missing. We need to apply that critical thought. As soon as we finish writing the first two tests above, we should immediately start looking for the “abusive” tests (as Eric Sink lovingly refers to them) that we can write to make our code fail. Where are the edge cases? Where are the exception cases? What should happen if we pass in a negative value? A nil value? Our test suite should be specific in its demands of our code.

Use It Wisely

A green test suite and high code coverage only means that we’ve satisfied the functionality that’s currently specified by our tests. We can (and should) rely on the coverage report to spot the code that isn’t yet exercised by our test suite, but we can’t stop there.

In Waltzing with Bears, Tom DeMarco and Timothy Lister remind us that “while it’s possible to specify a product ambiguously, it is not possible to build a product ambiguously.” The applications we build are always doing something in the corner cases. In the exception cases. But are they doing the right thing. And without proper tests (i.e., executable specifications), how can we be sure?

Notes

[1] If you recall from our recent discussion on code coverage types, this is the part where you ask, “Exactly what kind of coverage are we talking about here?” Well, despite the fact that rcov only reports line coverage analysis, we can still deduce fairly quickly that this particular code has 100% line coverage, 100% branch coverage, and 100% path coverage.

This series is taken from the How To Fail With 100% Test Coverage talk. Check the schedule for a talk near you.

–

Thanks to Aaron Bedra and Greg Vaughn for reading drafts of this post.