test: stop stubbing discovery_inquiry_flow, restore real coverage - #63
Open
garvitkaushik-123 wants to merge 1 commit into
Open
test: stop stubbing discovery_inquiry_flow, restore real coverage#63garvitkaushik-123 wants to merge 1 commit into
garvitkaushik-123 wants to merge 1 commit into
Conversation
…sue IABTechLab#60 part 1) The "pre-existing @listen() bugs with CrewAI version mismatch" stub that every ad_seller.flows-touching test file carried for discovery_inquiry_flow dates to when crewai was pinned at >=0.86.0 (commit 530df34, March 2026). crewai is >=1.14.4 now (resolves to 1.15.2) -- a major version bump, and the flow runs cleanly end to end on it: verified DiscoveryInquiryFlow.kickoff_async() across all four routing branches, both standalone and under pytest, and confirmed the production call path (the sync flow.query() wrapper -- which calls self.kickoff(), not kickoff_async() -- invoked from inside an async FastAPI handler) also works correctly. atc964 confirmed on the issue that the rationale was real at the time and just never got revisited as crewai moved forward. This wasn't just stale test hygiene: DiscoveryInquiryFlow backs a live endpoint (POST /discovery, products.py:201), so the blanket stub meant that endpoint had been running in production with zero real test coverage for as long as the stub existed. jaanijuk caught that on the issue -- grep for "DiscoveryInquiryFlow" in tests/ before this change returns nothing. execution_activation_flow stays stubbed. It has a separate, real bug (a cancel-scope leak in an MCP client cleanup path on ad-server connection failure, landing outside the code's own try/except) that's being tracked and fixed independently as issue IABTechLab#60 part 2 -- it just isn't the @listen() bug the old comment claimed either. Changes: - Removed the discovery_inquiry_flow entry from the _broken_flows stub list in all 33 test files that had it (execution_activation_flow entries are untouched). - Simplified test_deal_flow_e2e.py: its _get_deal_request_flow_class() helper existed solely to bypass flows/__init__.py via manual sys.modules surgery so importing DealRequestFlow wouldn't also trigger discovery_inquiry_flow. With the stub gone that's unnecessary -- replaced with a plain module import. - Fixed the stale docstring in test_linear_tv.py making the same claim. - Added tests/unit/test_discovery_inquiry_flow.py: the flow's own routing logic across all four response types run for real (no stub), plus POST /discovery exercised through the actual FastAPI app with the real flow (not mocked) -- closing the exact coverage gap this issue found. Full suite: 1491 passed, 28 skipped (pre-existing, unrelated), no regressions. ruff check / format clean. Part of IABTechLab#60.
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.
Summary
Part 1 of #60 (green-lit by @atc964 on the issue).
The
"pre-existing @listen() bugs with CrewAI version mismatch"stub that everyad_seller.flows-touching test file carried fordiscovery_inquiry_flowdates to whencrewaiwas pinned at>=0.86.0(commit530df34, March 2026). It's>=1.14.4now (resolves to1.15.2) — a major version bump. Verified on the issue that the flow runs cleanly end-to-end on the current version, including the exact production call path (flow.query()'s syncself.kickoff()invoked from inside an async FastAPI handler).This wasn't just stale test hygiene —
DiscoveryInquiryFlowbacks a live endpoint (POST /discovery,products.py:201), so the blanket stub meant that endpoint had zero real test coverage for as long as the stub existed (grep -rl "DiscoveryInquiryFlow" tests/returned nothing before this PR).execution_activation_flowstays stubbed — it has a separate, real bug (a cancel-scope leak in an MCP client cleanup path, not the@listen()issue the old comment claimed) that's tracked as #60 part 2 and will be fixed independently.Changes
discovery_inquiry_flowentry from the_broken_flowsstub list in all 33 test files that had it.execution_activation_flowentries are untouched.test_deal_flow_e2e.py: its_get_deal_request_flow_class()helper existed solely to bypassflows/__init__.pyvia manualsys.modulessurgery so importingDealRequestFlowwouldn't also triggerdiscovery_inquiry_flow. With the stub gone that's unnecessary — replaced with a plain module import (net -41 lines in that file alone).test_linear_tv.pymaking the same claim.tests/unit/test_discovery_inquiry_flow.py: the flow's own routing logic across all four response types (catalog/pricing/availability/targeting) run for real, plusPOST /discoveryexercised through the actual FastAPI app with the real flow — not mocked — closing the exact coverage gap the issue found.Test plan
Full suite: 1491 passed, 28 skipped (pre-existing, unrelated to this change), no regressions.
ruff check/format --checkclean.Part of #60.