Skip to content

Commit 60868d7

Browse files
stephentoubCopilot
andcommitted
Address CodeQL findings on E2E tests and harness
- Bind awaited results in async helpers to silence `no-effect'' warnings (Python `await x` -> `_ = await x`). - Add explanatory comments to intentional `except ExceptionGroup: pass` blocks in test_rpc_server_e2e.py. - HookLifecycleAndOutputE2ETests: remove unused errorInputs container. - ClientOptionsE2ETests: dispose TcpListener via `using`. - RpcShellAndFleetE2ETests: simplify `Contains(...) == true` to a null-coalescing form. - SessionConfigE2ETests/SessionE2ETests: use `catch (Exception)` in `DisposeAsync` cleanup blocks instead of bare `catch`. - E2ETestContext.ResolveSymlinks: replace libc realpath/free P/Invoke with managed `DirectoryInfo.ResolveLinkTarget`-based component walk; preserves macOS `/var -> /private/var` resolution behavior. Also switches the fallback `catch` to typed exceptions. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 4090c17 commit 60868d7

11 files changed

Lines changed: 61 additions & 32 deletions

dotnet/test/E2E/ClientOptionsE2ETests.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -244,7 +244,7 @@ public void Should_Accept_SessionIdleTimeoutSeconds_Option()
244244

245245
private static int GetAvailableTcpPort()
246246
{
247-
var listener = new TcpListener(IPAddress.Loopback, 0);
247+
using var listener = new TcpListener(IPAddress.Loopback, 0);
248248
listener.Start();
249249
try
250250
{

dotnet/test/E2E/HookLifecycleAndOutputE2ETests.cs

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -109,15 +109,13 @@ public async Task Should_Invoke_OnSessionEnd_Hook_When_Session_Is_Disconnected()
109109
[Fact]
110110
public async Task Should_Invoke_OnErrorOccurred_Hook_When_Error_Occurs()
111111
{
112-
var errorInputs = new List<ErrorOccurredHookInput>();
113112
CopilotSession? session = null;
114113
session = await CreateSessionAsync(new SessionConfig
115114
{
116115
Hooks = new SessionHooks
117116
{
118117
OnErrorOccurred = (input, invocation) =>
119118
{
120-
errorInputs.Add(input);
121119
Assert.Equal(session!.SessionId, invocation.SessionId);
122120
Assert.True(input.Timestamp > 0);
123121
Assert.False(string.IsNullOrEmpty(input.Cwd));

dotnet/test/E2E/RpcShellAndFleetE2ETests.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,7 @@ public async Task Should_Start_Fleet_And_Complete_Custom_Tool_Task()
7171
Assert.Contains(
7272
messages.OfType<ToolExecutionCompleteEvent>(),
7373
message => message.Data.Success &&
74-
message.Data.Result?.Content.Contains(marker, StringComparison.Ordinal) == true);
74+
(message.Data.Result?.Content?.Contains(marker, StringComparison.Ordinal) ?? false));
7575
Assert.Contains(
7676
messages.OfType<AssistantMessageEvent>(),
7777
message => (message.Data.Content ?? string.Empty).Contains("fleet task", StringComparison.OrdinalIgnoreCase));

dotnet/test/E2E/SessionConfigE2ETests.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -289,7 +289,7 @@ public async Task Should_Create_Session_With_Custom_Provider_Config()
289289
{
290290
await session.DisposeAsync();
291291
}
292-
catch
292+
catch (Exception)
293293
{
294294
// disconnect may fail since the provider is fake
295295
}

dotnet/test/E2E/SessionE2ETests.cs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -858,7 +858,7 @@ public async Task Should_Create_Session_With_Custom_Provider()
858858
{
859859
await session.DisposeAsync();
860860
}
861-
catch
861+
catch (Exception)
862862
{
863863
// disconnect may fail since the provider is fake
864864
}
@@ -887,7 +887,7 @@ public async Task Should_Create_Session_With_Azure_Provider()
887887
{
888888
await session.DisposeAsync();
889889
}
890-
catch
890+
catch (Exception)
891891
{
892892
// disconnect may fail since the provider is fake
893893
}
@@ -915,7 +915,7 @@ public async Task Should_Resume_Session_With_Custom_Provider()
915915
{
916916
await session2.DisposeAsync();
917917
}
918-
catch
918+
catch (Exception)
919919
{
920920
// disconnect may fail since the provider is fake
921921
}

dotnet/test/Harness/E2ETestContext.cs

Lines changed: 45 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@
33
*--------------------------------------------------------------------------------------------*/
44

55
using System.Runtime.CompilerServices;
6-
using System.Runtime.InteropServices;
76
using System.Text.RegularExpressions;
87
using Microsoft.Extensions.Logging;
98

@@ -51,42 +50,68 @@ public static async Task<E2ETestContext> CreateAsync()
5150
return new E2ETestContext(homeDir, workDir, proxyUrl, proxy, repoRoot);
5251
}
5352

53+
/// <summary>
54+
/// Returns a canonical path with symlinks resolved in every directory
55+
/// component. .NET has no built-in equivalent of POSIX <c>realpath</c>
56+
/// that walks all parents, so we walk the components ourselves and use
57+
/// <see cref="DirectoryInfo.ResolveLinkTarget(bool)"/> on each one.
58+
/// On Windows, where the test temp paths don't traverse symlinks,
59+
/// <see cref="Path.GetFullPath(string)"/> is sufficient.
60+
/// </summary>
5461
private static string ResolveSymlinks(string path)
5562
{
5663
if (OperatingSystem.IsWindows())
5764
{
5865
return Path.GetFullPath(path);
5966
}
6067

61-
IntPtr resolved = IntPtr.Zero;
6268
try
6369
{
64-
resolved = NativeRealpath(path, IntPtr.Zero);
65-
if (resolved == IntPtr.Zero)
70+
var fullPath = Path.GetFullPath(path);
71+
var root = Path.GetPathRoot(fullPath);
72+
if (string.IsNullOrEmpty(root))
6673
{
67-
return Path.GetFullPath(path);
74+
return fullPath;
6875
}
69-
return Marshal.PtrToStringAnsi(resolved) ?? Path.GetFullPath(path);
76+
77+
var components = fullPath
78+
.Substring(root.Length)
79+
.Split(Path.DirectorySeparatorChar, StringSplitOptions.RemoveEmptyEntries);
80+
81+
var resolved = root;
82+
foreach (var component in components)
83+
{
84+
resolved = Path.Combine(resolved, component);
85+
try
86+
{
87+
var info = new DirectoryInfo(resolved);
88+
if (info.Exists && info.LinkTarget != null)
89+
{
90+
var target = info.ResolveLinkTarget(returnFinalTarget: true);
91+
if (target != null && !string.IsNullOrEmpty(target.FullName))
92+
{
93+
resolved = target.FullName;
94+
}
95+
}
96+
}
97+
catch (Exception ex) when (ex is IOException or UnauthorizedAccessException)
98+
{
99+
// Component we can't inspect; keep what we have and continue.
100+
}
101+
}
102+
103+
return resolved;
70104
}
71-
catch
105+
catch (Exception ex) when (ex is IOException
106+
or UnauthorizedAccessException
107+
or ArgumentException
108+
or NotSupportedException
109+
or PathTooLongException)
72110
{
73111
return Path.GetFullPath(path);
74112
}
75-
finally
76-
{
77-
if (resolved != IntPtr.Zero)
78-
{
79-
NativeFree(resolved);
80-
}
81-
}
82113
}
83114

84-
[DllImport("libc", EntryPoint = "realpath", CharSet = CharSet.Ansi, BestFitMapping = false, ThrowOnUnmappableChar = true)]
85-
private static extern IntPtr NativeRealpath([MarshalAs(UnmanagedType.LPStr)] string path, IntPtr resolved);
86-
87-
[DllImport("libc", EntryPoint = "free")]
88-
private static extern void NativeFree(IntPtr ptr);
89-
90115
private static string FindRepoRoot()
91116
{
92117
var dir = new DirectoryInfo(AppContext.BaseDirectory);

python/e2e/test_pending_work_resume_e2e.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -159,7 +159,7 @@ def original_tool_handler(args):
159159
await session1.send(
160160
"Use resume_permission_tool with value 'alpha', then reply with the result."
161161
)
162-
await captured_request
162+
_captured_permission_request = await captured_request
163163
permission_event = await permission_event_task
164164

165165
# Force-stop the suspended client without releasing the in-flight

python/e2e/test_rpc_mcp_and_skills_e2e.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@ def _assert_skill(skills, skill_name: str, *, enabled: bool):
6161

6262
async def _assert_failure(awaitable, expected: str) -> None:
6363
with pytest.raises(Exception) as excinfo:
64-
await awaitable
64+
_result = await awaitable
6565
assert expected.lower() in str(excinfo.value).lower()
6666

6767

python/e2e/test_rpc_server_e2e.py

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -104,6 +104,9 @@ async def test_should_call_rpc_models_list_with_typed_result(self, authed_ctx: E
104104
try:
105105
await client.stop()
106106
except ExceptionGroup:
107+
# Intentional: shutting down the per-test client can race the
108+
# CLI's own teardown and surface as an aggregated cancellation
109+
# error from anyio. We don't want it to fail the test.
107110
pass
108111

109112
async def test_should_call_rpc_account_get_quota_when_authenticated(
@@ -140,6 +143,9 @@ async def test_should_call_rpc_account_get_quota_when_authenticated(
140143
try:
141144
await client.stop()
142145
except ExceptionGroup:
146+
# Intentional: shutting down the per-test client can race the
147+
# CLI's own teardown and surface as an aggregated cancellation
148+
# error from anyio. We don't want it to fail the test.
143149
pass
144150

145151
async def test_should_call_rpc_tools_list_with_typed_result(self, ctx: E2ETestContext):

python/e2e/test_rpc_session_state_e2e.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ def _conversation_messages(events) -> list[tuple[str, str]]:
4343

4444
async def _assert_implemented_failure(awaitable, method: str) -> None:
4545
with pytest.raises(Exception) as excinfo:
46-
await awaitable
46+
_result = await awaitable
4747
assert f"Unhandled method {method}".lower() not in str(excinfo.value).lower()
4848

4949

0 commit comments

Comments
 (0)