Start requiring tests for new features

changed the description

afterwards, let's require tests for new features. make this very obvious in the README or something and enforce it.

Maybe add it to the MR template?

This could be totally over-engineering it, but maybe we could detect in CI if code coverage % decreases (measure using py3-coverage or something) with a MR, and fail. We could use a CI commit tag thing to skip that check if the change in coverage was related to a bug fix or other thing that's not a "new feature". A CI failure lets us (in theory) enforce this consistently, and we would have a record about why a change was done without adding tests (in the case where we tell CI to skip the check)

Would growing an already untested function also cause a reduction in coverage percentage? If that's the case, it seems like something that would be a problem since I feel like it doesn't make sense to require people to add tests to code that isn't theirs if they are just fixing a bug for instance. Or if it's code we can't easily cover with unit tests, like main() in pmb/__init__.py.

ya so the idea is that in that case, you just add a thing in the commit message to tell CI to skip that check for the branch/MR. A similar situation is like how we sometimes use the ci:skip-build thing in pmaports to tell CI to skip the build testing if we know it won't work or isn't needed.

You can make pytest print coverage with the following:

diff --git a/pyproject.toml b/pyproject.toml
index 0e2308a5..16632918 100644
--- a/pyproject.toml
+++ b/pyproject.toml
@@ -16,7 +16,7 @@ license = {text = "GPL-3.0-or-later"}
 pmbootstrap = "pmb:main"
 
 [project.optional-dependencies]
-test = ["pytest"]
+test = ["pytest", "pytest-cov"]
 completion = ["argcomplete"]
 
 [project.urls]
@@ -51,7 +51,10 @@ docstring-code-format = true
 [tool.pytest.ini_options]
 addopts = [
     "--import-mode=importlib",
-    "--basetemp=.pytest_tmp"
+    "--basetemp=.pytest_tmp",
+    "--cov=pmb",
+    "--cov-report=term-missing:skip-covered",
+    "--no-cov-on-fail",
 ]

At least this gives visibility to what's missing and what's not.

This information also fits to CONTRIBUTING.md, it can be linked from README too instead of including the requirements there since README is considered too big already (#2485).

It could also be in the documentation. Makes me wonder how we should decide what documentation goes where.

Start requiring tests for new features

Child items ...

Activity