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
Forbid catch-all test files; require one test_<module>.py per source module
I hit a quality issue in github-community-projects/issue-metrics#775 where
I (with AI assistance) added a 945-line test_coverage_additions.py file
that bundled 36 tests across 11 source modules in a single catch-all
test file. That breaks the source-to-test mapping a maintainer relies on
to find the tests for a given module.
I added an explicit rule under Code Style & Languages forbidding
catch-all aggregator test file names (test_coverage_additions.py,
test_extra.py, test_misc.py, test_new_stuff.py, ...) and requiring new
tests to land in the existing test_<module>.py file for the module
they exercise. I also tightened the OSS Actions Project Structure
bullet to reinforce the same rule.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Signed-off-by: Zack Koppert <zkoppert@github.com>
- Run `make lint` and `make test` before committing in repos that have a Makefile
117
117
- Write unit tests for new functionality
118
+
-**Test file naming - one per source module, no catch-alls**: When a repo follows a per-module test convention (e.g., `test_<module>.py` for each `<module>.py`), put new tests for a module in that module's existing test file (creating it if missing). Never create catch-all aggregator files like `test_coverage_additions.py`, `test_extra.py`, `test_misc.py`, or `test_new_stuff.py` - they hide what is being tested and break the source-to-test mapping a maintainer relies on. If a test exercises multiple modules, file it under the module whose behavior is most central to the assertion. Before adding a new `test_*.py` file, confirm there is no existing per-module test file that would be a better home.
118
119
- Document changes to environment variables in the `README.md` file
119
120
-**Linting philosophy**: When linting errors arise, **always fix the code to pass the linter** - do not suppress, ignore, or disable lint rules. Only disable a rule as a last resort if fixing the code is truly impossible or would make it significantly worse, and explain why in a comment. This applies to all linters (flake8, pylint, mypy, markdownlint, eslint, etc.).
120
121
-**Cross-reference existing patterns**: When adding new code to a file that already has similar blocks (e.g., a new job in a workflow, a new route in a router, a new test in a suite), explicitly compare the new code against the existing code for naming conventions, formatting, and runtime behavior before committing. Don't pattern-match on the name you're defining - check how existing code actually references the same concept.
@@ -301,7 +302,7 @@ The following conventions apply to my suite of GitHub Actions in the `github-com
301
302
### Project Structure
302
303
- **Flat module layout** - all Python source files live at the repo root (no `src/` directory)
303
304
- Common module split: `{main}.py`, `auth.py`, `env.py`, `markdown_writer.py` (or `markdown.py`)
304
-
- Test files: `test_*.py` at root, one per module
305
+
- Test files: `test_*.py` at root, one per source module - never catch-all aggregator files like `test_coverage_additions.py`
305
306
- Linter configs live in `.github/linters/` (not repo root)
0 commit comments