Add capacity status edge fixture#79
Conversation
Reviewer's GuideAdds a deterministic CLI test fixture for File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- The new CLI capacity test heavily monkeypatches global functions (e.g.,
load_catalog,QuotaStore,effective_env); consider extracting a small helper or fixture to construct this synthetic environment so future changes to these APIs don’t require updating multiple inline monkeypatch calls. - In
upsert_config_key, usinggetattr(os, "fchmod", None)is a good Windows guard, but you might want to centralize this permission handling (e.g., a small helper that documents the platform-specific behavior) so other code paths don’t accidentally assume POSIX semantics in the future.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The new CLI capacity test heavily monkeypatches global functions (e.g., `load_catalog`, `QuotaStore`, `effective_env`); consider extracting a small helper or fixture to construct this synthetic environment so future changes to these APIs don’t require updating multiple inline monkeypatch calls.
- In `upsert_config_key`, using `getattr(os, "fchmod", None)` is a good Windows guard, but you might want to centralize this permission handling (e.g., a small helper that documents the platform-specific behavior) so other code paths don’t accidentally assume POSIX semantics in the future.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
|
Addressed the Sourcery review cleanup in
Fresh validation on the PR branch:
|
|
Updated the branch with current Fresh validation on the updated head:
I also attempted the repo coverage command locally on Windows. The coverage threshold script passed, but the full |
Summary
capacity status --all --no-catalog-syncCLI fixture for issue Add a capacity-status fixture for quota edge cases #54.tmp_path/ monkeypatches so local developer credentials cannot affect the result.os.fchmodinupsert_config_key; the focused Windows test command otherwise crashed in unrelatedkeys addtests before this PR's fixture could be validated. POSIX still enforces0600; Windows asserts successful write/content without pretending POSIX mode bits apply.Closes #54.
Validation
Passed:
PYTHONPATH=src python -m pytest tests/test_cli.py::test_cli_capacity_status_all_reports_quota_edges_without_local_state -qPYTHONPATH=src python -m pytest tests/test_capacity.py tests/test_cli.py -qPYTHONPATH=src python -m pytest tests/test_capacity.py tests/test_cli.py tests/test_key_inventory.py tests/test_robustness.py -qPYTHONPATH=src python -m ruff check .Full coverage gate attempted but not green in this Windows checkout:
tests/test_metaswarm_integration.pycaused by the Windows checkout materializingintegrations/metaswarm/freellmpool-review-adapter.shwith CRLF, producing/usr/bin/env: bash\randcase ... in\rshell parse failures.I left those metaswarm line-ending policy issues untouched to keep this PR focused.
Summary by Sourcery
Add a deterministic CLI capacity status fixture that isolates local state and adjust key config permissions handling for cross-platform compatibility.
New Features:
capacity status --all --no-catalog-syncacross keyless, low-quota, and missing-key providers without relying on local configuration.Bug Fixes:
os.fchmodby guarding key config permission changes inupsert_config_key.Tests:
os.fchmodis available, aligning expectations with platform behavior.