loading...

Sometimes arbitrary rules have a place

ben profile image Ben Halpern Updated on ・1 min read

We've adopted a fairly arbitrary rule that all pull requests have to improve test coverage, even if the code you are submitting is already 100% covered. It's simply a matter of forcing everyone to take care of low-hanging fruit in terms of untested code that's been sitting around for a while.

I'm not always a fan of strict arbitrary rules. It's a bit silly if I think about it too hard, but so far I'm really enjoying it. It forces us to make our code more testable if we don't want this constraint to be too tedious. It also makes us quicker to remove dead code because it's a quick way to improve test coverage.

Discussion

markdown guide
 

You run the risk of either:

  • duplicated code coverage
  • invalid tests

If the author is writing a test for something that is already covered, then you end up with a duplicate test. This increases maintenance cost. It also risks confusing future contributos as they struggle to understand the difference between the tests. Probably not a tragic outcome though.

Invalid tests start to arise if people write tests for code unrelated to what they fixed or refactored. Now you have somebody writing a test which they lack complete knowledge and may end up writing an invalid one. Invalid tests hamper refactoring with false negatives -- testing conditions that weren't meant to be supported.

There's another aspect here though, the only way the code could be fully covered is if the PR is 100% refactoring, no bug fix nor new feature added. The moment it's a bug fix, or feature, implies there is a need for new test code. Though I find it unlikely that the code would be 100% covered anyway -- mainly since I don't believe 100% is something that can actually exist. :)

 

I would be interested if and how you are "enforcing" this specific rule.