Skip to content

Enforce 100% coverage on the test modules themselves #32659

@kdmccormick

Description

@kdmccormick

More specifically: check for 100% coverage on modules named */tests/* or */test_*.py as a required PR check.

This is not a proposal to bring edx-platform as a whole to 100% coverage.

Details

Enforcing a coverage threshold on application code certainly comes with its pros and cons. But we can all agree that if a test case is added to edx-platform, running the test suite should run that test case, right?

This would prevent folks from adding code that they think will be run as unit tests, but due to an honest mistake, won't be. That happened here:

This would also shield us from tooling issues which might silently stop part of the test suite from running for some amount of time, potentially letting swaths of tests regress until it's noticed. This happened to the code in xmodule/ once (back when it was common/lib/xmodule/xmodule and had its own test settings), and we were lucky that the suite was easy to restore.

An implication of this, which I'm personally OK with, is that it would enforce that test helpers get removed if the unit tests that call them are removed. I'm sure there are some individual exceptions (eg, maybe codejail-related tests can only be run locally), but those should be possible to exempt as needed.

Metadata

Metadata

Assignees

No one assigned

    Labels

    code healthProactive technical investment via refactorings, removals, etc.

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions