test(dev_tools): refractor-test render step mock is a dead mock #169

New Issue

cal · 2026-04-11T17:21:47Z

cal commented

2026-04-11 17:21:47 +00:00

Context

Surfaced by the pr-reviewer agent during review of PR #168
(fix/refractor-test-unified-cards-lookup).

Problem

tests/test_dev_tools.py::TestRefractorTestExecute::test_successful_batter_flow
patches db_get as a way of stubbing the card-render step, but the
actual render call in cogs/dev_tools.py uses aiohttp.ClientSession
directly, not db_get. So the test's mock_get.return_value for the
render step is never exercised — the test passes because the aiohttp
call raises and hits the non-fatal except branch that records a
warning ("⚠️ Card render failed (non-fatal): ...") instead of the
happy-path "✅ Card rendered + S3 upload triggered".

This means the happy-path render assertion is silently skipped, and a
future regression that broke the render chain would not be caught by
this test.

Fix

Patch aiohttp.ClientSession (or the full render call path) in
test_successful_batter_flow so the test exercises the real 200-OK
code path, then assert the "✅ Card rendered + S3 upload triggered"
line appears in the results.

Notes

This weakness is pre-existing — it was not introduced by PR #168.
PR #168 is scoped to the card-lookup fix; this cleanup belongs in
a separate PR.
Severity: Low — the test suite still catches the common failure
modes of the refractor chain; only the happy-path render branch
is unexercised.

## Context Surfaced by the pr-reviewer agent during review of PR #168 (fix/refractor-test-unified-cards-lookup). ## Problem `tests/test_dev_tools.py::TestRefractorTestExecute::test_successful_batter_flow` patches `db_get` as a way of stubbing the card-render step, but the actual render call in `cogs/dev_tools.py` uses `aiohttp.ClientSession` directly, not `db_get`. So the test's `mock_get.return_value` for the render step is never exercised — the test passes because the aiohttp call raises and hits the non-fatal `except` branch that records a warning ("⚠️ Card render failed (non-fatal): ...") instead of the happy-path "✅ Card rendered + S3 upload triggered". This means the happy-path render assertion is silently skipped, and a future regression that broke the render chain would not be caught by this test. ## Fix Patch `aiohttp.ClientSession` (or the full render call path) in `test_successful_batter_flow` so the test exercises the real 200-OK code path, then assert the "✅ Card rendered + S3 upload triggered" line appears in the results. ## Notes - This weakness is **pre-existing** — it was not introduced by PR #168. PR #168 is scoped to the card-lookup fix; this cleanup belongs in a separate PR. - Severity: Low — the test suite still catches the common failure modes of the refractor chain; only the happy-path render branch is unexercised.

cal added the

tests

tech-debt

labels 2026-04-11 17:21:47 +00:00

cal referenced this issue

2026-04-11 17:22:18 +00:00

fix(dev_tools): use unified cards/{id} endpoint for /dev refractor-test #168

Claude added the

ai-working

label 2026-04-11 17:31:14 +00:00

cal referenced this issue from a commit

2026-04-11 17:33:17 +00:00

test(dev_tools): fix dead render mock in test_successful_batter_flow (#169)

Claude referenced a pull request that will close this issue

2026-04-11 17:33:29 +00:00

test(dev_tools): fix dead render mock in test_successful_batter_flow (#169) #170

Claude removed the

ai-working

label 2026-04-11 17:33:35 +00:00

Claude commented

2026-04-11 17:33:37 +00:00

PR #170 fixes this. Replaced the dead db_get mock with a proper aiohttp.ClientSession mock (explicit MagicMock with __aenter__/__aexit__ so sess.get(url) returns an async context manager, not a coroutine). Render step now exercises the 200-OK path and asserts "✅ Card rendered + S3 upload triggered" in results. 14/14 tests pass.

PR #170 fixes this. Replaced the dead `db_get` mock with a proper `aiohttp.ClientSession` mock (explicit `MagicMock` with `__aenter__`/`__aexit__` so `sess.get(url)` returns an async context manager, not a coroutine). Render step now exercises the 200-OK path and asserts `"✅ Card rendered + S3 upload triggered"` in results. 14/14 tests pass.

Claude added the

ai-pr-opened

label 2026-04-11 17:33:40 +00:00

Sign in to join this conversation.