Fix CI lint gate and restore >95% test coverage#21
Conversation
|
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Sorry @dr-gareth-roberts, your pull request is larger than the review limit of 150000 diff characters
|
Related Documentation Checked 15 published document(s) in 1 knowledge base(s). No updates required. How did I do? Any feedback? |
📝 WalkthroughWalkthroughThis PR refactors 30+ test files with cosmetic formatting changes (multi-line reflowing, consolidating with-statements, removing unused variable assignments) and updates the CI workflow to add PyYAML/pytest dependencies and adjust the test environment configuration. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
Code Review SummaryStatus: No Issues Found | Recommendation: Merge Files Reviewed (36 files)
|
There was a problem hiding this comment.
Pull request overview
This PR updates the test suite to satisfy the CI lint/format gates (Ruff) while keeping measured coverage above the configured 95% threshold, primarily by reformatting coverage-focused tests and removing unused locals that were tripping lint.
Changes:
- Reformat many test files to satisfy
ruff format --check .(line wrapping,with (...)blocks, etc.). - Remove/avoid unused local variables in tests so
ruff check .passes. - Minor test refactors to preserve behavior/coverage while meeting lint requirements.
Reviewed changes
Copilot reviewed 36 out of 36 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/test_trace_config_branch_coverage.py | Reformat monkeypatch calls for Ruff formatting compliance. |
| tests/test_template_versioning_branch_coverage.py | Reformat helper construction; remove unused local. |
| tests/test_structured_extraction_branch_coverage.py | Reformat schema construction and monkeypatch lambdas. |
| tests/test_stability_contract.py | Reformat generator expression for style compliance. |
| tests/test_semantic_obs_coverage.py | Import block reordering + extensive formatting changes; remove unused locals. |
| tests/test_runtime_results_coverage.py | Reformat calls/collections; minor import reordering. |
| tests/test_runtime_reproducibility_branch_coverage.py | Reformat monkeypatch call and long test signature. |
| tests/test_runtime_high_level_branch_coverage.py | Reformat monkeypatch calls and list literals. |
| tests/test_runtime_config_loader_branch_coverage.py | Reformat callsite to single line per formatter. |
| tests/test_routing_branch_coverage.py | Replace lambda assignment with def for style/clarity. |
| tests/test_rate_limiting_branch_coverage.py | Reformat multi-patch context managers using with (...). |
| tests/test_probes_models_coverage.py | Reformat long strings/assertions; remove unused locals. |
| tests/test_orchestration_branch_coverage.py | Reformat long any(...) assert across lines. |
| tests/test_models_config_coverage.py | Insert blank lines / reorder imports to match formatter output. |
| tests/test_misc_coverage.py | Reformat long constructor calls; minor import placement tweaks. |
| tests/test_latency_branch_coverage.py | Reformat function signature and long callsite. |
| tests/test_langchain_integration_coverage.py | Reformat list literals passed to assertions/calls. |
| tests/test_hitl_branch_coverage.py | Reformat monkeypatch.setattr with multi-line args. |
| tests/test_experiment_tracking_coverage.py | Reformat multi-patch context managers using with (...). |
| tests/test_evaluation_branch_coverage.py | Reformat long object construction calls. |
| tests/test_deployment_coverage.py | Reformat config constructions and client.post calls. |
| tests/test_cost_tracking_branch_coverage.py | Reformat nested dict literals. |
| tests/test_context_window_branch_coverage.py | Reformat helper signature and long method call. |
| tests/test_cli_validate_benchmark_branches.py | Reformat multi-patch blocks and long write_text calls. |
| tests/test_cli_schema_coverage.py | Remove unused captured locals; reformat JSON literals. |
| tests/test_cli_run_command_coverage.py | Reformat write_text and multi-patch blocks; reflow list comprehension. |
| tests/test_cli_remaining_coverage.py | Remove unused captured locals in capsys tests. |
| tests/test_cli_misc_command_branches.py | Reformat long patch/call expressions and multi-patch blocks. |
| tests/test_cli_interactive_coverage.py | Remove unused captured locals; reformat patch usage. |
| tests/test_cli_doctor_coverage.py | Import reordering + remove unused locals (but dropped rc assertions in a few tests). |
| tests/test_cli_diff_harness_coverage.py | Reformat list literals and long arg construction. |
| tests/test_cli_compare_coverage.py | Reformat long cmd_compare calls. |
| tests/test_cli_benchmark_coverage.py | Reformat nested patch blocks and long calls. |
| tests/test_caching_unified_branch_coverage.py | Reformat long test signature. |
| tests/test_adapters_export_coverage.py | Remove unused locals; reformat list construction. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| def test_text_format_with_fail_on_warn(self, capsys): | ||
| rc = cmd_doctor(_make_args(format="text", fail_on_warn=True)) | ||
| cmd_doctor(_make_args(format="text", fail_on_warn=True)) | ||
| captured = capsys.readouterr() | ||
| assert "insideLLMs Doctor" in captured.out | ||
|
|
There was a problem hiding this comment.
cmd_doctor(...) return value is no longer asserted here. This weakens the test and also loses a cheap sanity check that the command exits successfully (or with the expected nonzero code). Capture the return code (e.g., rc = ...) and assert it is in the expected set (typically 0 or 1 when fail_on_warn=True).
| def test_checks_structure(self, capsys): | ||
| rc = cmd_doctor(_make_args(format="json")) | ||
| cmd_doctor(_make_args(format="json")) | ||
| captured = capsys.readouterr() | ||
| data = json.loads(captured.out) |
There was a problem hiding this comment.
This test no longer checks the return code from cmd_doctor. Please capture the return value and assert it’s an int with an expected value (at least rc in (0, 1)), so the test still validates command success/failure behavior rather than only stdout structure.
| def test_all_check_categories(self, capsys): | ||
| rc = cmd_doctor(_make_args(format="json")) | ||
| cmd_doctor(_make_args(format="json")) | ||
| captured = capsys.readouterr() | ||
| data = json.loads(captured.out) |
There was a problem hiding this comment.
Same as above: the return code from cmd_doctor is ignored. Capture and assert it (even a loose rc in (0, 1) is better than dropping it) to keep this test validating the CLI contract in addition to output content.
| import argparse | ||
| import json | ||
| from unittest.mock import patch, MagicMock | ||
| from unittest.mock import MagicMock, patch |
There was a problem hiding this comment.
Import of 'MagicMock' is not used.
Import of 'patch' is not used.
| from unittest.mock import MagicMock, patch |
Code reviewNo issues found. Checked for bugs and CLAUDE.md compliance. |
2 similar comments
Code reviewNo issues found. Checked for bugs and CLAUDE.md compliance. |
Code reviewNo issues found. Checked for bugs and CLAUDE.md compliance. |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
1 participant
Summary
ruff format --check .passesVerification
ruff check .ruff format --check .pytest --cov=insideLLMs --cov-report=term --cov-fail-under=95Summary by CodeRabbit
Chores
Tests