Skip to content

Commit 6cabb00

Browse files
committed
Fix Windows test compatibility for ProcessBuilder usage
On Windows, Java's ProcessBuilder cannot directly run shell wrappers like `npx` (installed as npx.cmd) or Unix commands like `cat`. Tests that used these commands failed with "Cannot run program" errors. Additionally, Unix-style paths like "/nonexistent/copilot" are not absolute on Windows, causing assertThrows(IOException) tests to pass unexpectedly when CliServerManager wrapped them with "cmd /c". Changes: - CapiProxy: use "cmd /c npx" on Windows to launch the test harness - CliServerManagerTest: replace "cat" with cross-platform dummy process; use a platform-appropriate nonexistent absolute path so IOException is thrown on all platforms - JsonRpcClientTest: replace "cat" with cross-platform dummy process All changes use runtime os.name detection and preserve existing behavior on Linux and macOS. Full test suite passes on all platforms (556 tests, 0 failures, 0 errors).
1 parent c05b0a2 commit 6cabb00

File tree

3 files changed

+38
-16
lines changed

3 files changed

+38
-16
lines changed

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

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,11 @@ public String start() throws IOException, InterruptedException {
8989
}
9090

9191
// Start the harness server using npx tsx
92-
var pb = new ProcessBuilder("npx", "tsx", "server.ts");
92+
// On Windows, npx is installed as npx.cmd which requires cmd /c to launch
93+
boolean isWindows = System.getProperty("os.name").toLowerCase().contains("win");
94+
var pb = isWindows
95+
? new ProcessBuilder("cmd", "/c", "npx", "tsx", "server.ts")
96+
: new ProcessBuilder("npx", "tsx", "server.ts");
9397
pb.directory(harnessDir.toFile());
9498
pb.redirectErrorStream(false);
9599

src/test/java/com/github/copilot/sdk/CliServerManagerTest.java

