test: expand coverage and harden regression checks - #12
Conversation
📝 WalkthroughWalkthroughExpanded regression coverage for malformed reference data, air-quality and weather parsing, geocoding providers, caching, configuration, service validation, datetime parsing, and precision-reduced location alerts. ChangesRobustness Regression Coverage
Estimated code review effort: 4 (Complex) | ~45 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #12 +/- ##
==========================================
+ Coverage 94.60% 99.17% +4.56%
==========================================
Files 34 34
Lines 3170 3862 +692
Branches 201 232 +31
==========================================
+ Hits 2999 3830 +831
+ Misses 123 21 -102
+ Partials 48 11 -37 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
This PR expands the automated test suite across multiple subsystems (weather context providers, air quality, CLI wiring, configuration validation, geocoding behavior, reference data loading, time parsing, and service behavior) to better exercise validation/guardrail paths and to harden previously flaky regression checks.
Changes:
- Added comprehensive negative/edge-case tests for QWeather/Open-Meteo weather context and AQICN air quality parsing/validation.
- Hardened determinism in tests by isolating monotonic timing for Nominatim rate-limit behavior and making the unknown-feed forecast regression behavior-sensitive.
- Expanded coverage for configuration and time utility parsing error paths, plus CLI provider-building and run/daemon behavior.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| tests/test_weather_context.py | Adds extensive coverage for QWeather/Open-Meteo success and failure paths, plus snapshot document conversion behavior. |
| tests/test_time_utils.py | Adds tests ensuring parsing helpers reject non-datetime parse results. |
| tests/test_service.py | Adds service-level regression tests (briefing size validation, unknown-feed forecast behavior). |
| tests/test_reference_data.py | Adds coverage for invalid reference data root types and cache clearing behavior. |
| tests/test_geocoding.py | Adds coverage for Open-Meteo API key passing, Nominatim behaviors (rate limiting, invalid responses), precision reduction, caching, and mainland-China reference rules validation. |
| tests/test_config.py | Adds configuration validation test for non-numeric coordinates in locations file. |
| tests/test_cli.py | Adds tests for CLI construction helpers, weather provider selection/config validation, run/daemon behavior, and alerting on precision reduction. |
| tests/test_air_quality.py | Adds AQICN observed-at edge cases and validates air-quality guidance/reference-data validation behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
tests/test_weather_context.py (1)
556-589: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract the repeated QWeather payload boilerplate into a shared fixture/helper.
The same ~20-line weather-forecast JSON blob and
{"code": "200", "daily": []}indices payload are copy-pasted verbatim across roughly a dozen new tests in this range. A small helper (e.g._qweather_weather_response(fx_link=None)/ a module-level constant) that each handler reuses would cut this duplication drastically and make future payload-shape changes a one-line edit instead of a dozen.♻️ Example helper
_QWEATHER_DAILY_ITEM = { "fxDate": "2026-07-13", "textDay": "晴", "textNight": "晴", "tempMin": "20", "tempMax": "30", "windDirDay": "南风", "windScaleDay": "3-4", "humidity": "60", "precip": "0.0", } def _qweather_weather_response(*, fx_link: str | None = None) -> httpx.Response: payload = {"code": "200", "updateTime": "2026-07-13T08:00", "daily": [_QWEATHER_DAILY_ITEM]} if fx_link: payload["fxLink"] = fx_link return httpx.Response(200, json=payload)Also applies to: 717-755, 758-816, 818-859, 898-932, 935-985, 988-1043, 1046-1103, 1106-1163, 1166-1223, 1226-1283
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/test_weather_context.py` around lines 556 - 589, Extract the duplicated QWeather forecast payload and successful indices payload used across the affected tests into module-level shared fixtures or helpers, such as a daily-item constant and weather-response helper supporting optional fxLink. Update each handler in the listed test ranges, including test_qweather_rejects_non_success_indices_status, to reuse them while preserving each test’s request-specific behavior and response status.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@tests/test_weather_context.py`:
- Around line 556-589: Extract the duplicated QWeather forecast payload and
successful indices payload used across the affected tests into module-level
shared fixtures or helpers, such as a daily-item constant and weather-response
helper supporting optional fxLink. Update each handler in the listed test
ranges, including test_qweather_rejects_non_success_indices_status, to reuse
them while preserving each test’s request-specific behavior and response status.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 239c46eb-61d8-4e2e-ad42-5a6a41a1cf93
📒 Files selected for processing (8)
tests/test_air_quality.pytests/test_cli.pytests/test_config.pytests/test_geocoding.pytests/test_reference_data.pytests/test_service.pytests/test_time_utils.pytests/test_weather_context.py
Summary
Why
The added coverage exercises remaining validation and guardrail paths. Review found that one service test could pass without proving the intended forecast classification and that one timing assertion depended on real scheduler latency. These changes make both tests behavior-sensitive and deterministic.
Validation
301 passedruff check .prek run --all-filesgit diff --checkSummary by CodeRabbit