Skip to content

Commit 0c7886c

Browse files
stephentoubCopilot
stephentoub
and
Copilot
authored
Fix flaky SDK E2E tests (#1379)
* Fix flaky SDK E2E tests Avoid waiting for full assistant completions in tool-filter tests when the assertions only need captured CAPI requests. Replace fake echo MCP server configs with the shared Node MCP test server and wait for MCP connectivity before prompts in real MCP E2E tests. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Format Python E2E harness Apply Ruff formatting to the new exchange polling helper. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Address CodeQL review comments Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Avoid rooted path combine in test harness lookup Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Stabilize custom config dir E2E test Use SendAndWaitAsync so the test subscribes for response events before sending the prompt, and dispose the session after the custom config dir check. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Stabilize permission handler E2E test Start the send-and-wait operation before awaiting permission callbacks so response events are observed by the operation's subscription. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 578782e commit 0c7886c

28 files changed

Lines changed: 691 additions & 536 deletions

‎dotnet/test/E2E/PermissionE2ETests.cs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -50,14 +50,14 @@ public async Task Should_Invoke_Permission_Handler_For_Write_Operations()
5050

5151
await File.WriteAllTextAsync(Path.Combine(Ctx.WorkDir, "test.txt"), "original content");
5252

53-
await session.SendAsync(new MessageOptions
53+
var sendTask = session.SendAndWaitAsync(new MessageOptions
5454
{
5555
Prompt = "Edit test.txt and replace 'original' with 'modified'"
5656
});
5757

5858
var readRequest = await readPermissionRequestReceived.Task.WaitAsync(TimeSpan.FromSeconds(30));
5959
var writeRequest = await writePermissionRequestReceived.Task.WaitAsync(TimeSpan.FromSeconds(30));
60-
await TestHelper.GetFinalAssistantMessageAsync(session);
60+
await sendTask;
6161

6262
List<PermissionRequest> observedPermissionRequests;
6363
lock (permissionRequestsLock)

‎dotnet/test/E2E/PreMcpToolCallHookE2ETests.cs‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ namespace GitHub.Copilot.Test.E2E;
1515
public class PreMcpToolCallHookE2ETests(E2ETestFixture fixture, ITestOutputHelper output)
1616
: E2ETestBase(fixture, "pre_mcp_tool_call_hook", output)
1717
{
18-
private static string FindTestHarnessDir()
18+
private static string FindMetaEchoTestHarnessDir()
1919
{
2020
var dir = new DirectoryInfo(AppContext.BaseDirectory);
2121
while (dir != null)
@@ -42,7 +42,7 @@ private static string FindTestHarnessDir()
4242
[Fact]
4343
public async Task Should_Set_Meta_Via_PreMcpToolCall_Hook()
4444
{
45-
var testHarnessDir = FindTestHarnessDir();
45+
var testHarnessDir = FindMetaEchoTestHarnessDir();
4646
var hookInputs = new List<PreMcpToolCallHookInput>();
4747

4848
var session = await CreateSessionAsync(new SessionConfig
@@ -84,7 +84,7 @@ public async Task Should_Set_Meta_Via_PreMcpToolCall_Hook()
8484
[Fact]
8585
public async Task Should_Replace_Meta_Via_PreMcpToolCall_Hook()
8686
{
87-
var testHarnessDir = FindTestHarnessDir();
87+
var testHarnessDir = FindMetaEchoTestHarnessDir();
8888
var hookInputs = new List<PreMcpToolCallHookInput>();
8989

9090
var session = await CreateSessionAsync(new SessionConfig
@@ -125,7 +125,7 @@ public async Task Should_Replace_Meta_Via_PreMcpToolCall_Hook()
125125
[Fact]
126126
public async Task Should_Remove_Meta_Via_PreMcpToolCall_Hook()
127127
{
128-
var testHarnessDir = FindTestHarnessDir();
128+
var testHarnessDir = FindMetaEchoTestHarnessDir();
129129
var hookInputs = new List<PreMcpToolCallHookInput>();
130130

131131
var session = await CreateSessionAsync(new SessionConfig

‎dotnet/test/E2E/RpcMcpAndSkillsE2ETests.cs‎

Lines changed: 8 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
* Copyright (c) Microsoft Corporation. All rights reserved.
33
*--------------------------------------------------------------------------------------------*/
44

5+
using GitHub.Copilot.Rpc;
56
using Xunit;
67
using Xunit.Abstractions;
78
using RpcSkill = GitHub.Copilot.Rpc.Skill;
@@ -67,21 +68,14 @@ public async Task Should_List_Mcp_Servers_With_Configured_Server()
6768
const string serverName = "rpc-list-mcp-server";
6869
var session = await CreateSessionAsync(new SessionConfig
6970
{
70-
McpServers = new Dictionary<string, McpServerConfig>
71-
{
72-
[serverName] = new McpStdioServerConfig
73-
{
74-
Command = "echo",
75-
Args = ["rpc-list-mcp-server"],
76-
Tools = ["*"],
77-
},
78-
},
71+
McpServers = CreateTestMcpServers(serverName),
7972
});
8073

74+
await WaitForMcpServerStatusAsync(session, serverName, McpServerStatus.Connected);
8175
var result = await session.Rpc.Mcp.ListAsync();
8276

8377
var server = Assert.Single(result.Servers, server => string.Equals(server.Name, serverName, StringComparison.Ordinal));
84-
Assert.False(string.IsNullOrWhiteSpace(server.Status.Value));
78+
Assert.Equal(McpServerStatus.Connected, server.Status);
8579
}
8680

8781
[Fact]
@@ -143,16 +137,9 @@ public async Task Should_Report_Error_When_Mcp_Oauth_Server_Is_Not_Configured()
143137
{
144138
var session = await CreateSessionAsync(new SessionConfig
145139
{
146-
McpServers = new Dictionary<string, McpServerConfig>
147-
{
148-
["configured-stdio-server"] = new McpStdioServerConfig
149-
{
150-
Command = "echo",
151-
Args = ["configured-stdio-server"],
152-
Tools = ["*"],
153-
},
154-
},
140+
McpServers = CreateTestMcpServers("configured-stdio-server"),
155141
});
142+
await WaitForMcpServerStatusAsync(session, "configured-stdio-server", McpServerStatus.Connected);
156143

157144
await AssertFailureAsync(
158145
() => session.Rpc.Mcp.Oauth.LoginAsync("missing-server"),
@@ -165,16 +152,9 @@ public async Task Should_Report_Error_When_Mcp_Oauth_Server_Is_Not_Remote()
165152
const string serverName = "configured-stdio-server";
166153
var session = await CreateSessionAsync(new SessionConfig
167154
{
168-
McpServers = new Dictionary<string, McpServerConfig>
169-
{
170-
[serverName] = new McpStdioServerConfig
171-
{
172-
Command = "echo",
173-
Args = [serverName],
174-
Tools = ["*"],
175-
},
176-
},
155+
McpServers = CreateTestMcpServers(serverName),
177156
});
157+
await WaitForMcpServerStatusAsync(session, serverName, McpServerStatus.Connected);
178158

179159
await AssertFailureAsync(
180160
() => session.Rpc.Mcp.Oauth.LoginAsync(serverName, forceReauth: true, clientName: "SDK E2E", callbackSuccessMessage: "Done"),

‎dotnet/test/E2E/SessionConfigE2ETests.cs‎

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -435,12 +435,17 @@ public async Task Should_Apply_AvailableTools_On_Session_Resume()
435435
AvailableTools = ["view"],
436436
});
437437

438-
await session2.SendAndWaitAsync(new MessageOptions { Prompt = "What is 1+1?" });
439-
440-
var exchange = Assert.Single(await Ctx.GetExchangesAsync());
441-
Assert.Equal(["view"], GetToolNames(exchange));
442-
443-
await session2.DisposeAsync();
438+
try
439+
{
440+
var exchange = Assert.Single(await SendAndWaitForExchangesAsync(
441+
session2,
442+
new MessageOptions { Prompt = "What is 1+1?" }));
443+
Assert.Equal(["view"], GetToolNames(exchange));
444+
}
445+
finally
446+
{
447+
await session2.DisposeAsync();
448+
}
444449
}
445450

446451
[Fact]

‎dotnet/test/E2E/SessionE2ETests.cs‎

Lines changed: 51 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -130,16 +130,20 @@ public async Task Should_Create_A_Session_With_AvailableTools()
130130
AvailableTools = ["view", "edit"]
131131
});
132132

133-
await session.SendAsync(new MessageOptions { Prompt = "What is 1+1?" });
134-
await TestHelper.GetFinalAssistantMessageAsync(session);
135-
136-
var traffic = await Ctx.GetExchangesAsync();
137-
Assert.NotEmpty(traffic);
138-
139-
var toolNames = GetToolNames(traffic[0]);
140-
Assert.Equal(2, toolNames.Count);
141-
Assert.Contains("view", toolNames);
142-
Assert.Contains("edit", toolNames);
133+
try
134+
{
135+
var traffic = await SendAndWaitForExchangesAsync(
136+
session,
137+
new MessageOptions { Prompt = "What is 1+1?" });
138+
var toolNames = GetToolNames(traffic[0]);
139+
Assert.Equal(2, toolNames.Count);
140+
Assert.Contains("view", toolNames);
141+
Assert.Contains("edit", toolNames);
142+
}
143+
finally
144+
{
145+
await session.DisposeAsync();
146+
}
143147
}
144148

145149
[Fact]
@@ -150,16 +154,20 @@ public async Task Should_Create_A_Session_With_ExcludedTools()
150154
ExcludedTools = ["view"]
151155
});
152156

153-
await session.SendAsync(new MessageOptions { Prompt = "What is 1+1?" });
154-
await TestHelper.GetFinalAssistantMessageAsync(session);
155-
156-
var traffic = await Ctx.GetExchangesAsync();
157-
Assert.NotEmpty(traffic);
158-
159-
var toolNames = GetToolNames(traffic[0]);
160-
Assert.DoesNotContain("view", toolNames);
161-
Assert.Contains("edit", toolNames);
162-
Assert.Contains("grep", toolNames);
157+
try
158+
{
159+
var traffic = await SendAndWaitForExchangesAsync(
160+
session,
161+
new MessageOptions { Prompt = "What is 1+1?" });
162+
var toolNames = GetToolNames(traffic[0]);
163+
Assert.DoesNotContain("view", toolNames);
164+
Assert.Contains("edit", toolNames);
165+
Assert.Contains("grep", toolNames);
166+
}
167+
finally
168+
{
169+
await session.DisposeAsync();
170+
}
163171
}
164172

165173
[Fact]
@@ -180,14 +188,18 @@ public async Task Should_Create_A_Session_With_DefaultAgent_ExcludedTools()
180188
},
181189
});
182190

