Repository navigation
[python] Backfill remote test coverage ahead of local execution path removal - #881
Merged
Merged
Conversation
…kwargs Backfill remote (MiniCluster) coverage for two durable_execute behaviors previously exercised only by the in-process local execution path: propagation of exceptions raised through synchronous durable_execute, and calling durable_execute with keyword arguments. Existing remote tests covered only the async-exception and positional-argument paths. Mirrors the established execute_test.py pattern (FileSource in, StreamingFileSink out, check_result assertion) with matching ground-truth files.
…del get_resource Backfill remote (MiniCluster) coverage, using a deterministic mock chat model, for behavior previously exercised only by the in-process local execution path: the built-in chat_model_action + tool_call_action content flow with correct tool binding, and get_resource(CHAT_MODEL) inside an action with custom ResourceDescriptor field pass-through. The mock stubs only LLM inference, so the framework's built-in actions remain the code under test. Follows the execute_test.py agent/test split and resource convention.
Backfill remote (MiniCluster) MCP coverage — none existed. The test invokes the MCP tool and fetches the MCP prompt directly via get_resource, with no LLM, mirroring the Java MCPCrossLanguageAgent pattern. It self-starts the existing MCP server scripts and gates only on server readiness (no Ollama), asserting the server-computed sum and prompt content for both with-prompts and without-prompts server modes.
github-actions
Bot
added
doc-not-needed
…non-Ollama chat providers Backfill the remaining behaviors covered only by the in-process local execution path: the apply(unknown-name) ValueError guard on the remote builder (unit test, no cluster); short-term-memory workflow semantics (same-key visit_count accumulation and auto-generated key for a keyless record) on a real MiniCluster; and chat model coverage for the non-Ollama providers (Tongyi/OpenAI/AzureOpenAI) on the remote path, keeping the original per-provider credential gating.
weiqingy
force-pushed
the
829-pr1-backfill-remote-tests
branch
from
July 7, 2026 18:10
91ceefc to
8b146fe
Compare
This was referenced Jul 7, 2026
wenjin272
reviewed
Jul 8, 2026
| return int(result[0]) | ||
|
|
||
|
|
||
| class MCPRemoteWithPromptsAgent(Agent): |
Contributor
There was a problem hiding this comment.
It seems this Agent is intended to replace MyMCPAgent in e2e_tests_mcp/mcp_test.py. However, unlike its predecessor, it does not bind MCP prompts and tools to the ChatModel, which I consider a regression in test coverage. Could we continue using MyMCPAgent instead, limiting changes to only the submission and validation logic within the test_mcp function?
Collaborator
Author
There was a problem hiding this comment.
Thanks for the review @wenjin272 . Good catch. Fixed in e2272ee: MyMCPAgent is kept as-is and only test_mcp's submission/validation move to the MiniCluster path; the direct-invocation agent/test are removed. Could you take another look? Let me know if you have any concerns.
Preserves the MCP->ChatModel binding coverage (the add tool and ask_sum prompt bound into a chat_model_setup) that the direct-invocation agent lacked. Only test_mcp's submission and validation move from the local from_list/to_list path to a FileSource + StreamingFileSink round-trip on a real StreamExecutionEnvironment; MyMCPAgent itself is unchanged. The direct-invocation agent/test and its ground-truth fixtures are removed.
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
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.
Linked issue: #829
Purpose of change
This is the first of two PRs retiring the Python in-process local execution path (
local_runner/local_execution_environment) in favor of validating agent logic on Flink's MiniCluster, per #829.This PR is purely additive. Several behaviors are currently exercised only through the local path (via its
from_list/to_list/get_tool_request_eventstest API); before the local path can be removed, that coverage has to exist on the remote (real-Flink) path. This PR backfills it, so the follow-up deletion PR removes the local path without losing any coverage.New remote (MiniCluster) tests added:
durable_executeexception propagation and keyword-argument calls — the existing remote durable-execute tests only covered the async-exception and positional-argument paths.chat_model_action+tool_call_actioncontent and tool binding, driven by a deterministic mock chat model (the mock stubs only LLM inference, so the built-in actions remain the code under test).get_resource(CHAT_MODEL)inside an action, including customResourceDescriptorfield pass-through.get_resource(...).call(...)with no LLM — mirroring the existing JavaMCPCrossLanguageAgentpattern and gated only on the self-started MCP server. This is the first remote MCP coverage in the repo.apply(<unknown-name>)→ValueErrorguard on the remote builder (unit test; the guard fires before a cluster starts).visit_countaccumulation across records, and auto-generated key for a record with no key.Tests
This PR is entirely tests. Verified on a Flink MiniCluster: one end-to-end session runs 11 passed, 3 skipped (the skips are the credential-gated provider cases). The deterministic tests (durable-execute, built-in action, MCP, workflow memory) run without any external service; the MCP test self-starts the MCP server. The
apply-guard unit test runs without a cluster.API
No public API changes. Test-only.
Documentation
doc-neededdoc-not-neededdoc-included