From 568afdd293536ee49c521f2fb744ac86fcc6e287 Mon Sep 17 00:00:00 2001 From: Tao Chen Date: Mon, 8 Jun 2026 16:30:59 -0700 Subject: [PATCH] Fix tests and address comments --- .../agent_framework/_workflows/_runner.py | 2 +- .../agent_framework/_workflows/_workflow.py | 3 +-- .../core/tests/workflow/test_runner.py | 4 ++-- .../tests/test_http_request_executor.py | 22 +++++++++---------- .../test_http_request_yaml_integration.py | 2 +- .../tests/test_invoke_mcp_tool_executor.py | 20 ++++++++--------- .../tests/test_workflow_factory.py | 8 +++---- 7 files changed, 30 insertions(+), 31 deletions(-) diff --git a/python/packages/core/agent_framework/_workflows/_runner.py b/python/packages/core/agent_framework/_workflows/_runner.py index efe9bcaead..bd02a22bc1 100644 --- a/python/packages/core/agent_framework/_workflows/_runner.py +++ b/python/packages/core/agent_framework/_workflows/_runner.py @@ -114,7 +114,7 @@ class Runner: WorkflowRunnerException: If the runner is already reserved or running. """ if self._lifecycle is not _RunnerLifecycle.IDLE: - raise WorkflowRunnerException("Runner is already running.") + raise WorkflowRunnerException("Runner is already reserved or running.") self._lifecycle = _RunnerLifecycle.RESERVED def release(self) -> None: diff --git a/python/packages/core/agent_framework/_workflows/_workflow.py b/python/packages/core/agent_framework/_workflows/_workflow.py index 9135677cba..4a36e39574 100644 --- a/python/packages/core/agent_framework/_workflows/_workflow.py +++ b/python/packages/core/agent_framework/_workflows/_workflow.py @@ -556,8 +556,7 @@ class Workflow(DictConvertible): self._runner.state.set(WORKFLOW_RUN_KWARGS_KEY, {}) self._runner.state.commit() # Commit immediately so kwargs are available - # Set streaming mode (always set explicitly per run since - # reset_for_new_run() no longer runs to clear it). + # Explicitly set streaming mode per run self._runner.context.set_streaming(streaming) # Execute initial setup if provided diff --git a/python/packages/core/tests/workflow/test_runner.py b/python/packages/core/tests/workflow/test_runner.py index 2c8ea4d22f..0019ed1e92 100644 --- a/python/packages/core/tests/workflow/test_runner.py +++ b/python/packages/core/tests/workflow/test_runner.py @@ -335,7 +335,7 @@ def test_runner_reserve_twice_raises(): """ runner = _make_runner() runner.reserve() - with pytest.raises(WorkflowRunnerException, match="Runner is already running."): + with pytest.raises(WorkflowRunnerException, match="Runner is already reserved or running."): runner.reserve() @@ -428,7 +428,7 @@ async def test_runner_rejects_concurrent_run_until_convergence(): await started.wait() # first run is now executing try: - with pytest.raises(WorkflowRunnerException, match="Runner is already running."): + with pytest.raises(WorkflowRunnerException, match="Runner is already reserved or running."): runner.reserve() finally: release.set() diff --git a/python/packages/declarative/tests/test_http_request_executor.py b/python/packages/declarative/tests/test_http_request_executor.py index 4030cf4294..f07f19d27a 100644 --- a/python/packages/declarative/tests/test_http_request_executor.py +++ b/python/packages/declarative/tests/test_http_request_executor.py @@ -90,7 +90,7 @@ async def _run(yaml_def: dict[str, Any], handler: HttpRequestHandler) -> Any: def _state(workflow: Any, events: Any) -> dict[str, Any]: """Read declarative state out of the workflow after run completes.""" - return workflow._state.get(DECLARATIVE_STATE_KEY) or {} + return workflow._runner.state.get(DECLARATIVE_STATE_KEY) or {} # Helper used by parametrised path tests @@ -151,7 +151,7 @@ class TestSuccessPath: workflow = factory.create_workflow_from_definition(_yaml(_action(method="GET", response="Local.Result"))) await workflow.run({}) - decl = workflow._state.get(DECLARATIVE_STATE_KEY) + decl = workflow._runner.state.get(DECLARATIVE_STATE_KEY) assert decl["Local"]["Result"] == {"key": "value", "number": 42} assert handler.last_info is not None assert handler.last_info.method == "GET" @@ -164,7 +164,7 @@ class TestSuccessPath: workflow = factory.create_workflow_from_definition(_yaml(_action(response="Local.Result"))) await workflow.run({}) - decl = workflow._state.get(DECLARATIVE_STATE_KEY) + decl = workflow._runner.state.get(DECLARATIVE_STATE_KEY) assert decl["Local"]["Result"] == "not-json content" @pytest.mark.asyncio @@ -174,7 +174,7 @@ class TestSuccessPath: workflow = factory.create_workflow_from_definition(_yaml(_action(response="Local.Result"))) await workflow.run({}) - decl = workflow._state.get(DECLARATIVE_STATE_KEY) + decl = workflow._runner.state.get(DECLARATIVE_STATE_KEY) assert decl["Local"]["Result"] is None @pytest.mark.asyncio @@ -184,7 +184,7 @@ class TestSuccessPath: workflow = factory.create_workflow_from_definition(_yaml(_action(response={"path": "Local.Result"}))) await workflow.run({}) - decl = workflow._state.get(DECLARATIVE_STATE_KEY) + decl = workflow._runner.state.get(DECLARATIVE_STATE_KEY) assert decl["Local"]["Result"] == {"x": 1} @pytest.mark.asyncio @@ -517,7 +517,7 @@ class TestResponseHeaders: factory = WorkflowFactory(http_request_handler=handler) workflow = factory.create_workflow_from_definition(_yaml(_action(response_headers="Local.H"))) await workflow.run({}) - decl = workflow._state.get(DECLARATIVE_STATE_KEY) + decl = workflow._runner.state.get(DECLARATIVE_STATE_KEY) h = decl["Local"]["H"] assert h["Content-Type"] == "application/json" assert h["Set-Cookie"] == "a=1,b=2" @@ -528,7 +528,7 @@ class TestResponseHeaders: factory = WorkflowFactory(http_request_handler=handler) workflow = factory.create_workflow_from_definition(_yaml(_action(response_headers="Local.H"))) await workflow.run({}) - decl = workflow._state.get(DECLARATIVE_STATE_KEY) + decl = workflow._runner.state.get(DECLARATIVE_STATE_KEY) assert decl["Local"]["H"] is None @pytest.mark.asyncio @@ -538,7 +538,7 @@ class TestResponseHeaders: workflow = factory.create_workflow_from_definition(_yaml(_action(response_headers="Local.H"))) with pytest.raises(DeclarativeActionError): await workflow.run({}) - decl = workflow._state.get(DECLARATIVE_STATE_KEY) + decl = workflow._runner.state.get(DECLARATIVE_STATE_KEY) assert decl["Local"]["H"] == {"X-Trace": "abc"} @@ -559,7 +559,7 @@ class TestConversationAppend: ) ) await workflow.run({}) - decl = workflow._state.get(DECLARATIVE_STATE_KEY) + decl = workflow._runner.state.get(DECLARATIVE_STATE_KEY) conv = decl["System"]["conversations"].get("conv-test-1") assert conv is not None assert len(conv["messages"]) == 1 @@ -570,7 +570,7 @@ class TestConversationAppend: factory = WorkflowFactory(http_request_handler=handler) workflow = factory.create_workflow_from_definition(_yaml(_action(response="Local.Result", conversation_id=""))) await workflow.run({}) - decl = workflow._state.get(DECLARATIVE_STATE_KEY) + decl = workflow._runner.state.get(DECLARATIVE_STATE_KEY) # Auto-init creates an entry for the System.ConversationId conversation, # but it should NOT have HTTP-appended messages from us. for _cid, conv in decl["System"]["conversations"].items(): @@ -582,7 +582,7 @@ class TestConversationAppend: factory = WorkflowFactory(http_request_handler=handler) workflow = factory.create_workflow_from_definition(_yaml(_action(conversation_id="conv-test-1"))) await workflow.run({}) - decl = workflow._state.get(DECLARATIVE_STATE_KEY) + decl = workflow._runner.state.get(DECLARATIVE_STATE_KEY) # No conversation entry should have been created either. assert "conv-test-1" not in decl["System"]["conversations"] diff --git a/python/packages/declarative/tests/test_http_request_yaml_integration.py b/python/packages/declarative/tests/test_http_request_yaml_integration.py index 49cd0d15e8..64454e2b84 100644 --- a/python/packages/declarative/tests/test_http_request_yaml_integration.py +++ b/python/packages/declarative/tests/test_http_request_yaml_integration.py @@ -73,7 +73,7 @@ async def test_http_request_yaml_roundtrip() -> None: workflow = factory.create_workflow_from_yaml_path(FIXTURE_PATH) await workflow.run({}) - decl: dict[str, Any] = workflow._state.get(DECLARATIVE_STATE_KEY) or {} + decl: dict[str, Any] = workflow._runner.state.get(DECLARATIVE_STATE_KEY) or {} local = decl.get("Local") or {} assert local.get("RepoOwner") == "dotnet" diff --git a/python/packages/declarative/tests/test_invoke_mcp_tool_executor.py b/python/packages/declarative/tests/test_invoke_mcp_tool_executor.py index fdee1f7df1..cfff22b84e 100644 --- a/python/packages/declarative/tests/test_invoke_mcp_tool_executor.py +++ b/python/packages/declarative/tests/test_invoke_mcp_tool_executor.py @@ -244,7 +244,7 @@ class TestOutput: factory = WorkflowFactory(mcp_tool_handler=handler) workflow = factory.create_workflow_from_definition(_yaml(_action(output={"result": "Local.Result"}))) await workflow.run({}) - decl = workflow._state.get(DECLARATIVE_STATE_KEY) + decl = workflow._runner.state.get(DECLARATIVE_STATE_KEY) assert decl["Local"]["Result"] == [{"k": "v", "n": 1}] @pytest.mark.asyncio @@ -253,7 +253,7 @@ class TestOutput: factory = WorkflowFactory(mcp_tool_handler=handler) workflow = factory.create_workflow_from_definition(_yaml(_action(output={"result": "Local.Result"}))) await workflow.run({}) - decl = workflow._state.get(DECLARATIVE_STATE_KEY) + decl = workflow._runner.state.get(DECLARATIVE_STATE_KEY) assert decl["Local"]["Result"] == ["plain text not json"] @pytest.mark.asyncio @@ -262,7 +262,7 @@ class TestOutput: factory = WorkflowFactory(mcp_tool_handler=handler) workflow = factory.create_workflow_from_definition(_yaml(_action(output={"messages": "Local.Messages"}))) await workflow.run({}) - decl = workflow._state.get(DECLARATIVE_STATE_KEY) + decl = workflow._runner.state.get(DECLARATIVE_STATE_KEY) msg = decl["Local"]["Messages"] # Single Tool-role message containing both contents (parity with .NET). assert isinstance(msg, Message) @@ -276,7 +276,7 @@ class TestOutput: factory = WorkflowFactory(mcp_tool_handler=handler) workflow = factory.create_workflow_from_definition(_yaml(_action(output={"result": "Local.Result"}))) await workflow.run({}) - decl = workflow._state.get(DECLARATIVE_STATE_KEY) + decl = workflow._runner.state.get(DECLARATIVE_STATE_KEY) assert decl["Local"]["Result"] == ["https://example.com/file.txt"] @pytest.mark.asyncio @@ -285,7 +285,7 @@ class TestOutput: factory = WorkflowFactory(mcp_tool_handler=handler) workflow = factory.create_workflow_from_definition(_yaml(_action(output={"result": {"path": "Local.Result"}}))) await workflow.run({}) - decl = workflow._state.get(DECLARATIVE_STATE_KEY) + decl = workflow._runner.state.get(DECLARATIVE_STATE_KEY) assert decl["Local"]["Result"] == ["ok"] @@ -306,7 +306,7 @@ class TestConversation: ) ) await workflow.run({}) - decl = workflow._state.get(DECLARATIVE_STATE_KEY) + decl = workflow._runner.state.get(DECLARATIVE_STATE_KEY) conv = decl["System"]["conversations"]["conv-42"] msgs = conv["messages"] if isinstance(conv, dict) else conv.messages assert len(msgs) == 1 @@ -328,7 +328,7 @@ class TestConversation: ) ) await workflow.run({}) - decl = workflow._state.get(DECLARATIVE_STATE_KEY) + decl = workflow._runner.state.get(DECLARATIVE_STATE_KEY) # Empty conversation id must not produce a `""` entry under System.conversations. conversations = decl.get("System", {}).get("conversations", {}) assert "" not in conversations @@ -562,7 +562,7 @@ class TestErrorHandling: factory = WorkflowFactory(mcp_tool_handler=handler) workflow = factory.create_workflow_from_definition(_yaml(_action(output={"result": "Local.Result"}))) await workflow.run({}) - decl = workflow._state.get(DECLARATIVE_STATE_KEY) + decl = workflow._runner.state.get(DECLARATIVE_STATE_KEY) assert decl["Local"]["Result"] == "Error: server down" @pytest.mark.asyncio @@ -571,7 +571,7 @@ class TestErrorHandling: factory = WorkflowFactory(mcp_tool_handler=handler) workflow = factory.create_workflow_from_definition(_yaml(_action(output={"result": "Local.Result"}))) await workflow.run({}) - decl = workflow._state.get(DECLARATIVE_STATE_KEY) + decl = workflow._runner.state.get(DECLARATIVE_STATE_KEY) assert decl["Local"]["Result"] == "Error: invalid arguments" @pytest.mark.asyncio @@ -580,7 +580,7 @@ class TestErrorHandling: factory = WorkflowFactory(mcp_tool_handler=handler) workflow = factory.create_workflow_from_definition(_yaml(_action(output={"result": "Local.Result"}))) await workflow.run({}) - decl = workflow._state.get(DECLARATIVE_STATE_KEY) + decl = workflow._runner.state.get(DECLARATIVE_STATE_KEY) result = decl["Local"]["Result"] assert isinstance(result, str) assert result.startswith("Error:") diff --git a/python/packages/declarative/tests/test_workflow_factory.py b/python/packages/declarative/tests/test_workflow_factory.py index f163cd18f0..b6d0aaa6da 100644 --- a/python/packages/declarative/tests/test_workflow_factory.py +++ b/python/packages/declarative/tests/test_workflow_factory.py @@ -289,11 +289,11 @@ actions: # Stamp a marker into the declarative state between turns. The # continuation branch must preserve it; a state-clearing run would # wipe ``DECLARATIVE_STATE_KEY`` and force re-initialization. - state_data = workflow._state.get(DECLARATIVE_STATE_KEY) + state_data = workflow._runner.state.get(DECLARATIVE_STATE_KEY) assert isinstance(state_data, dict), "Expected declarative state to be initialized after turn 1" state_data["Local"] = {"persisted_marker": "kept-from-turn-1"} - workflow._state.set(DECLARATIVE_STATE_KEY, state_data) - workflow._state.commit() + workflow._runner.state.set(DECLARATIVE_STATE_KEY, state_data) + workflow._runner.state.commit() second = await agent.run("turn-2-msg") assert second.text == "turn-2-msg", ( @@ -303,7 +303,7 @@ actions: # The continuation branch in ``_ensure_state_initialized`` must: # 1. preserve the cross-turn marker we stamped above # 2. refresh Inputs.input and System.LastMessage* to the new turn - post_state = workflow._state.get(DECLARATIVE_STATE_KEY) + post_state = workflow._runner.state.get(DECLARATIVE_STATE_KEY) assert isinstance(post_state, dict), "declarative state vanished between turns" local = post_state.get("Local", {}) assert local.get("persisted_marker") == "kept-from-turn-1", (