183-
await session.SendAsync(new MessageOptions { Prompt = "What is 1+1?" });
184-
await TestHelper.GetFinalAssistantMessageAsync(session);
185-
186-
var traffic = await Ctx.GetExchangesAsync();
187-
Assert.NotEmpty(traffic);
188-
189-
var toolNames = GetToolNames(traffic[0]);
190-
Assert.DoesNotContain("secret_tool", toolNames);
191+
try
192+
{
193+
var traffic = await SendAndWaitForExchangesAsync(
194+
session,
195+
new MessageOptions { Prompt = "What is 1+1?" });
196+
var toolNames = GetToolNames(traffic[0]);
197+
Assert.DoesNotContain("secret_tool", toolNames);
198+
}
199+
finally
200+
{
201+
await session.DisposeAsync();
202+
}
191203
}
192204

193205
[Fact]
@@ -539,11 +551,17 @@ public async Task Should_Create_Session_With_Custom_Config_Dir()
539551

540552
Assert.Matches(@"^[a-f0-9-]+$", session.SessionId);
541553

542-
// Session should work normally with custom config dir
543-
await session.SendAsync(new MessageOptions { Prompt = "What is 1+1?" });
544-
var assistantMessage = await TestHelper.GetFinalAssistantMessageAsync(session);
545-
Assert.NotNull(assistantMessage);
546-
Assert.Contains("2", assistantMessage!.Data.Content);
554+
try
555+
{
556+
// Session should work normally with custom config dir.
557+
var assistantMessage = await session.SendAndWaitAsync(new MessageOptions { Prompt = "What is 1+1?" });
558+
Assert.NotNull(assistantMessage);
559+
Assert.Contains("2", assistantMessage!.Data.Content);
560+
}
561+
finally
562+
{
563+
await session.DisposeAsync();
564+
}
547565
}
548566

549567
[Fact]

0 commit comments

Comments
 (0)