Qualtran icon indicating copy to clipboard operation
Qualtran copied to clipboard
verified publisher icon quantumlib

Test coverage

Open mpharrigan opened this issue 2 years ago • 5 comments

  • decide a policy for test coverage
  • add a CI check that enforces that policy

mpharrigan avatar Jul 12 '23 18:07 mpharrigan

In cirq, each changed line must have 100% line coverage. I think this is overzealous

  • line coverage is a very rough proxy for logical coverage
  • mandating test coverage for e.g. input validation code and exception handling perversely incentivizes not writing input validation code
  • lines may be covered by other unit tests that incidentally exercise the code in question rather than by an actual, designed unit test

My preference would be for a reviewer-enforced (rather than tooling enforced) policy of at least one unit test per function, method, or data class

mpharrigan avatar Jul 12 '23 18:07 mpharrigan

My preference would be for a reviewer-enforced (rather than tooling enforced) policy of at least one unit test per function

This is fine for a while but I don't think this should be the long term strategy. Tooling enforced test coverage has proved to be very useful to find bugs in the past and I think it's important to have as the size of the codebase and number of people contributing to it grows over time.

tanujkhattar avatar Jul 12 '23 21:07 tanujkhattar

It would be nice to have a few examples of popular scientific python projects' policy and tooling for coverage. I'm pretty ignorant of what is best practice here

mpharrigan avatar Jul 12 '23 21:07 mpharrigan

Do you want to put in the incremental coverage checks we have in cirq?

dstrain115 avatar May 14 '24 22:05 dstrain115

No, I think tooling-enforced 100% line coverage requirements has costs and externalities that don't outweigh the benefit

mpharrigan avatar May 16 '24 23:05 mpharrigan