Lines changed: 21 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -75,8 +75,11 @@ void connectToServerStdioMode() throws Exception {
7575
var options = new CopilotClientOptions();
7676
var manager = new CliServerManager(options);
7777

78-
// Create a dummy process for stdio mode
79-
Process process = new ProcessBuilder("cat").start();
78+
// Create a dummy process for stdio mode (use a cross-platform command)
79+
boolean isWindows = System.getProperty("os.name").toLowerCase().contains("win");
80+
Process process = isWindows
81+
? new ProcessBuilder("cmd", "/c", "type", "NUL").start()
82+
: new ProcessBuilder("cat").start();
8083
try {
8184
JsonRpcClient client = manager.connectToServer(process, null, null);
8285
assertNotNull(client);
@@ -126,6 +129,14 @@ void processInfoWithNullPort() {
126129
// resolveCliCommand is private, so we test indirectly through startCliServer
127130
// with specific cliPath values.
128131

132+
// On Windows, "/nonexistent/copilot" is not an absolute path (no drive letter),
133+
// so resolveCliCommand wraps it with "cmd /c" and ProcessBuilder.start()
134+
// succeeds
135+
// (launching cmd.exe). Use a Windows-absolute path to ensure IOException.
136+
private static final String NONEXISTENT_CLI = System.getProperty("os.name").toLowerCase().contains("win")
137+
? "C:\\nonexistent\\copilot"
138+
: "/nonexistent/copilot";
139+
129140
@Test
130141
void startCliServerWithJsFile() throws Exception {
131142
// Using a .js file path causes resolveCliCommand to prepend "node"
@@ -147,8 +158,8 @@ void startCliServerWithJsFile() throws Exception {
147158
@Test
148159
void startCliServerWithCliArgs() throws Exception {
149160
// Test that cliArgs are included in the command
150-
var options = new CopilotClientOptions().setCliPath("/nonexistent/copilot")
151-
.setCliArgs(new String[]{"--extra-flag"}).setUseStdio(true);
161+
var options = new CopilotClientOptions().setCliPath(NONEXISTENT_CLI).setCliArgs(new String[]{"--extra-flag"})
162+
.setUseStdio(true);
152163
var manager = new CliServerManager(options);
153164

154165
var ex = assertThrows(IOException.class, () -> manager.startCliServer());
@@ -158,7 +169,7 @@ void startCliServerWithCliArgs() throws Exception {
158169
@Test
159170
void startCliServerWithExplicitPort() throws Exception {
160171
// Test the explicit port branch (useStdio=false, port > 0)
161-
var options = new CopilotClientOptions().setCliPath("/nonexistent/copilot").setUseStdio(false).setPort(9999);
172+
var options = new CopilotClientOptions().setCliPath(NONEXISTENT_CLI).setUseStdio(false).setPort(9999);
162173
var manager = new CliServerManager(options);
163174

164175
var ex = assertThrows(IOException.class, () -> manager.startCliServer());
@@ -168,7 +179,7 @@ void startCliServerWithExplicitPort() throws Exception {
168179
@Test
169180
void startCliServerWithGitHubToken() throws Exception {
170181
// Test the github token branch
171-
var options = new CopilotClientOptions().setCliPath("/nonexistent/copilot").setGitHubToken("ghp_test123")
182+
var options = new CopilotClientOptions().setCliPath(NONEXISTENT_CLI).setGitHubToken("ghp_test123")
172183
.setUseStdio(true);
173184
var manager = new CliServerManager(options);
174185

@@ -179,7 +190,7 @@ void startCliServerWithGitHubToken() throws Exception {
179190
@Test
180191
void startCliServerWithUseLoggedInUserExplicit() throws Exception {
181192
// Test the explicit useLoggedInUser=false branch (adds --no-auto-login)
182-
var options = new CopilotClientOptions().setCliPath("/nonexistent/copilot").setUseLoggedInUser(false)
193+
var options = new CopilotClientOptions().setCliPath(NONEXISTENT_CLI).setUseLoggedInUser(false)
183194
.setUseStdio(true);
184195
var manager = new CliServerManager(options);
185196

@@ -190,7 +201,7 @@ void startCliServerWithUseLoggedInUserExplicit() throws Exception {
190201
@Test
191202
void startCliServerWithGitHubTokenAndNoExplicitUseLoggedInUser() throws Exception {
192203
// When gitHubToken is set and useLoggedInUser is null, defaults to false
193-
var options = new CopilotClientOptions().setCliPath("/nonexistent/copilot").setGitHubToken("ghp_test123")
204+
var options = new CopilotClientOptions().setCliPath(NONEXISTENT_CLI).setGitHubToken("ghp_test123")
194205
.setUseStdio(true);
195206
var manager = new CliServerManager(options);
196207

@@ -220,8 +231,7 @@ void startCliServerWithTelemetryAllOptions() throws Exception {
220231
// so even with a nonexistent CLI path, the telemetry code path is exercised
221232
var telemetry = new TelemetryConfig().setOtlpEndpoint("http://localhost:4318").setFilePath("/tmp/telemetry.log")
222233
.setExporterType("otlp-http").setSourceName("test-app").setCaptureContent(true);
223-
var options = new CopilotClientOptions().setCliPath("/nonexistent/copilot").setTelemetry(telemetry)
224-
.setUseStdio(true);
234+
var options = new CopilotClientOptions().setCliPath(NONEXISTENT_CLI).setTelemetry(telemetry).setUseStdio(true);
225235
var manager = new CliServerManager(options);
226236

227237
var ex = assertThrows(IOException.class, () -> manager.startCliServer());
@@ -232,8 +242,7 @@ void startCliServerWithTelemetryAllOptions() throws Exception {
232242
void startCliServerWithTelemetryCaptureContentFalse() throws Exception {
233243
// Test the false branch of getCaptureContent()
234244
var telemetry = new TelemetryConfig().setCaptureContent(false);
235-
var options = new CopilotClientOptions().setCliPath("/nonexistent/copilot").setTelemetry(telemetry)
236-
.setUseStdio(true);
245+
var options = new CopilotClientOptions().setCliPath(NONEXISTENT_CLI).setTelemetry(telemetry).setUseStdio(true);
237246
var manager = new CliServerManager(options);
238247

239248
var ex = assertThrows(IOException.class, () -> manager.startCliServer());

src/test/java/com/github/copilot/sdk/JsonRpcClientTest.java

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -135,15 +135,21 @@ void testIsConnectedWithSocketClosed() throws Exception {
135135

136136
@Test
137137
void testIsConnectedWithProcess() throws Exception {
138-
Process proc = new ProcessBuilder("cat").start();
138+
boolean isWindows = System.getProperty("os.name").toLowerCase().contains("win");
139+
Process proc = isWindows
140+
? new ProcessBuilder("cmd", "/c", "type", "NUL").start()
141+
: new ProcessBuilder("cat").start();
139142
try (var client = JsonRpcClient.fromProcess(proc)) {
140143
assertTrue(client.isConnected());
141144
}
142145
}
143146

144147
@Test
145148
void testIsConnectedWithProcessDead() throws Exception {
146-
Process proc = new ProcessBuilder("cat").start();
149+
boolean isWindows = System.getProperty("os.name").toLowerCase().contains("win");
150+
Process proc = isWindows
151+
? new ProcessBuilder("cmd", "/c", "type", "NUL").start()
152+
: new ProcessBuilder("cat").start();
147153
var client = JsonRpcClient.fromProcess(proc);
148154
proc.destroy();
149155
proc.waitFor(5, TimeUnit.SECONDS);
@@ -155,7 +161,10 @@ void testIsConnectedWithProcessDead() throws Exception {
155161

156162
@Test
157163
void testGetProcessReturnsProcess() throws Exception {
158-
Process proc = new ProcessBuilder("cat").start();
164+
boolean isWindows = System.getProperty("os.name").toLowerCase().contains("win");
165+
Process proc = isWindows
166+
? new ProcessBuilder("cmd", "/c", "type", "NUL").start()
167+
: new ProcessBuilder("cat").start();
159168
try (var client = JsonRpcClient.fromProcess(proc)) {
160169
assertSame(proc, client.getProcess());
161170
}

0 commit comments

Comments
 (0)