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

DEFRA / software-development-standards

Standards and guidance relating to software development in Defra

https://defra.github.io/software-development-standards/

Other

47 stars 17 forks source link

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

Open ben-sagar opened 4 years ago

ben-sagar commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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.