I recently got into a debate with a coworker about my requirement on an internal project that the code coverage cannot fall below 100%, or the build will break. He put up some very good points, but I’d like to spell out my thinking.
The project is an internal project we use for staffing, time entry and billing, so it is a production system, that is used for billing our clients, so I treat it as a very high priority. Unfortunately, it’s an internal project, so I work on it when I can, and I get bench resources when they are free. This causes many developers to come on and off the project with no predictable pattern. I kept encountering this experience:
A bug is found. I test it locally, and sure enough, it’s true, something’s not right. I look at the code and see the problem, let’s say in a helper. I determine the fix, and know 3 new tests that will assert the bug conditions. So I open the test for the helper, BUT THERE IS NO TEST! I swear in many languages. Then I run the test coverage. It’s 100%. I swear in several other languages. I look at the functional test, and see that it merely gets the page, and asserts a few unrelated things to the helper code.
So, as I am very upset, I add a 100% coverage requirement to the project. Why? Well, I am trying to force developers who come on and off the project to write tests first. I can’t understand why any of my developers would ever touch a model or helper without writing their tests first. It’s so easy, and makes the code so much better. I will admit that when I write controller or view code, I generally will write the code, and then the tests. Actually, I usually write a bunch of tests that assert the response and template. Then I code the view, and then come back and add a lot of assert_select to test the view, as it’s harder to know what to assert.
So I guess I am pissed on many levels. Partly, because code coverage can really give you a false sense of security. Writing a functional test that gets the index of a given controller, will mark most of the helper and controller code as covered, even if there are no assertions. There is no good way of testing your code base against “how good are my tests.”
In any event, I think that every project should have a code coverage report, and it should be part of the build. I also think that the build should fail if the coverage falls below a certain percent. This number should be agreed upon by the team. For some projects, the technologies at play make it really hard to get to 100% coverage. Sometimes the coverage tools have bugs that report erroneous holes in coverage. Regardless, coverage should be reviewed as a team periodically (a kick off of the iteration retrospective is a good place). Holes should be fixed as soon as they are uncovered.
Code coverage does not give a good indication of how good your tests are. Generally, if you have good developers, your tests will be excellent. Code coverage merely indicates that there is no automated test on part of your code, and you should address it.
Don’t you just love rcov’s drive by coverage?
While I think 100% coverage is a worthy goal, I think I am going to have to side with Noel in that it’s not realistic. To mangle a quote, ‘I do not think 100% coverage from rcov means what you think it means.’
If rcov is improved to define coverage as assertions then 100% coverage means significantly more. Unfortunately that is easier said than done.
When you ask for 100% test coverage, you’re asking for crappy tests. Code coverage is not a code quality metric. It’s not even a test quality metric. Don’t pretend that it is.
One way I’m exploring right now to address this is to run coverage tests for controllers, helpers and models individually and exclude the other parts from the coverage statistics. This way controller specs don’t mess up helper or model coverage results.
My take on it is that if the code coverage goes down *significantly* by some agreed upon amount, that should be a build breakage. I think that point is going to be lost behind the 100% number. “Coverage Driven Testing” is a very real problem, and I’ve seen first-hand that people can get too caught up in the numbers, but as we know, the code coverage numbers don’t mean that the tests are any good.
Imagine the coverage was at a respectable 70%, and then someone checks in a big change that brings the coverage down to 60%, but the tests are all still passing. Wouldn’t you want to know that your coverage just went down by 10%?
If it turns out that the person found a bunch of useless tests and got rid of them, that’s fine we’ll reset our number. There are always exceptions to rules, but I just want to be Alerted to big changes. (this is even more true when working with bigger teams or people you don’t know as well)
If I was on this project with Jason, I would probably suggest that the build should break if the coverage went down by a certain percent, let’s say 2-5% depending on how big the codebase is.
As Noel pointed out in his post, there are two phases
1. Detect
2. Fix
and I agree with him that its the Fixing part that is more important and indicative of how well your team works together.
imho code coverage is a very misleading metric and i like to keep distance with it.
it wouldn’t cover most serious bugs and gives you a false feeling of security. you would also find lazy/bad developers who tend to write test cases with one goal in mind: 100% should happen and that will be it.
well it might make sense in perl, where if nothing it tells you the syntax is alright
heres what i would profess:
1. have automated regression test suite
2. for every feature you add have an exhaustive set of test cases added to the suite