-
Notifications
You must be signed in to change notification settings - Fork 3k
ci: add strict-no-cover to detect unnecessary coverage pragmas #1897
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
78d6633 to
edefce3
Compare
| uv run --frozen --no-sync coverage report | ||
| - name: Check for unnecessary no cover pragmas | ||
| if: runner.os != 'Windows' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Kludex without this all the windows pipelines fail to even run, seems like a strict-no-cover bug? Example failure from previous CI run: https://github.com/modelcontextprotocol/python-sdk/actions/runs/21070443439/job/60598385217
|
Is this working already? All the pragmas are actually being used? |
not sure, will test |
|
@Kludex Found two issues that prevented 1. Coverage 7.13.0 changed Fix: pin 2.
Fix: remove I also added a test |
Add strict-no-cover from pydantic to CI pipeline. This tool identifies `# pragma: no cover` comments on lines that are actually covered by tests, helping keep coverage pragmas accurate. Changes: - Add strict-no-cover as dev dependency (installed from git) - Add `pragma: lax no cover` to coverage exclude_lines for partial coverage - Add CI step to run strict-no-cover after coverage report (Linux only due to Windows bug in the tool)
Two issues prevented strict-no-cover from detecting unnecessary pragmas: 1. Coverage 7.13.x changed analysis_from_file_reporter to filter executed lines with '& statements', which removes excluded lines from the executed set. This breaks strict-no-cover's detection (it checks the intersection of excluded_lines and executed_lines). Pin to <7.13. 2. relative_files = true causes coverage to store relative paths, but strict-no-cover creates a temp rcfile without [tool.coverage.run] settings. Without relative_files in that temp config, coverage can't match paths, resulting in empty executed_lines. Also adds a test pragma on a covered line to verify the check fails.
464a9ff to
e942316
Compare
Remove ~100 unnecessary '# pragma: no cover' annotations from 30 test files. These lines are actually covered by tests, as detected by strict-no-cover in CI.
Add strict-no-cover from pydantic to the CI pipeline. This tool identifies
# pragma: no covercomments on lines that are actually covered by tests, helping keep coverage pragmas accurate.Changes
pragma: lax no coverto coverage exclude_lines for partial coverage scenariosHow it works
# pragma: no cover- errors if any marked lines are actually covered# pragma: lax no cover- permits partial coverage without erroring (useful for flaky or inconsistently covered code)