test(audit): close the four remaining minor items from the May 1 audit#104
Conversation
All four are nice-to-haves the original audit flagged as low-impact; none are bugs. Bundled into a single cleanup PR. 1. Add clipboard.reset_backend_cache() public helper. Test fixtures in tests/test_server.py and tests/test_integration_x11.py previously reached into the module-private _backend global directly; the helper makes the reset hook discoverable. 2. Direct unit tests for parser._has_header_row covering the len(rows) < 2 early-return, the uniform-text-no-header path, and a positive text-header-over-numeric-data case. Previously exercised only indirectly via format_table JSON output. 3. Regression test for clipboard_paste include_schema=True when data rows are wider than the header row; asserts synthetic Col 3 / Col 4 labels emitted by the padding loop at server.py:222-228. 4. Coverage for MCP_CLIPBOARD_MAX_WRITE_BYTES and MCP_CLIPBOARD_MAX_IMAGE_BYTES env-var validation. Both are parsed via int(os.environ.get(...)) at module import; non-integer values raise ValueError before anything else runs. Tests run each in a subprocess so the partially-imported state stays isolated from the in-process module cache (reloading in-process broke 21 other tests because they hold references to the module's exception classes).
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
|
[QA] Starting review at 2411086. Will verify the new |
cmeans
left a comment
There was a problem hiding this comment.
QA review — round 1
Clean cleanup PR at 2411086. Zero findings.
The new reset_backend_cache() helper is the right shape: small, well-documented as test-only, and the migration of the two existing autouse fixtures (tests/test_server.py:1112-1124 and tests/test_integration_x11.py:46-52) is uniform. The subprocess approach for env-var validation tests is the correct call — the PR body's implementation note about importlib.reload() clobbering 21 other tests via class-identity divergence is an accurate diagnosis (a reloaded module's ClipboardError is a different class object than the one captured in already-imported test references). The _has_header_row direct tests cover the len(rows) < 2 early return + the type-comparison branch, and the padding-loop regression test exploits a real ragged-row scenario (HTML parser doesn't normalize widths at parse time, so a 2-cell <th> header followed by 4-cell <td> rows lands ragged in rows, and infer_column_types returns 4 entries based on max_cols — headers then needs synthetic Col 3 / Col 4 labels). All four CHANGELOG entries reference #104 and KaC ordering is preserved.
Verification
| Check | Status |
|---|---|
uv run pytest |
511 passed, 11 deselected, 5 xfailed — matches PR body claim (504 prior + 7 new = 4 _has_header_row + 2 env-var subprocess + 1 padding-loop) |
uv run ruff check src/ tests/ |
clean |
uv run ruff format --check src/ tests/ |
10 files already formatted |
uv run mypy src/ |
clean |
Repo-wide grep for residual cb._backend = / clipboard._backend = direct mutation |
0 hits in tests/ and src/ — both fixtures fully migrated to reset_backend_cache() |
Manual import of reset_backend_cache |
imports cleanly, docstring renders correctly |
| Manual reproduction of env-var ValueError | MCP_CLIPBOARD_MAX_IMAGE_BYTES=ten-megs uv run python -c "import mcp_clipboard.clipboard" raises ValueError: invalid literal for int() with base 10: 'ten-megs' at line 41 of clipboard.py — confirms the subprocess test's assertions are exercising real behavior |
_has_header_row test coverage |
all four added tests target a distinct branch: len(rows) < 2 early return (single + empty), uniform-text negative case, text-over-numeric positive case |
| CI gates | all green (lint, typecheck, test 3.11/3.12/3.13, integration-x11, codecov/patch, qa-approved) |
PR-body checkboxes all flipped.
Applying Ready for QA Signoff as the final act. Awaiting maintainer QA Approved.
|
[QA] Verdict: Ready for QA Signoff. Single round, zero findings. Gates re-verified (511/11/5, ruff/format/mypy clean, CI green); reset_backend_cache migration complete (zero residual direct _backend mutations); env-var ValueError reproduces manually. Awaiting maintainer QA Approved. |
Bump pyproject.toml 2.2.1 -> 2.3.0 and convert the [Unreleased] block into [2.3.0] - 2026-05-02. A fresh empty [Unreleased] section sits above for the next cycle. 13 PRs aggregated since v2.2.1: #88, #92, #93, #94, #95, #96, #98, #99, #100, #101, #102, #103, #104. Tag-push (v2.3.0) after merge triggers .github/workflows/publish.yml. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Summary
Closes the four minor audit items left after PRs #101 and #102. All four are nice-to-haves; none are bugs. Bundled into a single PR per the audit synthesis.
Added
clipboard.reset_backend_cache()public helper. Replaces direct mutation of the module-private_backendglobal in test fixtures (tests/test_server.pyandtests/test_integration_x11.py). Production code has no reason to call this; the helper exists for tests that switch backends or re-readMCP_CLIPBOARD_BACKENDmid-process._has_header_rowtests covering thelen(rows) < 2early-return path, the uniform-text-no-header path, and a positive text-header-over-numeric-data case. Previously exercised only indirectly viaformat_tableJSON output.clipboard_paste(include_schema=True)when the data rows are wider than the header row. Asserts syntheticCol 3/Col 4labels are emitted by the padding loop atserver.py:222-228.MCP_CLIPBOARD_MAX_WRITE_BYTESandMCP_CLIPBOARD_MAX_IMAGE_BYTES. Both vars are parsed viaint(os.environ.get(...))at module import; non-integer values raiseValueErrorbefore anything else runs. Tests exercise each in a subprocess so the partial-import state stays isolated from the in-process module cache.Implementation note
The env-var-validation tests originally tried
importlib.reload()in-process, which broke 21 other tests because theirpytest.raises(ClipboardError)calls compared against the pre-reload class object, while the reloaded module raised the new class. Switched to subprocess-based assertions that capture stderr and verify both theValueErrortraceback and the offending value appear.Test plan
uv run pytest(511 passed, 11 deselected, 5 xfailed)uv run ruff check src/ tests/uv run ruff format --check src/ tests/uv run mypy src/CHANGELOG
## [Unreleased]updated with four### Addedentries referencing test(audit): close the four remaining minor items from the May 1 audit #104.