Skip to content

test: achieve 100% test coverage#383

Merged
zkoppert merged 4 commits intomainfrom
test/improve-env-coverage
Apr 8, 2026
Merged

test: achieve 100% test coverage#383
zkoppert merged 4 commits intomainfrom
test/improve-env-coverage

Conversation

@zkoppert
Copy link
Copy Markdown
Collaborator

@zkoppert zkoppert commented Apr 7, 2026

Pull Request

Proposed Changes

Adds 6 tests to cover all previously uncovered paths in env.py, bringing the repo from 96% to 100% test coverage.

The uncovered lines were all validation/error paths that had no test exercising them:

Line(s) What was uncovered New test
41-42 get_int_env_var returning None for non-integer values test_get_int_env_var_returns_none_for_non_integer
90-91 get_env_vars loading .env file when test=False test_get_env_vars_loads_dotenv_when_not_test
102 REPOSITORY starting with / validation test_get_env_vars_raises_for_repository_starting_with_slash
147 TITLE exceeding 70 char limit test_get_env_vars_raises_for_title_too_long
157 BODY exceeding 65536 char limit test_get_env_vars_raises_for_body_too_long
166 COMMIT_MESSAGE exceeding 65536 char limit test_get_env_vars_raises_for_commit_message_too_long

Testing

  • All 69 tests pass (up from 63 on main)
  • Coverage: 100% (204 statements, 0 missed)
  • make lint clean (pylint 10/10, mypy, flake8, isort, black)

Readiness Checklist

Author/Contributor

  • If documentation is needed for this change, has that been included in this pull request
  • run make lint and fix any issues that you have introduced
  • run make test and ensure you have test coverage for the lines you are introducing

Add 6 tests covering previously uncovered paths in env.py:
- get_int_env_var returning None for non-integer values
- get_env_vars loading .env file when test=False
- REPOSITORY validation for values starting with /
- TITLE exceeding 70 character limit
- BODY exceeding 65536 character limit
- COMMIT_MESSAGE exceeding 65536 character limit

Coverage: 96% → 100% (204 statements, 0 missed)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Signed-off-by: Zack Koppert <zkoppert@github.com>
@zkoppert zkoppert self-assigned this Apr 7, 2026
The test was covering lines 90-91 for coverage metrics but not
asserting that load_dotenv() was actually invoked. Mock load_dotenv
and verify it is called once when test=False.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Signed-off-by: Zack Koppert <zkoppert@github.com>
@zkoppert zkoppert marked this pull request as ready for review April 7, 2026 22:22
@zkoppert zkoppert requested a review from jmeridth as a code owner April 7, 2026 22:22
@zkoppert
Copy link
Copy Markdown
Collaborator Author

zkoppert commented Apr 7, 2026

should we pin coverage requirements to 100%?

@jmeridth
Copy link
Copy Markdown
Collaborator

jmeridth commented Apr 8, 2026

should we pin coverage requirements to 100%?

Let's live dangerously. Why not?

zkoppert and others added 2 commits April 7, 2026 21:39
Bump --cov-fail-under from 80 to 100 now that all code paths
are covered, per review feedback on PR #383.

Signed-off-by: Zack Koppert <zkoppert@github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Signed-off-by: Zack Koppert <zkoppert@github.com>
@zkoppert zkoppert merged commit 739b262 into main Apr 8, 2026
35 checks passed
@zkoppert zkoppert deleted the test/improve-env-coverage branch April 8, 2026 15:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants