Fix/check disk health tests#1155
Conversation
Greptile SummaryAdds two deterministic unit tests for Confidence Score: 5/5Safe to merge — all findings are P2 style suggestions that do not affect correctness or runtime behavior. The new tests are logically correct and cover the two primary threshold branches of _check_disk_health(). Remaining feedback (duplicate comment, pre-patch filesystem call, missing "missing" branch test) are all non-blocking P2 suggestions. No files require special attention. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[_check_disk_health called] --> B[shutil.disk_usage '/']
B --> C{usage.total == 0?}
C -- yes --> D[status = 'missing']
C -- no --> E[used_pct = used/total * 100]
E --> F{used_pct < 85?}
F -- yes --> G[status = 'passed']
F -- no --> H[status = 'warn']
G --> I[return DeepHealthCheck]
H --> I
D --> I
style G fill:#2ecc71,color:#fff
style H fill:#f39c12,color:#fff
style D fill:#95a5a6,color:#fff
subgraph Tests Added
T1[test_passed_when_below_warn_threshold - 50 GiB / 100 GiB = 50% covered]
T2[test_warn_when_at_or_above_threshold - 90 GiB / 100 GiB = 90% covered]
T3[No test for total==0 branch]
end
G -.covered by.- T1
H -.covered by.- T2
D -.not covered.- T3
Reviews (1): Last reviewed commit: "chore: remove personal notes from featur..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
VaibhavUpreti
left a comment
VaibhavUpreti
left a comment
There was a problem hiding this comment.
awesome @Bhavarth7 , thanks a lot for your contribution!
|
🎊 Achievement unlocked: PR Merged. @Bhavarth7 passed code review, survived CI, and shipped. Respect. 🤝
👋 Join us on Discord - OpenSRE : hang out, contribute, or hunt for features and issues. Everyone's welcome. |
2 participants
Fixes #1124
Describe the changes you have made in this PR -
Added deterministic tests for the
_check_disk_health()threshold logic intests/app/remote/test_server.py.Previously, both the passed and warn branches had no test coverage. Since the function relies on
shutil.disk_usage("/"), tests would depend on the actual machine disk state, making them non-deterministic.This PR introduces controlled tests using monkeypatching to ensure consistent behavior:
status == "passed"status == "warn"To avoid dependency on real system state,
shutil.disk_usageis monkeypatched to return a synthetic usage object constructed using the same named tuple type as the real return value.Demo/Screenshot for feature changes and bug fixes -
All tests pass consistently regardless of actual disk state.
Code Understanding and AI Usage
Did you use AI assistance (ChatGPT, Claude, Copilot, etc.) to write any part of this code?
If you used AI assistance:
Explain your implementation approach:
The goal was to make the tests deterministic and independent of the host machine.
The main challenge was that
_check_disk_health()reads real disk usage usingshutil.disk_usage("/"), which makes direct testing unreliable.I used
monkeypatchto overrideshutil.disk_usageand return controlled values.Instead of incorrectly trying to instantiate
shutil.disk_usage.__class__, I derived the correct return type usingtype(shutil.disk_usage("/"))and constructed synthetic usage values.Two focused tests were added to cover:
This approach ensures:
Checklist before requesting a review
Note: Maintainers are welcome to suggest improvements or edits if needed.