Stabilize Python and Java E2E harnesses - #2411
Merged
Merged
Conversation
Use the Windows shell's built-in echo command and a relative marker path so the test avoids a flaky child-process launch while continuing to validate shell execution and cwd handling. Ensure the session is disconnected on assertion failures. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
This comment has been minimized.
This comment has been minimized.
Contributor
There was a problem hiding this comment.
Pull request overview
Improves Python shell RPC E2E reliability on Windows and ensures session cleanup.
Changes:
- Replaces nested PowerShell with
cmd.exe’s built-inecho. - Uses async context management for session cleanup.
- Attempts to validate shell
cwdhandling via a relative marker path.
Show a summary per file
| File | Description |
|---|---|
python/e2e/test_rpc_shell_and_fleet_e2e.py |
Updates Windows marker creation and session lifecycle handling. |
Review details
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
- Files reviewed: 1/1 changed files
- Comments generated: 1
- Review effort level: Balanced
Observe the session lock from a separate runtime and wait for the TCP server to process the original client's disconnect before resuming. This preserves the cold-resume assertion without relying on timing. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
roji
changed the title
Contributor
Cross-SDK Consistency Review ✅This PR only modifies Python E2E test files (
No public API surface was added or changed, so no cross-SDK consistency concerns apply here. These are Python-specific test hardening changes that do not require equivalent updates in other language SDKs.
|
roji
enabled auto-merge
August 26, 2026 15:00
Run the marker command from a distinct subdirectory and use a relative filename on every platform so the test fails if shell.exec ignores cwd. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Parse folded YAML prompt scalars correctly and share replay proxy HTTP resources across the monolithic test JVM. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
roji
changed the title
roji
changed the title
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.
Summary
Harden the Python and Java E2E harnesses against four independent sources of nondeterminism:
cmd.exedirectly instead of spawning nested PowerShellHttpClientacross the test JVMPython fixes
Shell RPC process lifecycle
Merge-group run 32978642851 failed in
windows-latest, inprocessaftersession.shell.execreturned a valid process ID but its nested PowerShell marker command never created the expected file. The test now uses the platform shell's built-inecho, runs in a distinct subdirectory, and writes a relative marker path. This removes an unnecessary child-process boundary while preserving shell execution andcwdcoverage. The session is also scoped withasync withso failures cannot leak runtime state.Cold-resume synchronization
The requeued merge-group run 32980446331 failed in
macos-latest, defaultbecauseforce_stop()had closed the original TCP client locally, but the shared server had not yet processed the disconnect. An immediate resume therefore sometimes reportedsessionWasActive=trueinstead of exercising the intended cold-resume path.The test now observes
sessions.checkInUsefrom a separate runtime and waits for the original session lock to disappear before resuming. This provides a server-observable lifecycle barrier without adding sleeps or weakening the cold-resume assertions.Java fixes
Folded YAML prompts
E2ETestContextnow reconstructs folded YAML prompt scalars (>-) using YAML folding semantics instead of treating each source line as a separate prompt line. Regression coverage verifies the parsed prompt sent to the replay proxy.Replay proxy resources
CapiProxynow shares oneHttpClientacross proxy instances. The Java E2E suite creates many proxies in one JVM; allocating a client per fixture accumulated selector-manager threads and native resources over the full run. Regression coverage verifies the shared-client behavior.Validation
Python
uv run ruff format --check .uv run ruff checkuv run ty check copilotJava
mvn spotless:checkmvn -pl sdk test -Dtest=E2ETestContextTest,CapiProxyTest: 2 passedThe Windows shell path cannot be executed locally on macOS; the PR matrix provides the definitive Windows validation.