You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
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.
The text was updated successfully, but these errors were encountered:
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 wascommon/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.
The text was updated successfully, but these errors were encountered: