fix #22: [13] Tests mock the wrong code path — assertions never fail even when code is broken - #46
Merged
Conversation
…even when code is broken test_create_idea_final_turn_conclusion_parsing (and its fallback companion) patched service.groq_client and asserted on groq_client.chat.completions.create, but create_idea never touches the Groq SDK — it drives LangChain's LLMChain.arun (a ChatGroq instance). The patch intercepted nothing, so the assertions could never fail even if the real code broke. The tests also asserted the stale model id "qwen-qwq-32b" and treated create_idea's single-dict return as a list. - Patch langchain.chains.LLMChain.arun with AsyncMock so the tests exercise the real code path; a mutation of models_to_try now fails the test, proving the assertions bite. - Remove the dead groq_client mocks/assertions. - Update model-name expectations to the current "moonshotai/kimi-k2-instruct". - Handle create_idea's single-dict return correctly (not a list). - Remove the dead nested duplicate test_create_idea_final_turn_fallback_parsing defined inside test_create_idea_live_groq_call (never collected). - CI: run the two now-fixed tests (remove their deselects), add a named "Run LLM service final-turn parsing regression" step, and fix the mislabeled deselect comment. Closes #22 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
CuriosityQuantified
added a commit
that referenced
this pull request
Aug 10, 2026
Issue #23 reported a `test_*` function defined *inside* another test function in tests/unit/test_llm_service.py. Pytest only collects module-level functions and Test*-class methods, so the nested copy was never run -- dead code masquerading as fallback-parsing coverage. The literal deletion already landed via PR #46 (commit d275ebf, which closed #22) as a side-effect of the same-file mock fix, so nothing is left to delete. What was still missing is a guard against recurrence, which this PR adds following the repo's per-issue regression convention. - tests/unit/test_nested_test_discovery_regression.py: AST-based, offline (parses, never imports/execs) guard that scans every tests/unit/test_*.py and fails if any `test_*` function is nested inside another function. Class-body scopes reset the enclosing-function context so Test*-class methods stay allowed. Two targeted assertions lock in the exact #23 fix: `test_create_idea_final_turn_fallback_parsing` is defined exactly once at module level and is not nested inside `test_create_idea_live_groq_call`. - .github/workflows/ci.yml: add a named "Run nested test discovery regression" step in the unit-tests job, mirroring the sibling per-issue regression steps. Deselect list and other jobs untouched. Local gates: unit suite 231 passed / 1 skipped / 6 deselected (was 228; +3 new); Next.js `npm run build` green. No graphify-out/ or code-graph CI job in this repo, so graph refresh is not applicable. Closes #23 Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #22
Problem
tests/unit/test_llm_service.py::test_create_idea_final_turn_conclusion_parsing(and its module-level fallback companion) patchedservice.groq_client— a directGroqSDK instance — and assertedservice.groq_client.chat.completions.create.assert_called_once(). Butcreate_ideanever callsself.groq_client; it drives LangChain'sLLMChain.arun()on aChatGroqinstance (services/llm_service.py:313/:318). The patch intercepted nothing, so the assertions could never fail even if the real code broke. The tests also asserted the stale model idqwen-qwq-32b(nowmoonshotai/kimi-k2-instruct) and treatedcreate_idea's single-dict return as a list.Fix
langchain.chains.LLMChain.arunwithAsyncMockso both final-turn tests exercise the real code path. Verified by mutation test: changingmodels_to_trynow fails the test, proving the assertions bite.groq_clientmocks/assertions.moonshotai/kimi-k2-instruct.create_idea's single-dict return correctly (it returns a dict, not a list).test_create_idea_final_turn_fallback_parsingdefined insidetest_create_idea_live_groq_call(never collected by pytest).CI
--deselectlines for the now-fixed llm_service final-turn tests so they run in the general unit suite.Run LLM service final-turn parsing regression(mirrors the existing named-regression pattern).LLMChainAPI will break on LangChain upgrade #24; [15] DeprecatedLLMChainAPI will break on LangChain upgrade #24 is the separate LLMChain→LCEL migration and is untouched here). Kept the R2 and media_utils ([4]/debug/media-checkcrashes withNameErroronMountandStaticFiles#13) deselects.Scope
Only
tests/unit/test_llm_service.pyand.github/workflows/ci.ymlchanged. No LCEL migration or dependency changes (#24's territory). No production code changed.Gates run locally
npm run build(Next.js production build): success.Graphify: N/A — repo has no
.mcp.json,graphify-out/, or code-graph CI job.🤖 Generated with Claude Code