Skip to content

v0.2.33: fix: keep the test suite hermetic

Choose a tag to compare

@OrenAshkenazy OrenAshkenazy released this 06 Aug 19:33
· 14 commits to main since this release

The re-review cap check in request_rereview.py calls gh_login() and
count_agent_pings(), which shell out to the real gh. Fourteen tests reach
that path without stubbing it.

Where gh is absent (CI) the lookup fails, the cap degrades to permissive, and
the suite is green — so the leak is invisible. Where gh is authenticated the
suite made live API calls against a real repo, and on a PR that had already used
its cap the write was refused, failing test_custom_phrase_is_preserved_exactly.
It also read and wrote ~/.config/gh-gemini-review-loop/.

Changes

  • Two autouse conftest.py fixtures: one raises if any test invokes the real
    gh, one points GGRL_STATE_DIR at a tmp dir.
  • gh_login / count_agent_pings / post_rereview resolve the subprocess
    runner at call time (runner or subprocess.run) instead of binding
    subprocess.run as a default argument, so monkeypatching works.
  • gh_login stubbed in three test modules.
  • The sibling sweep no longer matches comment-only lines.

Verification

A normal run cannot show this, because the leak is silent when gh is missing
and masked when the developer's own config lifts the cap. Both halves were
verified against the pre-patch tree under identical conditions.

1. Fake gh first on PATH — echoes a login for api user, and five prior
pings for any comments call (the local preferences.json sets
max_rereview_requests: 5, so five is what trips the cap):

tree result real gh invocations
pre-patch 1 failed, 755 passed — test_custom_phrase_is_preserved_exactly, KeyError: 'phrase' (write refused, post_rereview never reached) 2+
this branch 769 passed 0

The zero is the load-bearing number: the guard fixture means no test reaches
gh at all, whatever it would have returned.

2. HOME set to an empty tmp dir:

tree files left in HOME
pre-patch 1 — .config/gh-gemini-review-loop/preferences.json
this branch 0 (not even a directory)

Gates: pytest -q → 769 passed. ruff check plugins/ tests/ evals/ → clean.

Summary by Sourcery

Enforce hermetic behavior for the gh-review-loop test suite and tighten sibling sweep behavior to ignore comment-only lines.

New Features:

  • Classify code versus comment lines in the sibling sweeper and expose a helper for this classification.

Bug Fixes:

  • Prevent comment-only lines from being proposed as sibling candidates or used as flagged source when sweeping.
  • Ensure the re-review cap check is inert in tests that are not explicitly about cap enforcement so they do not hit live gh APIs.

Enhancements:

  • Add global test fixtures to forbid real gh subprocess calls and to isolate per-user state under a temporary directory.
  • Resolve subprocess runners for gh-related helpers at call time to allow effective monkeypatching in tests.

Tests:

  • Add focused tests verifying comment-line classification and sweep behavior when flagged sites point at comments.
  • Add a self-check test that asserts the hermetic guard intercepts attempts to shell out to gh.
  • Extend request re-review, JSON stdout discipline, and acceptance cycle tests with fixtures that neutralize the cap check by default.