Skip to content

ci: report test coverage in the unit test job - #143

Merged
cvas22 merged 1 commit into
masterfrom
ci/142-coverage-reporting
Oct 24, 2026
Merged

cvas22 merged 1 commit into
masterfrom
ci/142-coverage-reporting

Conversation

@cvas22

@cvas22 cvas22 commented Oct 24, 2026

Copy link
Copy Markdown
Collaborator

Closes #142 (steps 1 and 2).

  • Adds pytest-cov to test-requirements.txt.
  • The unit-test job now runs pytest --ignore tests/smoketests --cov --cov-report=term-missing.
  • setup.cfg gets [coverage:run] source = json_logging and [coverage:report] show_missing = True.

Report only: no threshold, upload or artifact, so CI results do not change. Smoke tests are not measured, so the number is a lower bound.

Tested locally (Python 3.14.6, Windows): the same command prints the per-file table, 95 passed, 80% total on current master. The CI workflow itself has not run yet.

Add pytest-cov and run the unit tests with --cov, printing per-file
coverage with missing lines in the CI log. Coverage settings live in
setup.cfg.

Closes #142

Signed-off-by: Srinivas Chippagiri <cvas22@gmail.com>
@qodo-code-review

Copy link
Copy Markdown

ⓘ Qodo reviews are paused because the subscription is no longer active. Ask your workspace admin to reactivate the subscription to resume reviews. Manage billing

@coderabbitai

coderabbitai Bot commented Oct 24, 2026

Copy link
Copy Markdown

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 45 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used all 2 included reviews currently available.

Learn how review limits work.

Review configuration:

Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: defaults
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 54a09e0b-3cbd-4468-a469-21aa17237e02
Commits

Reviewing files that changed from the base of the PR and between 549176a and 1e284d9.

Files selected for processing: 3
  • .github/workflows/code_quality.yml
  • setup.cfg
  • test-requirements.txt
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@llamapreview llamapreview Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LlamaPReview — No blocking issues found

Safe to merge: the unit-test matrix now emits a per-file coverage table via pytest-cov and setup.cfg, and all six Python-version test jobs plus Flake8 pass at this head, so CI outcomes are unchanged as claimed.

1 non-blocking finding retained — highest: Shared test manifest silently drops sanic-testing; six-version unit matrix passes without it.

Review details and evidence
Priority File Finding Evidence
P2 test-requirements.txt Shared test manifest silently drops sanic-testing; six-version unit matrix passes without it. confirmed

Finding details

P2 · Shared test manifest silently drops sanic-testing; six-version unit matrix passes without it.

test-requirements.txt

test-requirements.txt both adds pytest-cov and deletes sanic-testing, while the PR description and commit message mention only coverage reporting. The shared manifest is installed by the flake8 job and every Tests (3.9–3.14) job via pip install -r test-requirements.txt, so the removal silently shrinks the environment used by the whole unit matrix.

Reachable consequence if a test ever needs Sanic's in-process test-client utilities: the unit job would error ("sanic-testing is not installed") or skip rather than run, and coverage would quietly become a weaker lower bound.

Strongest counterevidence: all six version jobs pass at this head with the manifest as changed, and the Sanic smoke job never consumed the shared manifest for this purpose — its own tests/smoketests/sanic/requirements.txt lists Sanic pins, requests, pytest, and -e. only, and both Sanic smoke jobs succeed. sanic itself remains a manifest entry, so only the testing helper disappeared.

This is documentation/traceability of an unrelated delta, not a demonstrated defect: nonblocking.

Owner action: Post-merge acceptable: state the motive (e.g. a sanic>=25.12/Python 3.14 conflict) in the PR body or commit message, or split the removal into its own PR.

Verification boundary: confirmed; scope: bounded reviewed context.

Material unknowns

  • Whether a coverage threshold such as fail_under exists in a config file other than setup.cfg (e.g. pyproject.toml, pytest.ini, tox.ini, .coveragerc). Cannot change the posture: any such setting would be pre-existing and not introduced by this PR, and setup.cfg itself contributes none.
    • Check: No action required; if desired, confirm no coverage threshold is configured outside setup.cfg.
  • Whether any non-executed test path imports sanic_testing or calls app.test_client. Does not decide closure: the six-version unit matrix and both Sanic smoke jobs pass with sanic-testing absent, so no exercised path depends on it; only silent skips, unsupported by supplied evidence, would escape that reasoning.
    • Check: No action required; raise only if a future test needs Sanic test-client utilities.

LlamaPReview checks

  • Read the complete PR-head file .github/workflows/code_quality.yml.
  • Read the complete PR-head file tests/smoketests/sanic/requirements.txt.
  • Read the complete PR-head file setup.cfg.

Still coding alone? Open CarbonChat, a chat room right inside Codex.

@cvas22
cvas22 merged commit 1f49248 into master Oct 24, 2026
16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add test coverage reporting to CI

1 participant