Skip to content

test: cover _check_memory_health threshold branches#1262

Merged
muddlebee merged 1 commit intoTracer-Cloud:mainfrom
kespineira:issue/1156-memory-health-threshold-tests
May 4, 2026
Merged

test: cover _check_memory_health threshold branches#1262
muddlebee merged 1 commit intoTracer-Cloud:mainfrom
kespineira:issue/1156-memory-health-threshold-tests

Conversation

@kespineira
Copy link
Copy Markdown
Contributor

Fixes #1156

Describe the changes you have made in this PR -

Added deterministic unit coverage for the remaining _check_memory_health() cases not handled by #1212:

  • memory usage below 90% returns passed
  • memory usage at the 90% warning threshold returns warn
  • missing MemTotal returns missing
  • meminfo parsing ignores non-key/value noise lines without touching the real /proc/meminfo

No production code changes were needed.

Demo/Screenshot for feature changes and bug fixes -

Terminal validation:

uv run python -m pytest tests/remote/test_server.py -q --cov=app.remote.server --cov-report=term-missing
# 43 passed; _check_memory_health parsing/threshold lines covered

make lint
# All checks passed

make format-check
# 1065 files already formatted

make typecheck
# Success: no issues found in 456 source files

env PATH=".venv/bin:/usr/bin:/bin:/usr/sbin:/sbin" make test-cov
# 4866 passed, 3 skipped

The adjusted PATH makes the local authenticated codex binary unavailable so the Codex CLI smoke test follows its intended skip path.


Code Understanding and AI Usage

Did you use AI assistance (ChatGPT, Claude, Copilot, etc.) to write any part of this code?

  • No, I wrote all the code myself
  • Yes, I used AI assistance (continue below)

If you used AI assistance:

  • I have reviewed every single line of the AI-generated code
  • I can explain the purpose and logic of each function/component I added
  • I have tested edge cases and understand how the code handles them
  • I have modified the AI output to follow this project's coding standards and conventions

Explain your implementation approach:

#1212 covered the missing-file, incomplete-MemAvailable, and OSError branches. This PR covers the remaining threshold behavior by replacing app.remote.server.Path with small per-test fake path classes. That keeps the tests deterministic and prevents reads from the host /proc/meminfo.

The fixtures use fixed MemTotal and MemAvailable values so the expected percentages are clear: 50% for the passed branch and exactly 90% for the warn branch. I also added a MemTotal-missing case because the issue explicitly calls out both required meminfo fields.


Checklist before requesting a review

  • I have added proper PR title and linked to the issue
  • I have performed a self-review of my code
  • I can explain the purpose of every function, class, and logic block I added
  • I understand why my changes work and have tested them thoroughly
  • I have considered potential edge cases and how my code handles them
  • If it is a core feature, I have added thorough tests
  • My code follows the project's style guidelines and conventions

Note: Please check Allow edits from maintainers if you would like us to assist in the PR.

@kespineira kespineira changed the title [codex] test memory health threshold branches test: cover _check_memory_health threshold branches May 4, 2026
@kespineira kespineira marked this pull request as ready for review May 4, 2026 10:34
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented May 4, 2026

Greptile Summary

Adds four deterministic unit tests for _check_memory_health() covering the passed branch (50% usage), warn boundary (exactly 90% usage), and missing when MemTotal is absent — complementing the OSError, absent-file, and absent-MemAvailable branches from #1212. All maths (int(((total_kb - avail_kb) / total_kb) * 100)) and detail-string format are verified against the production implementation, and the fake Path classes correctly absorb the encoding="utf-8" keyword arg passed by read_text.

Confidence Score: 5/5

Safe to merge — test-only changes with no production code modifications.

All four new tests are arithmetically correct against the production implementation, fake Path classes properly handle the encoding keyword arg, and the warn-boundary case (exactly 90%) is correctly validated. No production code is changed.

No files require special attention.

Important Files Changed

Filename Overview
tests/remote/test_server.py Adds four well-structured _check_memory_health tests with deterministic fake Path classes; all assertions match production logic exactly.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[_check_memory_health] --> B{/proc/meminfo\nexists?}
    B -- No --> C[status: missing\n'unavailable on this platform']
    B -- Yes --> D[read_text encoding=utf-8]
    D -- OSError --> E[status: missing\n'Unable to read meminfo']
    D -- success --> F[parse key:value lines\nskip lines without ':']
    F --> G{MemTotal and\nMemAvailable\npresent?}
    G -- No --> H[status: missing\n'Incomplete /proc/meminfo data.']
    G -- Yes --> I[used_pct = int\ntotal - avail / total * 100]
    I --> J{used_pct < 90?}
    J -- Yes --> K[status: passed\n✅ NEW TEST: 50% usage]
    J -- No --> L[status: warn\n✅ NEW TEST: 90% usage boundary]
    H -.->|NEW TEST| M[MemTotal absent]
    H -.->|Existing test| N[MemAvailable absent]
Loading

Reviews (1): Last reviewed commit: "test: cover memory health thresholds" | Re-trigger Greptile

@muddlebee muddlebee merged commit d111ee8 into Tracer-Cloud:main May 4, 2026
10 checks passed
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 4, 2026

💜 One more reason the project grows. Thanks @kespineira — your contribution just landed!


👋 Join us on Discord - OpenSRE : hang out, contribute, or hunt for features and issues. Everyone's welcome.

@kespineira kespineira deleted the issue/1156-memory-health-threshold-tests branch May 4, 2026 13:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[IMPROVEMENT] Add _check_memory_health() threshold and error-path tests

2 participants