Skip to content

Commit 4c5c80e

Browse files
committed
test: harden plan-still-loads-state guards
The previous assertion `call.kwargs.get("load_state", True)` silently passes if the `load_state=load_state` line is removed from cli/main.py entirely - the kwarg becomes absent, defaults to True, assertion holds. Combined with the earlier point that `mock.called` alone is too loose (because `context.plan(...)` touches state regardless of load_state), the right guard checks three complementary things: - "load_state" is in call.kwargs (catches kwarg deletion) - the value is True (catches `plan` being added to the tuple) - the patched get_versions was actually called (catches `plan` being added to the tuple from the other direction) Both regression modes verified to fail loudly with the new asserts. Coding-Agent: pi Model: anthropic/claude-opus-4-7 Signed-off-by: Joe Hartshorn <8881940+j-hartshorn@users.noreply.github.com>
1 parent 95d2f9d commit 4c5c80e

1 file changed

Lines changed: 21 additions & 9 deletions

File tree

tests/cli/test_cli.py

Lines changed: 21 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2272,24 +2272,36 @@ def test_lint_runs_without_state(runner: CliRunner, tmp_path: Path, mocker):
22722272

22732273

22742274
def test_plan_still_loads_state(runner: CliRunner, tmp_path: Path, mocker):
2275-
"""Guard-rail: confirm `plan` is not in LOCAL_ONLY_COMMANDS by checking
2276-
the Context constructor received load_state=True for it.
2275+
"""Guard-rail: confirm `plan` is not in LOCAL_ONLY_COMMANDS.
2276+
2277+
Asserts two complementary things, because either alone has a blind spot:
2278+
1. The CLI explicitly passes `load_state=True` to `Context(...)` for `plan`.
2279+
Checking `"load_state" in call.kwargs` (not just the value) catches a
2280+
regression where someone deletes the `load_state=load_state` line in
2281+
cli/main.py entirely — the kwarg would be absent and silently default
2282+
to True without this check.
2283+
2. State sync was actually accessed. Catches a regression where someone
2284+
adds `"plan"` to LOCAL_ONLY_COMMANDS — `load_state` would be False and
2285+
the patched method would never be called.
22772286
22782287
The `plan` invocation is expected to fail because state access is patched
22792288
to raise. We don't assert on exit_code; mocker.spy records the constructor
2280-
call before the wrapped __init__ runs, so the kwargs assertion holds
2281-
regardless of whether the call ultimately raised.
2289+
call before the wrapped __init__ runs, so kwargs are recorded regardless.
22822290
"""
2283-
_setup_local_only_project(tmp_path, mocker)
2291+
mock = _setup_local_only_project(tmp_path, mocker)
22842292
init_spy = mocker.spy(Context, "__init__")
22852293

22862294
runner.invoke(cli, ["--paths", str(tmp_path), "plan"], input="n\n")
22872295

22882296
assert init_spy.called, "Context was never constructed"
2289-
load_state_values = [call.kwargs.get("load_state", True) for call in init_spy.call_args_list]
2290-
assert all(load_state_values), (
2291-
f"Context was constructed with load_state=False for `plan`: {load_state_values}"
2292-
)
2297+
for call in init_spy.call_args_list:
2298+
assert "load_state" in call.kwargs, (
2299+
"CLI didn't pass load_state= explicitly; missing kwarg defaults to True silently"
2300+
)
2301+
assert call.kwargs["load_state"] is True, (
2302+
f"Context was constructed with load_state={call.kwargs['load_state']} for `plan`"
2303+
)
2304+
assert mock.called, "state-sync was never accessed during `plan`"
22932305

22942306

22952307
def test_format_runs_without_state_credentials(

0 commit comments

Comments
 (0)