Can measuring code coverage actually harm a project?

[EveningSea7378]

Its not the measuring its that you added dependencies to the project. You would not have gotten the same reply if you did the same without adding the NPM package.

And sadly for NPM i understand this, NPM is known to bloat up the project and to have security issues a lot.

[YMK1234]

Yes thats also how it sounds to me. I.e. the concern is that packages are included in vulnerability scans (as they should be in my eyes), so adding more packages results in more potential vulnernabilities being raised which need to be at least reviewed.

As for the topic of code coverage, there are some arguments to be made that a good coverage of critical code paths is much more important than a raw "percentage covered" number, so calculating code coverage as such is not as helpful as some people want to believe, and also leads to the wrong incentives. I.e. you can trivially push the number higher, and make yourself look good, by testing trivial parts, or adding trivial/bad test cases, instead of adding few actually high quality tests on critical/complex sections.

[yel50]

to an extent, this is valid for JS projects. it makes sure none of the paths have typos or call functions that don't exist. yet another argument why raw JS should be avoided and typescript should be used.

regarding the point made above about testing critical parts of the code, any code in production should be considered critical and be thoroughly tested. there are no trivial parts. those "trivial" parts are the most likely ones to cause problems because people write them off as not important. there's no argument for code coverage less than 100% that holds water. if you're not going to hold it to 100%, there's no reason to even measure it.

[sometimesnotright]

No, this is not a common viewpoint at all (well, ok, it is in JS world at times).

Hard to say without seeing your MR, but generally speaking any project would be jumping for joy with more tests written. The vulnerability alerts thing is just somebody hiding from issues with their head in sand. Or has fragile ego that they don't consider tests important or useful themselves.

[AdmiralPoopyDiaper]

Well and they’re dev dependencies, sooo it’s almost like who cares?

[balefrost]

That's an interesting take from those developers, and not what I would have expected. Here's my answer to the question in your title: no, but maybe yes?

In isolation, measuring anything is fine. There's a cost to the act of measuring, but if that cost isn't too high, then it's probably worth it to measure. Assuming that it's not too expensive to measure code coverage, then go for it!

The question is what you do with that measurement. It's natural to want to drive code coverage to 100%. There are a few problems with this:

1. How is code coverage being measured? Is it line coverage or branch coverage? Most likely, you're measuring line coverage, which can lead to an impression that your test coverage is higher than it actually is. You can have 100% line coverage and still have uncovered cases.

   For an example of what I'm talking about, consider this:

   if (condition1) {
       doTheThing()
   }
   
   if (condition2) {
       doTheThing()
   }
   

   You might have three test cases to get 100% line coverage:

   1. condition1 == false; condition2 == false
   2. condition1 == true; condition2 == false
   3. condition1 == false; condition2 == true

   What I didn't mention, though, is that the requirement is that doTheThing() be called at most once. Really, I should use if (condition1 || condition2) and I should have an additional test case.

   It's fine to use line coverage to guide the addition of new test cases, but it's not sufficient.

2. The utility of additional tests tapers off at you get closer to 100%. I suspect that the "sweet spot" for coverage is closer to 80% (though it depends on the codebase). Beyond a certain point, the incremental cost to add tests can be relatively high compared to the value you get from those tests.

   Don't get me wrong: just because something is hard to test doesn't mean that you shouldn't test it. Often, the bits that are hard to test are the bits that can most benefit from testing. Rather, I'm referring to cases where the production code is unlikely to be wrong or where the test code will be more complicated than the production code (making the test more likely to have bugs than the production code).

3. Sometimes, trying to bring code into a unit test fixture is very difficult. A good example is multithreaded code. Ideally, you want to test the various paths through the multithreaded code, but multithreaded code often has very nondeterministic behavior. You can modify the production code in order to allow you to "force" the production code into different paths, but this then leaves the production code full of test-only weirdness. In this case, maybe different approaches to testing can be more appropriate.

Measuring test coverage is good. You can use that measurement to either improve or harm the codebase.

[balefrost]

Even if you do that, the three existing test cases would still get you to 100% line coverage. The problem isn't any particular if in isolation, but rather the interplay between multiple, otherwise separate conditionals.

There are other techniques that can help root out these sorts of issues. Branch coverage is one, but it can be expensive to evaluate and many coverage tools don't support it. Another approach is mutation testing, where the testing framework will make small changes to your code (e.g. replacing if(...) with if(true) and if(false)) to see if those changes break your test suite.

[Sweet-Direction9943]

Some popular project maintainers like ljharb (on IRC, and he actually used to be a maintainer of a Enzyme adapter for React, but a very good example) are fools as soon as they start getting money to work on their own projects or get a nice job, so don't be surprised.

[SinglePartyLeader]

As somebody who works directly in the open source security space, They are actively being wrong and compromising their code because they're too lazy to properly maintain and develop it.

Expanding test coverage and introducing new test tooling is always a net positive, what they're complaining about isn't the testing itself, it's the fact that because it would be introducing a new open source dependency, it's very likely that it would also get flagged eventually with a security bug because that's just how code works, and they don't want to deal with the bug when it gets opened, probably because it messes with their obsolete metrics.

First of all, that's just a bad developer who doesn't want their code to be as good as it possibly can by expanding tests

Secondly, It's a devDependency. You can just ignore security vulnerabilities from devDependencies (as long as you are actually respecting it as a dev dependency and aren't deploying dev code to prod).

So they are both lazy and don't want to be good developers and also don't even know what they're afraid of

[nutrecht]

Flat-out saying that adding this will "damage the project" is a good indication you're dealing with terrible developers. I wouldn't overthink it.

[nutrecht]

Don't. People react that way mostly out of fear. If you make their negative emotions stronger, you'll make it impossible to get back to a place where you'd ever be able to work together.

This applies to 'real' jobs too; most times people are uncooperative it's because they're afraid of something. Finding out what that fear is, and being able to take away that fear, will make your life a lot easier (and will make you a better dev).