test: raise test coverage to 100% and require it in CI#775
Merged
Conversation
I added 36 unit tests in test_coverage_additions.py that exercise branches not previously hit by the per-module test files. Each test is named to indicate the source file and line range it covers so future maintainers can trace coverage gaps quickly. All GitHub API interactions are mocked with unittest.mock (no network access). Coverage rises from 91% to 100% across all modules. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Zack Koppert <zkoppert@github.com>
Two production branches are not exercisable from real code paths but
are kept for safety; I marked them with '# pragma: no cover' and a
comment explaining why coverage cannot reach them through real input:
- markdown_writer.group_issues has an else-branch that is unreachable
because the function returns early when group_by is not in
valid_fields = {'author', 'assignee'} a few lines above.
- most_active_mentors.count_comments_per_user has a discussion branch
that is structurally dead in production: the GraphQL query in
discussions.get_discussions fetches no comment author data, the
attribute access here would AttributeError on dict nodes, and
ignore_comment is called with comment.user as both issue_user and
comment_user (self-reference always True). Tracked in #774.
With the remaining production code 100% exercised by tests, I bumped
--cov-fail-under in the Makefile from 80 to 100 so future drops in
coverage fail CI immediately.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Signed-off-by: Zack Koppert <zkoppert@github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR strengthens the repo’s quality gate by raising unit test coverage to 100% and enforcing that threshold in CI, while explicitly excluding two unreachable defensive branches via # pragma: no cover.
Changes:
- Add a new
test_coverage_additions.pytest module with targeted unit tests to cover previously missed branches across multiple modules. - Mark two confirmed-unreachable branches as excluded from coverage with explanatory comments (
most_active_mentors.py,markdown_writer.py). - Increase the pytest coverage failure threshold from 80% to 100% in the
Makefile.
Show a summary per file
| File | Description |
|---|---|
| test_coverage_additions.py | Adds targeted unit tests to close remaining coverage gaps and reach 100%. |
| most_active_mentors.py | Excludes a dead discussion-counting branch from coverage and documents why it’s unreachable. |
| markdown_writer.py | Excludes an unreachable defensive else branch in group_issues from coverage with rationale. |
| Makefile | Raises --cov-fail-under to 100 to enforce coverage in CI. |
Copilot's findings
- Files reviewed: 4/4 changed files
- Comments generated: 1
jmeridth
reviewed
Jun 12, 2026
jmeridth
left a comment
jmeridth
left a comment
Collaborator
There was a problem hiding this comment.
test_coverage_additions.py?
jmeridth
approved these changes
Jun 12, 2026
I moved the tests that previously lived in test_coverage_additions.py into the per-module test files (test_config.py, test_labels.py, etc.) that match the source modules they exercise. This matches the existing test_<module>.py naming convention in the repo so future maintainers can find coverage for a given module without grepping a catch-all file. I also dropped a duplicate test_get_owners_and_repositories_handles_user_prefix test in test_search.py that exactly matched the existing test_get_owners_and_repositories_with_user test, and kept the owner: prefix variant which was not previously covered. All 191 tests still pass with 100% coverage on every source module. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Zack Koppert <zkoppert@github.com>
zkoppert
added a commit
to zkoppert/dotfiles
that referenced
this pull request
Jun 12, 2026
…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>
Collaborator
Author
Wow that was lazy! making a rule to make sure I don't do that again. Thanks for flagging. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Pull Request
Proposed Changes
I added 35 targeted unit tests that cover branches the per-module test files were missing. Each new test class lives in the existing
test_<module>.pyfile that matches the source module it exercises (for example, the SORT_ORDER fallback test went intotest_config.py, the search rate-limit and exception tests went intotest_search.py), matching the repo's existing per-module test file naming convention. All GitHub API interactions are mocked withunittest.mock(no network access).Two production branches are structurally unreachable from real input. I annotated them with
# pragma: no coverand an explanatory comment rather than writing phantom tests that mock them past:markdown_writer.group_issueselse-branch: thevalid_fields = {"author", "assignee"}guard a few lines above returns early for any other value, so the else can never execute unlessvalid_fieldsis extended without updating the if/elif chain.most_active_mentors.count_comments_per_userdiscussion branch: three layered defects make this path dead in production (GraphQL query fetches no comment author data, attribute access wouldAttributeErroron dict nodes, andignore_commentis called withcomment.useras bothissue_userandcomment_userso the self-comparison always returns True). Tracked in Discussion mentor counting is dead code (GraphQL + ignore_comment self-reference + dict-vs-object mismatch) #774 for separate triage.With those exclusions, I bumped
--cov-fail-underin the Makefile from 80 to 100 so future drops in coverage fail CI immediately.Before / after coverage
Testing
I ran
make lintandmake testend-to-end against Python 3.14 in a clean environment (matching the CI runner). All five linters pass (flake8 syntax + style, isort, pylint 10.00/10, mypy, black) and pytest reports 191 passing withRequired test coverage of 100% reached. Total coverage: 100.00%.Rollout
In this PR I only add tests, one source-code annotation, and a Makefile threshold bump. No runtime behavior changes for end users of the action. The blast radius is the CI gate itself: if a future PR's coverage drops below 100%, CI will fail until the contributor adds tests or, for genuinely unreachable defensive branches, a
# pragma: no coverannotation with an explanatory comment.What to watch after merge
I will watch the next few PRs from outside contributors for friction around the 100% coverage gate. If contributors hit the gate on legitimately defensive branches without realizing
# pragma: no coveris the documented escape hatch, I will add a short note toCONTRIBUTING.mdlinking to the markdown_writer and most_active_mentors examples in this PR.Readiness Checklist
Author/Contributor
make lintand fix any issues that you have introducedmake testand ensure you have test coverage for the lines you are introducing