I so agree with this. I've been fighting this exact battle at work lately. People on my team have decided to take testing seriously, which is fantastic, but many team members' understanding of what that means is still at the "watch unit-test coverage numbers go up" stage. So let me be very clear where I stand.
* 100% unit-test coverage is a garbage goal. *
I don't hate unit tests. They can have enormous value shaking out edge cases in a self contained piece of code - usually a "leaf" in the module dependency graph - and making it future proof. Love it. However, unit tests don't tell you anything about whether the module's behavior in combination with others leads to a correct result. It's possible to chain a bunch of unit tests each with 100% coverage together, and still have the combination fail spectacularly. In operations, a lot of bugs live in the interstices between modules.
Even worse, 100% line coverage means a lot less than people seem to think. Let's say one line to compute an expression will compute the wrong value for some inputs. You might have a zillion unit tests that cause that line to be counted as covered, but still be missing the test that reveals the error case. I see this particularly with ASSERT or CHECK types of macros, which get counted toward 100% coverage even though no test ever exercises the failure case.
Striving too hard for 100% unit test coverage often means a lot of work - many of our unit tests have 3x more tricky mock code than actual test code - to get a thoroughly artificial and thus useless result. The cost:benefit ratio is abominable. By all means write thorough unit tests for those modules that have a good ratio, but in general I agree with the admonition to focus more effort on functional/integration tests. In my 30+ years of professional program, that has always been a way to find more bugs with less effort.
"You might have a zillion unit tests that cause that line to be counted as covered, but still be missing the test that reveals the error case."
Ironically it's actually easiest to have 100% coverage in worse code, because the more entangled and coupled your code is, the more likely you are to hit branches that are not under test.
> Even worse, 100% line coverage means a lot less than people seem to think. Let's say one line to compute an expression will compute the wrong value for some inputs. You might have a zillion unit tests that cause that line to be counted as covered, but still be missing the test that reveals the error case. I see this particularly with ASSERT or CHECK types of macros, which get counted toward 100% coverage even though no test ever exercises the failure case.
I'd rather have 30% test coverage where the lines under test are the complicated ones (not things like factories) with the testing of those lines hitting all the complex edge cases than 100% test coverage that confirms that all your source code is encoded in UTF-8.
Coverage driven testing is (almost) always (mostly) evil. Coverage as a metric is great. Maybe you're not going for 100%, however it does tell you something about your test set(s), providing a decent measure of "doneness". However all of this is depends on good test cases. Test cases that test something. That might sound obvious, but it's fairly easy to achieve 100% coverage while testing nothing. By the way, the reason I said "almost" and "mostly" before, is you can find bugs while attempting to improve coverage; provided you take a step back forget about achieving coverage, and instead write good tests, that happen to get you the coverage. There's a lot of temptation there and it's not an overall good strategy, but you'll find stuff.
If you think 100% code coverage with unit tests is bad you should see what happens when it's done with integration tests. I'm refactoring some tests now that were written with code coverage in mind, apparently with bonuses tied to the coverage stat. I think I've seen every testing anti-pattern possible in just this one group of ~25 tests.
There's the developers not understanding the difference between unit and integration tests. Both are fine, but integration tests aren't a good tool to hit corner cases.
Many of the tests don't actually test what they pretend to. A few weeks ago I broke production code that had a test specifically for the case that I broke, but the test didn't catch it because the input was wrong, but the coverage was there.
Most of the tests give no indication of what they're actually testing, or a misleading indication, you have to divine it yourself based on the input data, but most of that is copy/pasted, so much of it isn't actually relevant to the tests (I suspect it was included in the overall test coverage metric).
The results of the code defined the test. They literally ran the code, copied the file output to the "expected" directory and use that in future comparisons. If the files don't match it will open a diff viewer, but a lot of things like order aren't deterministic so the diff gives you no indication of where things went wrong.
Many tests succeed, but for the wrong reason, they check failure cases but don't actually check that the test failed for the right reason.
Some tests "helpers" are actually replicating production code, and the tests are mostly verifying that the helpers work.
Finally, due to some recent changes the tests don't even test production code paths. We can't delete them because it will reduce code coverage, but porting them to actually test new code will take time they aren't sure the want to invest.
Wow. Yeah, that sounds awful. You're absolutely right that pursuing 100% coverage in integration tests is bad too, perhaps even worse. I just haven't seen that in my own direct experience. Having too few requirements around integration tests seems like a far more common problem than having too many.
Before you battle too hard, let me introduce you to another way of thinking. It may not be to your liking, but I hope you'll find it interesting nonetheless. I'll try to keep it as short as I can. Usually when I type this same post, it takes me a while, but I'm getting better at it.
Imagine that the word "test" is a misnomer, when talking about unit tests. Often people think about testing as a way of checking whether or not the code works properly. This is great for what is known as "acceptance testing". However, as you no doubt agree, it's not so great with "unit testing".
For some reason, people hang on hard to the words "unit" and "test" and come to the conclusion that you should take a piece of code (usually a class), isolate it and show that the class does was it is supposed to. This is a completely reasonable supposition, however in practice it doesn't work that well (I will skip the discussion, because I think you're already in agreement with me on that front).
Instead, imagine that "unit" refers to any piece of code (at any level) that has an interface. Next imagine that "test" means that we will simply document what it does. We don't necessarily worry ourselves about whether it is correct or not (though we wish it to be correct). We just write code that asserts, "When I do X, the result is Y".
At the macro level, we still need to see if the code works. We do this either with automated acceptance tests, or manual testing. Both are fine. When the code works to our level of satisfaction, you can imagine that the "unit tests" (that are only documenting what the code at various levels is doing) are also correct. It is possible that there is some incorrect code that isn't used (which we should delete), or that there are some software errors that cancel each other out (which will be rare). However, once the code is working on a macro scale, in general, it is also working on a micro scale.
Let's say we change the code now. The acceptance tests may fail, but some of the "unit tests" will almost certainly fail (assuming we have full "unit test" coverage). If they don't there is a problem because "unit tests" are describing what the code is doing (the behaviour) and if we change the behaviour, the tests should fail.
For some types of unit testing styles (heavily mocked), often the unit tests will not fail when we change the behaviour. This means the tests, as a long lasting artefact are not particularly useful. It might have been useful for helping you write the code initially, but if the test doesn't fail when you change the behaviour, it's lost utility. Let's make a rule: if the test doesn't fail when the behaviour fail, it's a "bad" test. We need to remove it or replace it with a test that does fail.
The other problem you often run into is that when you change one line of code, 200 tests fail. This means that you spend more time fixing the tests than you gained from being informed that the test failed. Most of the time you know you are changing the behaviour, and so you want to have very little overhead in updating the tests. Let's make another rule: Unit tests must be specific. When you change specific behaviour only a few (on the order of 1) tests should fail.
This last one is really tricky because it means that you have to think hard about the way you write your code. Let's say you have a large function with many branch points in it. If you give it some input, then there are many possible outputs. You write a lot of unit tests. If you then change how one of the branch points are handled, a whole class of tests will fail. This is bad for our rule.
The result of this is that you need to refactor that code so that your functions have a minimum number of branch points (ideally 0 or 1). Additionally, if you split apart that function so that it is now several function, you have to make each of the functions available to your test suite. This exposes rather than hides these interfaces.
The end result is that you decouple the operation of your code. When you hear about TDD being "Test Driven Design", this is what it means. This is especially true for things like global variables (or near global instance variables in large classes). You can't get away with it because if your functions depend on a lot of global state, you end up having tests that depend on that (near) global state. When you change the operation surrounding that state, a whole whack of tests fail.
Committing to writing high coverage unit tests which also have high specificity forces you to write decoupled code that doesn't rely on hidden state. And because it doesn't depend on hidden state, you have to be able to explicitly set up the state in your tests, which force you to write code where the dependencies on the objects are clear and uncomplicated.
You mentioned code coverage. I'm going to say that I almost check code coverage when I'm doing TDD. That's because if you are writing tests that cover all the behaviour, you will have 100% code coverage and 100% branch coverage. However, as you correctly point out, the opposite is not the case. The test for the coverage of your code is not a code coverage tool, it's changing the behaviour of the code and noting that the tests fail.
Most people are familiar with the idea of "Test First" and often equate that with "Test Driven". "Test First" is a great way to learn "Test Driven", but it is not the only way to go. When you have full test coverage, you can easily modify the code and observe how the tests fail. The tests and the production code are two sides of the same coin. When you change one, you must change the other. It's like double entry accounting. By modifying the production code and seeing how the tests fail, you can information on what this code is related to. You no longer need to keep it in you head!
When I have a well tested piece of code and somebody asks me , "How hard is to to do X", I just sketch up some code that grossly does X and take a look to see where the tests fail. This tells me roughly what I'll need to do to accomplish X.
I see I've failed (once again) to keep this post small. Let me leave you with just one more idea. You will recall that earlier I mentioned that in order to have "full coverage" of unit tests with specificity, you need to factor your code into very small pieces and also expose all of the interfaces. You then have a series of "tests" that show you the input for those functions with the corresponding outputs. The inputs represent the initial state of the program and the outputs represents the resultant state. It's a bit like being in the middle of a debugging session and saving that state. When you run the tests, it's like bringing that debugging session back to life. The expectations are simply watch points in the debugger.
When I'm debugging a program with a good suite of unit tests, I never use a debugger. It is dramatically faster to set up the scenario in the tests and see what happens. Often I don't have to do that. I often already have tests that show me the scenario I'm interested in. For example, "Is it possible for this function to return null -- no. OK, my problem isn't here".
Richard Stallman once said that the secret to fixing bugs quickly is to only debug the code that is broken. "Unit tests" allow you to reason about your code. If you have so called unit tests that are unreadable, then you are giving up at least 50% of the value of the test. When I have problems, I spend more time looking at the tests than the production code -- because it helps me reason about the production code more easily.
I will leave you with one (probably not so small caveat). Good "unit testing" and "good TDD" is not for everyone. I talked about ruthlessly decoupling code, simplifying functions to contain single branch points, exposing state, exposing interfaces (privacy is a code smell). There are people for which this is terrible. They like highly coupled code (because it often comes with high cohesion). They like code that depends on global state (because explicitly handling state means having to think hard about how you pass data around). They like large functions with lots of branch points (because it's easier to understand the code as a whole when you have the context together -- i.e. cohesion). Good unit tests and TDD work against that. If you want to write code like the above, I don't think unit tests will work for you.
I personally like this style of programming and I think it is dramatically more productive that many other styles of programming. Not everybody is going to agree. I hope it gives you some insight as to why some people find unit testing and TDD to be very productive, though.
Great response. :) I don't actually see anything there to disagree with. Maybe we have a difference of ... perspective? style? ... on some points, but no actual disagreement. I rather think the two rants are quite complementary. Thanks!
My pleasure. Actually, someone a few seconds ago posted "Keep trying" with respect to my trying to keep it small and then I guess thought better of it and deleted the post. I'm a little sad about that because I really think it was completely on the ball. This stuff is so subtle and it's super hard say something that has any meaning without burning though a ton of trees. But the downside is that it requires a lot of effort to follow the discussion. I think there is a simple message in there somewhere, but I've yet to find a way to express it.
But I agree completely on the issue of style. I think it really comes down to that. Conflicting styles is one of the hardest things to combat on a team and you often end up with some bizarre hybrid that doesn't work at all.
That person might have been banned, deservedly so. If you want to enjoy the internet you either lurk, or you learn to ignore the haters.
FWIW I liked to read you post. Especially your point about balancing cohesion vs unit testing, as this is something that the TDD evangelists never bother to mention.
This is exactly what I've been doing for a while. I check whether my ideas are SOLID and this have been enough so far. When I change something it only affects its immediate surroundings so things are easy to fix. If something breaks it doesn't cascade throughout the system. I also like to have interfaces for all non-trivial things so I can stub them when I test and provide multiple implementations for different use cases (strategy). With this I feel that I'm insanely productive.
> The result of this is that you need to refactor that code so that your functions have a minimum number of branch points (ideally 0 or 1). Additionally, if you split apart that function so that it is now several function, you have to make each of the functions available to your test suite. This exposes rather than hides these interfaces.
> The end result is that you decouple the operation of your code. When you hear about TDD being "Test Driven Design", this is what it means.
So I think this is what creates Java-itis, promotes over-abstraction and shifts the locus of the semantics of the program away from code flow and towards dynamic runtime data shape, which may be driven via verbose construction, dependency injection, configuration data, or potentially an arbitrarily complex program that writes the actual program. I think it makes programs harder to understand because instead of being locally understood, the dynamic composition must be mentally evaluated to understand what's going on.
It's what Java programmers do to create a domain-specific language in the style of Lisp or Smalltalk, the kind of DSLs that enabled tightly knit teams to be pretty productive but create effectively custom languages that are all but incomprehensible to people coming into the project. There are strong reasons why most development isn't in Lisp or Smalltalk.
I believe abstractions should hide details. If your abstractions have so little details so that they only branch zero or once, the branches will be elsewhere; they'll be in the composition, where they are less visible and no longer comprehensible by merely understanding the language, instead one needs to understand the system.
> I talked about ruthlessly decoupling code, simplifying functions to contain single branch points, exposing state, exposing interfaces (privacy is a code smell). There are people for which this is terrible. They like highly coupled code
Exposed state and exposed interfaces are what create highly coupled code. I think you misunderstand what other people mean by the word "coupling". Coupling means local changes have non-local effects. Visibility and hiding is absolutely crucial to reducing coupling because things that can't be observed cannot have non-local effects.
Let's talk about coupling, from less controversial to what would appear to be more controversial in your perspective.
Inheritance has high coupling. Change the base class and chances are you need to modify all the descendants. Base classes are very hard to write such that they will work correctly when any virtual method may have their implementation replaced by a descendant. If the base class and the descendants are maintained by different teams, the ability to refactor the base class is highly limited; not only do they need to worry about the external API, but also the internal API, the implicit contract in which methods the implementation calls and when.
Large interfaces have high coupling. The fatter the interface, the more permutations of conversation that can occur over it. That makes the chances of a change breaking something higher.
Public data structures have high coupling - other code grows to take dependencies on the data structures and you can no longer freely modify the data structures without breaking the client code. Publicly mutable data structures are even worse: code cannot preserve local (to the code) invariants because other code elsewhere may violate those invariants.
> They like code that depends on global state
This is such a ludicrous straw man it borders on libel! Nobody who prefers data hiding will prefer global state. Just listen to it: it sounds like a contradiction by definition!
I understand why you feel your style makes you more productive. I believe it can make you more productive.
Can you understand why I don't think that style makes a team or a company more productive?
In my opinion this is bang on.
Millions of itty-bitty little classes that do so little themselves that they rely on eighteen other itty-bitty dependency-injected "services" are the bane of our industry.
They make it impossible to see how the code is really structured and works, hide state and data flow in the composition (which is where all the bugs then go to lurk), and make it harder to refactor anything without spending days changing all the tests.
For some reason people who are fond of this style seldom use real implementations to test their code, even if those things do no actual I/O, preferring instead to have masses of brittle mock set-up, such that all the tests really prove is that your mental model of how the rest of your code actually works is broken, as you watch the system fail in production in ways that seem surprising to you.
You'll then probably think you should do something about these failures and jump to the other extreme, writing brittle end-to-end tests that are very hard to actually diagnose failures in.
It's a good observation, but I'm still going to disagree. Let's look at the two cases.
(1) It really is dead code. OK, great, but I've seen people spend a whole day writing hundreds of lines trying to exercise it before they conclude it's truly dead. Is it worth it? If a small volume of dead code is worth expunging at all, I suggest that there are more efficient ways to solve that particular problem.
(2) It should be dead, but it's "revived" by constructing an artificial situation in which it does get called even though it never could in real life. Again, I've seen people waste days on this exercise. Now you're carrying around the dead code and the tests/mocks that make it undead.
So in what situation is there a net benefit? In my experience, any dead code that's found and removed that way is only so at great expense, by people who only found it because they were pursuing the arbitrary 100% goal. I don't think that makes the case that 100% unit test coverage is a goal worth pursuing.
In both your cases you have a situation where someone is creating tests based on what they think will increase coverage the most. I don't think that's necessarily what the parent is saying though. I think what they are saying, is that you can write tests based on the expected/documented behaviour of the module, and if the coverage ends up less than 100%, it's because your module has code paths which are not required according to the expected behaviour. The key is that adding new tests is not the solution unless you can specifically identify expected behaviours that you missed in the tests. Looking at the code and trying to reverse engineer what tests are necessary to achieve 100% coverage will always lead to the situations you describe.
These sorts of tests aren't efficient though. I suspect the only way to get this outcome from your tests is if you encode all your edge cases into end-to-end integration tests (which would reveal which portions of the code can never be hit)... I find that approach to testing to be too expensive, and prefer an approach where success cases and known to-be-difficult cases (like say, using a strange third party tool with weird error signaling) are encoded in end-to-end tests with edge cases being limited to small units of code.
> if you encode all your edge cases into end-to-end integration tests
I think that's what you should do. Your integration tests should check that all your specifications are validated. And your specifications should cover all edge cases.
So yes that's a lot of "slow" tests. But I think the best would be to work on the tooling to make those tests faster and easier to setup, not limit their quantity.
I don't believe tests (and the resulting coverage reports) are a time efficient way to locate dead code, you will need 100% logically covered code and some of your dead code may be under unit test and end up being included in the category of "covered code". I think the best way to locate dead code is to simplify it or refactor it - and when you approach a well factored code base then it's usually much easier to see which portions are unreferenced.
* 100% unit-test coverage is a garbage goal. *
I don't hate unit tests. They can have enormous value shaking out edge cases in a self contained piece of code - usually a "leaf" in the module dependency graph - and making it future proof. Love it. However, unit tests don't tell you anything about whether the module's behavior in combination with others leads to a correct result. It's possible to chain a bunch of unit tests each with 100% coverage together, and still have the combination fail spectacularly. In operations, a lot of bugs live in the interstices between modules.
Even worse, 100% line coverage means a lot less than people seem to think. Let's say one line to compute an expression will compute the wrong value for some inputs. You might have a zillion unit tests that cause that line to be counted as covered, but still be missing the test that reveals the error case. I see this particularly with ASSERT or CHECK types of macros, which get counted toward 100% coverage even though no test ever exercises the failure case.
Striving too hard for 100% unit test coverage often means a lot of work - many of our unit tests have 3x more tricky mock code than actual test code - to get a thoroughly artificial and thus useless result. The cost:benefit ratio is abominable. By all means write thorough unit tests for those modules that have a good ratio, but in general I agree with the admonition to focus more effort on functional/integration tests. In my 30+ years of professional program, that has always been a way to find more bugs with less effort.