Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Test coverage #316

Closed
scratscratte opened this issue Mar 7, 2013 · 16 comments
Closed

Test coverage #316

scratscratte opened this issue Mar 7, 2013 · 16 comments

Comments

@scratscratte
Copy link

Would it be a good idea to have test coverage in with the tests?
Heres a example of the tests on my own project, which started as the test platform from here and ive been slowly modding it and adding bits.
Untitled

@twpayne
Copy link
Contributor

twpayne commented Mar 22, 2013

To revive this, although testing is a very useful technique, I think that measuring test coverage as a simple percentage statistic leads to the perversion of that statistic. It's a truth of the Internet that if you publish a number on the Internet, be it anything from number of "friends", "followers", "karma" or "likes", then people naturally do whatever they can to boost that number - irrespective of the costs or the the spirit in which the statistic was created.

In the case of test coverage, this leads to people writing completely useless tests (expect(new Foo()).to.be.a(Foo)) just to get 100% coverage, or fiddling the coverage tool to ignore parts of the codebase (# pragma: nocover). Both approaches follow the letter of the law but are diametrically opposed to its spirit.

Tests are good, but they are not free. Everyone agrees that modular design is good and coupled components are bad. It's all too easy to write tests that are tightly coupled to the implementation - and looking to maximise coverage reinforces this behaviour. To write 100% coverage tests, you tie yourself to implementation details that simply do not matter, and as such only act as a dead weight on development.

Consider the following:

if (x == 1) {
  y = foo();
} else {
  y = bar();
}

vs.

y = x == 1 ? foo() : bar();

A test that covers only the case where x == 1 gets only 66% line coverage in the first case, and but 100% line coverage in the second. The quality of the test is the same, but the coverage statistic is utterly misleading and leads to either busy work (testing trivial cases - increasing coupling) or subversion (restructuring code to maximise test coverage).

Testing is a useful tool. Test coverage is a false idol. Do not worship false idols.

@justinvanwinkle
Copy link

Line coverage is not the same as coverage.

That is why most coverage tools have something akin to branch coverage. A line isn't covered until your tests hit all the branches: http://nedbatchelder.com/code/coverage/branch.html

100% test coverage, if implemented in proper unit tests, is a good goal. Coverage alone doesn't mean you are actually testing the code, but having code that is not touched by any test is in and of itself a BadThing™.

@trisweb
Copy link

trisweb commented Mar 22, 2013

@justinvanwinkle the excellent points against coverage being used as a metric of your code's quality still stand. Coverage is a fine statistic to be used as a general measure, however as soon as it changes to a target or goal, then it begins to affect the very thing it's trying to measure. If programmers are targeting code coverage above test quality, then there is a real likelihood that the quality of the tests will suffer. That was the point.

@twpayne you wouldn't, by chance, be familiar with the teachings of W. Edwards Deming. Your points align with his ideas about factory management and quality control almost exactly. Specifically his 11th principle, which states that you should eliminate numerical quotas and "management by objective." I've always been curious to apply his theories to software, as I think there are many parallels, and this is a perfect example.

@twpayne
Copy link
Contributor

twpayne commented Mar 22, 2013

@justinvanwinkle I'd argue that having code that is not - directly - covered by tests is actually a Good Thing™. Why? Because this is code that you can effortlessly improve. There is no friction here, and as long as you conform to the higher level API/behavioural tests then you can make things better.

Striving for 100% coverage means writing tests for code that is obviously correct. You might be very confident that 1 + 1 == 2 but unless you include a test that asserts that 1 + 1 == 2 then the test coverage tool - even if it's clever enough to understand the difference between line coverage and branch coverage - still needs persuading that you know what you're doing.

Test coverage focuses on trivial examples. If I can find one combination of inputs that allows my tests to pass, then the test coverage tool is happy. Consider the following example:

function add(a, b) {
  return a * 2;
}

assertEquals(add(2, 2), 4);

The tests pass. I have 100% line - and branch - coverage. My code is still wrong. Striving for 100% coverage has not improved my code quality.

@trisweb Is Deming the same guy who said "you get what you measure?". If so, I agree.

@justinvanwinkle
Copy link

@twpayne " I'd argue that having code that is not - directly - covered by tests is actually a Good Thing™. Why? Because this is code that you can effortlessly improve. "

Everyone agrees that coverage - by itself - doesn't mean anything.

If you write code that is hard to test, and you write bad tests, then your tests end up having a lot of implementation assumptions baked into them. This can be paralyzing, I agree.

However, if you black box your tests, and you write code with testing in mind, use dependency injection and avoid global state, then you are just testing the functionality of the code. As long as extending that code doesn't break the functionality, the tests still pass (although you still need to extend the tests to cover the new thing you added).

So, if you are careful and do a good job, you never break a test without breaking functionality.

If you write bad tests, for example by embedding white-box implementation assumptions in the tests, you are going to have a bad time.

You have to be just as smart and careful when you write your tests as you do when you write your code.

There is an easy measure to know if you have good tests: if adding more tests lets you develop new features faster, because you can be less careful while writing the actual code, because you trust that you won't break functionality without breaking tests.

If you feel like it is really hard to change code, because it will cause a bunch of test failures and the tests are hard to deal with, whoever wrote those tests did a really bad job and needs to be taught to do a better job.

@trisweb
Copy link

trisweb commented Mar 22, 2013

@twpayne Ha, he might have said, "You get what you know." His theories were more about holistic knowledge and understanding of statistics than the measurements themselves. An actual quote of his that may shed light on the subject: "The most important figures that one needs for management are unknown or unknowable, but successful management must nevertheless take account of them.” (from Out of the Crisis, p121)—may not be applicable, but I find his ideas fascinating and relevant to this discussion.

@twpayne
Copy link
Contributor

twpayne commented Mar 22, 2013

@justinvanwinkle I totally agree with you. When writing tests you do have to be smart and careful. This is the hallmark of good tests. What I don't understand is how increasing test coverage helps you write smart and careful tests.

@justinvanwinkle
Copy link

@twpayne

The other way around. If you are smart and careful, and your tests actually test what they cause to be covered, then increasing coverage is purely a good thing.

If your tests are bad, coverage is meaningless, and maybe even harmful.

I agree with you. It's better to have no tests at all than to have tests that are really bad, or don't actually test functionality. If your tests are really bad, you have a cost with no benefit. So in a project with bad tests, even discussing anything beyond "we should delete these tests and try again" is putting a really fine point on it.

@twpayne
Copy link
Contributor

twpayne commented Mar 22, 2013

@justinvanwinkle I think we'd work well together :) I agree with you that bad tests make coverage meaningless, maybe even harmful. I'd extend this to say that focusing on coverage encourages meaningless - and even harmfull - tests.

@justinvanwinkle
Copy link

Anyway, to bring it all the way back to the original issue:

If the tests on this project aren't any good, you had better start by fixing that issue.

If the tests on this project are good, measuring coverage can be helpful so that you know what isn't tested.

@trisweb
Copy link

trisweb commented Mar 22, 2013

@twpayne @justinvanwinkle - if I may interject, I think you two completely agree. Test quality comes first and is the top priority, test coverage is generally good to increase as long as you don't use it as the highest goal.

Or, as in @twpayne's initial response, "Testing is a useful tool. Test coverage is a false idol. Do not worship false idols." I believe the overall point is: the goal of testing is to have quality tests which lock down important functionality so that bugs are detected quickly and easily when they are created. The purpose of tests is not to cover every line of your codebase with tests. I believe no working programmer would disagree.

@justinvanwinkle
Copy link

@trisweb

I think you are right. But this way we get to show off how right we are. It's like the dialogs you read in Plato, only better.

@trisweb
Copy link

trisweb commented Mar 22, 2013

Perhaps a good way to put this is that the top goal is Quality software, the process control of Testing helps you achieve that goal, and the purpose of coverage is Knowledge about your process in order to achieve continual improvement, which leads to higher Quality. The coverage number should never be confused with the goal of quality process and quality software, and thus it should never be used as a goal, it is only information.

Exactly aligned with Deming's quality theories. Seriously, if you guys are interested in this stuff, you should read up on him.

@tschaub
Copy link
Member

tschaub commented Mar 22, 2013

How about we just skip the coverage nonsense and go right to mutation testing?

FWIW, I'm still (occasionally) working on the branch that will allow us to use Testacular Karma - and we'll get test coverage for free.

@justinvanwinkle
Copy link

Oh, I think I omitted one critical thing which makes coverage actually useful:

You have to write real unit tests. In that, the test only depends on a single 'unit' of code, any changes outside that unit cannot cause it to fail.

People who use code coverage as a tool and then write integration tests are fooling themselves. They are running a lot of their code, but that's not the same as testing it.

@Turbo87
Copy link
Contributor

Turbo87 commented Mar 27, 2013

@twpayne twpayne closed this as completed Apr 22, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants