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

Reduction in total unit test coverage is too stringent and drives unwanted behaviour #67

Open
ben-sagar opened this issue Oct 7, 2020 · 2 comments

Comments

@ben-sagar
Copy link
Contributor

In the section on unit test coverage in the common coding standards it says:

All changes must have sufficient coverage and the overall total coverage must not decrease.

The use of the phrase "must not decrease" causes the following effects:

  • individual PRs fail review even if, for example, the total coverage has only dropped from 95.6% down to 95.4%, i.e. by a very small amount and still comfortably above the 90% standard
  • total coverage levels continuously creep up as it is not considered acceptable to ever lower it, so it only ever gets higher

It would be better if this could say something like:

All new or changed code must have sufficient coverage and the overall total coverage must not decrease below 90%.

@timtamimi
Copy link

timtamimi commented Oct 15, 2020

All new or changed code must have sufficient coverage and the overall total coverage must not decrease below 90%.

This seems sensible.

Additionally, we can consider adding a generalised caveat, such as:

"While code quality is of great importance, there may be instances where test coverage metrics are unhelpful, or obstructive to making progress (delivering at pace). In those instance, teams are encouraged to think pragmatically and to work together (with the guidance of the principal developers) to find suitable benchmarks."

@Cruikshanks
Copy link
Member

Cruikshanks commented Oct 15, 2020

I'm ok with the new wording. I am wary though of including reasons within the standards at this time that discuss scenarios when not meeting the standard is ok.

Yes, I do engage in just such pragmatism within my own teams. However, having been dropped into yet another project where low to non-existent test coverage is the norm the behaviour I want to drive is that unit tests are not optional.

If you are high performing team actively discussing how to improve the quality of your tests I don't think you have anything to worry about from a standards point of view. Until we are all there though my preference would be that the standards are pretty unequivocal.

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

3 participants