Skip to content

Commit f0a575a

Browse files
rojiCopilot
roji
and
Copilot
authored
Stabilize Python and Java E2E harnesses (#2411)
* fix(python): avoid nested PowerShell in shell RPC test 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> * fix(python): await cold-resume lock release 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> * test(python): verify shell RPC working directory 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> * Stabilize Java E2E fixture handling 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> --------- Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
1 parent de17ac8 commit f0a575a

6 files changed

Lines changed: 222 additions & 41 deletions

File tree

‎java/sdk/src/test/java/com/github/copilot/CapiProxy.java‎

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -57,16 +57,15 @@ public class CapiProxy implements AutoCloseable {
5757

5858
private static final ObjectMapper MAPPER = new ObjectMapper();
5959
private static final Pattern LISTENING_PATTERN = Pattern.compile("Listening: (http://[^\\s]+)(?:\\s+(\\{.*\\}))?$");
60+
private static final HttpClient HTTP_CLIENT = HttpClient.newHttpClient();
6061

6162
private Process process;
6263
private String proxyUrl;
6364
private String connectProxyUrl;
6465
private String caFilePath;
65-
private final HttpClient httpClient;
6666
private BufferedReader stdoutReader;
6767

6868
public CapiProxy() {
69-
this.httpClient = HttpClient.newHttpClient();
7069
}
7170

7271
/**
@@ -212,7 +211,7 @@ public void configure(String filePath, String workDir, TestInfo testInfo) throws
212211
HttpRequest request = HttpRequest.newBuilder().uri(URI.create(proxyUrl + "/config"))
213212
.header("Content-Type", "application/json").POST(HttpRequest.BodyPublishers.ofString(body)).build();
214213

215-
HttpResponse<String> response = httpClient.send(request, HttpResponse.BodyHandlers.ofString());
214+
HttpResponse<String> response = HTTP_CLIENT.send(request, HttpResponse.BodyHandlers.ofString());
216215
if (response.statusCode() != 200) {
217216
throw new IOException("Proxy config failed with status " + response.statusCode() + ": " + response.body());
218217
}
@@ -234,7 +233,7 @@ public List<Map<String, Object>> getExchanges() throws IOException, InterruptedE
234233

235234
HttpRequest request = HttpRequest.newBuilder().uri(URI.create(proxyUrl + "/exchanges")).GET().build();
236235

237-
HttpResponse<String> response = httpClient.send(request, HttpResponse.BodyHandlers.ofString());
236+
HttpResponse<String> response = HTTP_CLIENT.send(request, HttpResponse.BodyHandlers.ofString());
238237
if (response.statusCode() != 200) {
239238
throw new IOException("Failed to get exchanges: " + response.statusCode());
240239
}
@@ -284,7 +283,7 @@ public void setCopilotUserByToken(String token, String login, String copilotPlan
284283
HttpRequest request = HttpRequest.newBuilder().uri(URI.create(proxyUrl + "/copilot-user-config"))
285284
.header("Content-Type", "application/json").POST(HttpRequest.BodyPublishers.ofString(body)).build();
286285

287-
HttpResponse<String> response = httpClient.send(request, HttpResponse.BodyHandlers.ofString());
286+
HttpResponse<String> response = HTTP_CLIENT.send(request, HttpResponse.BodyHandlers.ofString());
288287
if (response.statusCode() != 200) {
289288
throw new IOException(
290289
"Failed to set copilot user config: " + response.statusCode() + ": " + response.body());
@@ -331,7 +330,7 @@ public void setCopilotUserByToken(String token, Map<String, Object> response)
331330
HttpRequest request = HttpRequest.newBuilder().uri(URI.create(proxyUrl + "/copilot-user-config"))
332331
.header("Content-Type", "application/json").POST(HttpRequest.BodyPublishers.ofString(body)).build();
333332

334-
HttpResponse<String> response2 = httpClient.send(request, HttpResponse.BodyHandlers.ofString());
333+
HttpResponse<String> response2 = HTTP_CLIENT.send(request, HttpResponse.BodyHandlers.ofString());
335334
if (response2.statusCode() != 200) {
336335
throw new IOException(
337336
"Failed to set copilot user config: " + response2.statusCode() + ": " + response2.body());
@@ -376,7 +375,7 @@ public void stop(boolean skipWritingCache) throws IOException, InterruptedExcept
376375
HttpRequest request = HttpRequest.newBuilder().uri(URI.create(stopUrl))
377376
.POST(HttpRequest.BodyPublishers.noBody()).build();
378377

379-
httpClient.send(request, HttpResponse.BodyHandlers.ofString());
378+
HTTP_CLIENT.send(request, HttpResponse.BodyHandlers.ofString());
380379
} catch (Exception e) {
381380
// Best effort - ignore errors
382381
}

‎java/sdk/src/test/java/com/github/copilot/CapiProxyTest.java‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
/*---------------------------------------------------------------------------------------------
2+
* Copyright (c) Microsoft Corporation. All rights reserved.
3+
*--------------------------------------------------------------------------------------------*/
4+
5+
package com.github.copilot;
6+
7+
import static org.junit.jupiter.api.Assertions.assertTrue;
8+
9+
import java.lang.reflect.Modifier;
10+
11+
import org.junit.jupiter.api.Test;
12+
13+
class CapiProxyTest {
14+
15+
@Test
16+
void proxyInstancesShareHttpClientResources() throws Exception {
17+
var field = CapiProxy.class.getDeclaredField("HTTP_CLIENT");
18+
assertTrue(Modifier.isStatic(field.getModifiers()),
19+
"E2E contexts must not retain one HttpClient selector manager per test class");
20+
}
21+
}

‎java/sdk/src/test/java/com/github/copilot/E2ETestContext.java‎

Lines changed: 100 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -65,8 +65,8 @@ public class E2ETestContext implements AutoCloseable {
6565
*/
6666
private static final String DEFAULT_GITHUB_TOKEN = "fake-token-for-e2e-tests";
6767
private static final Pattern SNAKE_CASE = Pattern.compile("[^a-zA-Z0-9]");
68-
private static final Pattern USER_CONTENT_PATTERN = Pattern
69-
.compile("^\\s+-\\s+role:\\s+user\\s*$\\s+content:\\s*(.+?)$", Pattern.MULTILINE);
68+
private static final Pattern USER_ROLE_PATTERN = Pattern.compile("^(\\s*)-\\s+role:\\s+user\\s*$");
69+
private static final Pattern CONTENT_PATTERN = Pattern.compile("^(\\s*)content:\\s*(.*)$");
7070

7171
private final String cliPath;
7272
private final Path homeDir;
@@ -227,25 +227,109 @@ public List<String> getExpectedUserPrompts() {
227227
return List.of();
228228
}
229229
try {
230-
String content = Files.readString(currentSnapshotFile);
231-
List<String> prompts = new ArrayList<>();
232-
Matcher matcher = USER_CONTENT_PATTERN.matcher(content);
233-
while (matcher.find()) {
234-
String prompt = matcher.group(1).trim();
235-
// Remove quotes if present
236-
if ((prompt.startsWith("\"") && prompt.endsWith("\""))
237-
|| (prompt.startsWith("'") && prompt.endsWith("'"))) {
238-
prompt = prompt.substring(1, prompt.length() - 1);
230+
return parseExpectedUserPrompts(Files.readString(currentSnapshotFile));
231+
} catch (IOException e) {
232+
LOG.warning("Failed to read snapshot file: " + e.getMessage());
233+
return List.of();
234+
}
235+
}
236+
237+
static List<String> parseExpectedUserPrompts(String yaml) {
238+
String[] lines = yaml.split("\\R", -1);
239+
List<String> prompts = new ArrayList<>();
240+
241+
for (int i = 0; i < lines.length; i++) {
242+
Matcher roleMatcher = USER_ROLE_PATTERN.matcher(lines[i]);
243+
if (!roleMatcher.matches()) {
244+
continue;
245+
}
246+
247+
int roleIndent = roleMatcher.group(1).length();
248+
for (i++; i < lines.length; i++) {
249+
String line = lines[i];
250+
if (!line.isBlank() && leadingWhitespace(line) <= roleIndent) {
251+
i--;
252+
break;
239253
}
240-
if (!prompts.contains(prompt)) {
254+
255+
Matcher contentMatcher = CONTENT_PATTERN.matcher(line);
256+
if (!contentMatcher.matches()) {
257+
continue;
258+
}
259+
260+
int contentIndent = contentMatcher.group(1).length();
261+
String value = contentMatcher.group(2).trim();
262+
List<String> continuation = new ArrayList<>();
263+
while (i + 1 < lines.length
264+
&& (lines[i + 1].isBlank() || leadingWhitespace(lines[i + 1]) > contentIndent)) {
265+
continuation.add(lines[++i]);
266+
}
267+
268+
String prompt = isBlockScalar(value)
269+
? parseBlockScalar(value.charAt(0), continuation)
270+
: parsePlainScalar(value, continuation);
271+
if (!prompt.isEmpty() && !prompts.contains(prompt)) {
241272
prompts.add(prompt);
242273
}
274+
break;
243275
}
244-
return prompts;
245-
} catch (IOException e) {
246-
LOG.warning("Failed to read snapshot file: " + e.getMessage());
247-
return List.of();
248276
}
277+
278+
return prompts;
279+
}
280+
281+
private static String parsePlainScalar(String firstLine, List<String> continuation) {
282+
StringBuilder value = new StringBuilder(unquote(firstLine));
283+
for (String line : continuation) {
284+
if (!line.isBlank()) {
285+
if (!value.isEmpty()) {
286+
value.append(' ');
287+
}
288+
value.append(line.trim());
289+
}
290+
}
291+
return value.toString();
292+
}
293+
294+
private static String parseBlockScalar(char style, List<String> lines) {
295+
int contentIndent = lines.stream().filter(line -> !line.isBlank()).mapToInt(E2ETestContext::leadingWhitespace)
296+
.min().orElse(0);
297+
StringBuilder value = new StringBuilder();
298+
boolean previousWasContent = false;
299+
for (String line : lines) {
300+
String text = line.isBlank() ? "" : line.substring(Math.min(contentIndent, line.length()));
301+
if (text.isEmpty()) {
302+
value.append('\n');
303+
previousWasContent = false;
304+
} else {
305+
if (previousWasContent) {
306+
value.append(style == '>' ? ' ' : '\n');
307+
}
308+
value.append(text);
309+
previousWasContent = true;
310+
}
311+
}
312+
return value.toString().stripTrailing();
313+
}
314+
315+
private static boolean isBlockScalar(String value) {
316+
return value.matches("[>|][+-]?");
317+
}
318+
319+
private static String unquote(String value) {
320+
if (value.length() >= 2 && ((value.startsWith("\"") && value.endsWith("\""))
321+
|| (value.startsWith("'") && value.endsWith("'")))) {
322+
return value.substring(1, value.length() - 1);
323+
}
324+
return value;
325+
}
326+
327+
private static int leadingWhitespace(String value) {
328+
int index = 0;
329+
while (index < value.length() && Character.isWhitespace(value.charAt(index))) {
330+
index++;
331+
}
332+
return index;
249333
}
250334

251335
/**

‎java/sdk/src/test/java/com/github/copilot/E2ETestContextTest.java‎

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
/*---------------------------------------------------------------------------------------------
2+
* Copyright (c) Microsoft Corporation. All rights reserved.
3+
*--------------------------------------------------------------------------------------------*/
4+
5+
package com.github.copilot;
6+
7+
import static org.junit.jupiter.api.Assertions.assertEquals;
8+
9+
import java.util.List;
10+
11+
import org.junit.jupiter.api.Test;
12+
13+
class E2ETestContextTest {
14+
15+
@Test
16+
void expectedUserPromptsParseFoldedBlockScalars() {
17+
String snapshot = """
18+
conversations:
19+
- messages:
20+
- role: user
21+
content: First prompt
22+
continued here.
23+
- role: assistant
24+
content: response
25+
- role: user
26+
content: >-
27+
<system_notification>
28+
29+
Agent completed successfully.
30+
31+
</system_notification>
32+
""";
33+
34+
assertEquals(
35+
List.of("First prompt continued here.",
36+
"<system_notification>\nAgent completed successfully.\n</system_notification>"),
37+
E2ETestContext.parseExpectedUserPrompts(snapshot));
38+
}
39+
}

‎python/e2e/test_pending_work_resume_e2e.py‎

Lines changed: 38 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,11 +20,12 @@
2020
HandlePendingToolCallRequest,
2121
PermissionDecisionRequest,
2222
PermissionDecisionUserNotAvailable,
23+
SessionsCheckInUseRequest,
2324
)
2425
from copilot.session import PermissionHandler
2526
from copilot.tools import Tool, ToolInvocation, ToolResult
2627

27-
from .testharness import DEFAULT_GITHUB_TOKEN, E2ETestContext
28+
from .testharness import DEFAULT_GITHUB_TOKEN, E2ETestContext, wait_for_condition
2829

2930
pytestmark = pytest.mark.asyncio(loop_scope="module")
3031

@@ -464,6 +465,7 @@ async def blocking_external_tool(args):
464465
await server.start()
465466
try:
466467
cli_url = f"localhost:{server.runtime_port}"
468+
lock_observer: CopilotClient | None = None
467469

468470
suspended_client = CopilotClient(
469471
connection=RuntimeConnection.for_uri(
@@ -487,8 +489,41 @@ async def blocking_external_tool(args):
487489
assert (await asyncio.wait_for(tool_started, PENDING_WORK_TIMEOUT)) == "beta"
488490

489491
if disconnect_original_client:
492+
# force_stop closes the local socket before the server necessarily
493+
# processes that disconnect. Observe the session lock from another
494+
# runtime so resume cannot race the server's active-session cleanup.
495+
lock_observer = _make_subprocess_client(ctx)
496+
await lock_observer.start()
497+
498+
async def session_lock_is_held() -> bool:
499+
result = await lock_observer.rpc.sessions.check_in_use(
500+
SessionsCheckInUseRequest(session_ids=[session_id])
501+
)
502+
return session_id in result.in_use
503+
504+
await wait_for_condition(
505+
session_lock_is_held,
506+
timeout=PENDING_WORK_TIMEOUT,
507+
timeout_message=(
508+
f"Timed out waiting for session '{session_id}' to acquire its lock."
509+
),
510+
)
490511
await suspended_client.force_stop()
491512

513+
async def session_lock_is_released() -> bool:
514+
result = await lock_observer.rpc.sessions.check_in_use(
515+
SessionsCheckInUseRequest(session_ids=[session_id])
516+
)
517+
return session_id not in result.in_use
518+
519+
await wait_for_condition(
520+
session_lock_is_released,
521+
timeout=PENDING_WORK_TIMEOUT,
522+
timeout_message=(
523+
f"Timed out waiting for session '{session_id}' to release its lock."
524+
),
525+
)
526+
492527
resumed_client = CopilotClient(
493528
connection=RuntimeConnection.for_uri(
494529
cli_url, connection_token="py-tcp-shared-test-token"
@@ -550,6 +585,8 @@ async def resumed_external_tool(args):
550585
if not release_original.done():
551586
release_original.set_result("ORIGINAL_SHOULD_NOT_WIN")
552587
await _safe_force_stop(suspended_client)
588+
if lock_observer is not None:
589+
await _safe_force_stop(lock_observer)
553590
finally:
554591
await _safe_force_stop(server)
555592

‎python/e2e/test_rpc_shell_and_fleet_e2e.py‎

Lines changed: 18 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -37,11 +37,10 @@
3737

3838
def _write_file_command(marker_path: Path, marker: str) -> str:
3939
if sys.platform == "win32":
40-
return (
41-
f"powershell -NoLogo -NoProfile -Command "
42-
f"\"Set-Content -LiteralPath '{marker_path}' -Value '{marker}'\""
43-
)
44-
return f"sh -c \"printf '%s' '{marker}' > '{marker_path}'\""
40+
# shell.exec already runs through cmd.exe on Windows. Use its built-in echo
41+
# instead of spawning a nested PowerShell process just to write the marker.
42+
return f'echo {marker}>"{marker_path.name}"'
43+
return f"sh -c \"printf '%s' '{marker}' > '{marker_path.name}'\""
4544

4645

4746
async def _wait_for_file_text(path: Path, expected: str, *, timeout: float = 30.0) -> None:
@@ -57,19 +56,21 @@ async def _wait_for_file_text(path: Path, expected: str, *, timeout: float = 30.
5756

5857
class TestRpcShellAndFleet:
5958
async def test_should_execute_shell_command(self, ctx: E2ETestContext):
60-
session = await ctx.client.create_session(
59+
async with await ctx.client.create_session(
6160
on_permission_request=PermissionHandler.approve_all,
62-
)
63-
marker_path = Path(ctx.work_dir) / f"shell-rpc-{uuid.uuid4().hex}.txt"
64-
marker = "copilot-sdk-shell-rpc"
65-
66-
result = await session.rpc.shell.exec(
67-
ShellExecRequest(command=_write_file_command(marker_path, marker), cwd=ctx.work_dir)
68-
)
69-
assert (result.process_id or "").strip()
70-
await _wait_for_file_text(marker_path, marker)
71-
72-
await session.disconnect()
61+
) as session:
62+
command_dir = Path(ctx.work_dir) / f"shell-rpc-{uuid.uuid4().hex}"
63+
command_dir.mkdir()
64+
marker_path = command_dir / "marker.txt"
65+
marker = "copilot-sdk-shell-rpc"
66+
67+
result = await session.rpc.shell.exec(
68+
ShellExecRequest(
69+
command=_write_file_command(marker_path, marker), cwd=str(command_dir)
70+
)
71+
)
72+
assert (result.process_id or "").strip()
73+
await _wait_for_file_text(marker_path, marker)
7374

7475
async def test_should_kill_shell_process(self, ctx: E2ETestContext):
7576
session = await ctx.client.create_session(

0 commit comments

Comments
 (0)