Skip to content

Commit 40dbb31

Browse files
KSemenenkoKSemenenko
committed
Make CLI probe cleanup tests deterministic
1 parent b42d3bb commit 40dbb31

2 files changed

Lines changed: 97 additions & 33 deletions

File tree

‎ClaudeCodeSharpSDK.Tests/Unit/BoundedCliProcessProbeTests.cs‎

Lines changed: 84 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,10 @@
11
using System.Diagnostics;
2+
using System.Globalization;
23
using ManagedCode.ClaudeCodeSharpSDK.Internal;
34

45
namespace ManagedCode.ClaudeCodeSharpSDK.Tests.Unit;
56

7+
[NotInParallel]
68
public class BoundedCliProcessProbeTests
79
{
810
private const string ShellExecutableUnix = "/bin/sh";
@@ -13,11 +15,12 @@ public class BoundedCliProcessProbeTests
1315
private const string SentinelValue = "explicit-environment-value";
1416
private const string OverflowMessage = "CLI metadata process exceeded the configured output limit.";
1517
private const string TimeoutMessage = "CLI metadata process did not exit within the configured time limit.";
18+
private const string OutputTimeoutMessage = "CLI metadata process output did not close within the configured time limit.";
1619
private const string OutputCleanupFailedMessage = "CLI metadata process output readers did not stop within the configured time limit.";
1720
private const string ProbeBlockedMessage = "CLI metadata process probe is blocked by an earlier incomplete cleanup.";
1821
private const string ProbeBusyMessage = "CLI metadata process probe could not acquire its bounded process lease.";
1922
private const string LinuxFixtureSkipReason = "The detached inherited-pipe fixture requires Linux setsid.";
20-
private const string DetachedChildCommandPrefix = "/usr/bin/setsid /bin/sleep 30 >&2 & echo $! > '";
23+
private const string DetachedChildCommandPrefix = "/usr/bin/setsid /bin/sleep 30 & echo $! > '";
2124
private const string DetachedChildCommandSuffix = "'; exit 0";
2225
private const string FixtureDirectoryPrefix = "BoundedCliProcessProbe-";
2326
private const string ChildProcessIdFilePattern = "child-*.pid";
@@ -26,7 +29,7 @@ public class BoundedCliProcessProbeTests
2629
private const string StandardInputEofCommandUnix = "cat >/dev/null";
2730
private const string StandardInputEofCommandWindows = "more >NUL";
2831
private const int DetachedPipeProbeCount = 3;
29-
private static readonly TimeSpan ConcurrentProbeTimeout = TimeSpan.FromSeconds(3);
32+
private static readonly TimeSpan ConcurrentLeaseWait = TimeSpan.FromMilliseconds(100);
3033
private const string SentinelCommandUnix = "printf '%s' \"$SDK_METADATA_PROBE_SENTINEL\"";
3134
private const string SentinelCommandWindows = "echo %SDK_METADATA_PROBE_SENTINEL%";
3235
private const string PressureCommandUnix =
@@ -37,6 +40,10 @@ public class BoundedCliProcessProbeTests
3740
private const string OverflowCommandWindows = "for /L %i in (1,1,200) do @<nul set /p =0123456789";
3841
private const string LongRunningCommandUnix = "sleep 20";
3942
private const string LongRunningCommandWindows = "ping -n 21 127.0.0.1 >NUL";
43+
private const string ActiveLeaseCommandUnix = "sleep 2";
44+
private const string ActiveLeaseCommandWindows = "ping -n 3 127.0.0.1 >NUL";
45+
private const string CompletedProbeCommandUnix = "printf probe";
46+
private const string CompletedProbeCommandWindows = "echo probe";
4047
private static readonly TimeSpan ProbeTimeout = TimeSpan.FromMilliseconds(400);
4148
private static readonly TimeSpan ProcessTimeout = TimeSpan.FromSeconds(10);
4249

@@ -125,8 +132,8 @@ public async Task Run_RejectsConcurrentProcessBeforeStartingIt()
125132
firstReadersStarted.TrySetResult();
126133
}
127134
};
128-
var firstProbe = Task.Run(() => Run(LongRunningCommandUnix, LongRunningCommandWindows,
129-
environment: null, inheritEnvironmentVariables: true, timeout: ConcurrentProbeTimeout,
135+
var firstProbe = Task.Run(() => Run(ActiveLeaseCommandUnix, ActiveLeaseCommandWindows,
136+
environment: null, inheritEnvironmentVariables: true, timeout: ProcessTimeout,
130137
readerCountChanged: firstReaderCountChanged));
131138

132139
await firstReadersStarted.Task.WaitAsync(ProcessTimeout);
@@ -142,24 +149,45 @@ public async Task Run_RejectsConcurrentProcessBeforeStartingIt()
142149
timeout: ProbeTimeout,
143150
maximumOutputCharacters: 64,
144151
readerCountChanged: secondReaderCountChanged,
145-
leaseAcquisitionTimeout: ProbeTimeout)).ThrowsException();
152+
leaseAcquisitionTimeout: ConcurrentLeaseWait)).ThrowsException();
146153

147154
await Assert.That(secondException).IsTypeOf<InvalidOperationException>();
148155
await Assert.That(secondException!.Message).IsEqualTo(ProbeBusyMessage);
149156
await Assert.That(secondReaderCount).IsEqualTo(0);
150157

151-
Exception? firstException = null;
152-
try
153-
{
154-
await firstProbe;
155-
}
156-
catch (Exception exception)
157-
{
158-
firstException = exception;
159-
}
158+
var firstResult = await firstProbe;
159+
await Assert.That(firstResult.ExitCode).IsEqualTo(0);
160+
await Assert.That(BoundedCliProcessProbe.PendingReaderCleanupCount).IsEqualTo(0);
161+
}
160162

161-
await Assert.That(firstException).IsTypeOf<TimeoutException>();
163+
[Test]
164+
public async Task Run_RetainedReaderCleanupLeaseBlocksLaunchUntilBothReadersSettle()
165+
{
166+
var drains = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously);
167+
var leaseReleased = BoundedCliProcessProbe.RetainProbeLeaseForTesting(drains.Task);
168+
await Assert.That(BoundedCliProcessProbe.PendingReaderCleanupCount).IsEqualTo(1);
169+
170+
var readersStarted = 0;
171+
var exception = await Assert.That(() => BoundedCliProcessProbe.Run(
172+
OperatingSystem.IsWindows() ? Path.Combine(Environment.SystemDirectory, ShellExecutableWindows) : ShellExecutableUnix,
173+
[OperatingSystem.IsWindows() ? ShellArgumentWindows : ShellArgumentUnix,
174+
OperatingSystem.IsWindows() ? CompletedProbeCommandWindows : CompletedProbeCommandUnix],
175+
environment: null,
176+
inheritEnvironmentVariables: true,
177+
timeout: ProbeTimeout,
178+
maximumOutputCharacters: 64,
179+
readerCountChanged: delta => Interlocked.Add(ref readersStarted, delta))).ThrowsException();
180+
181+
await Assert.That(exception).IsTypeOf<InvalidOperationException>();
182+
await Assert.That(exception!.Message).IsEqualTo(ProbeBlockedMessage);
183+
await Assert.That(readersStarted).IsEqualTo(0);
184+
185+
drains.SetResult();
186+
await leaseReleased.WaitAsync(ProcessTimeout);
162187
await Assert.That(BoundedCliProcessProbe.PendingReaderCleanupCount).IsEqualTo(0);
188+
var result = Run(CompletedProbeCommandUnix, CompletedProbeCommandWindows,
189+
environment: null, inheritEnvironmentVariables: true, timeout: ProbeTimeout);
190+
await Assert.That(result.ExitCode).IsEqualTo(0);
163191
}
164192

165193
[Test]
@@ -202,26 +230,50 @@ public async Task Run_FailsWithinBoundWhenExitedRootHasDescendantHoldingPipes()
202230
leaseAcquisitionTimeout: ProcessTimeout)).ThrowsException();
203231
stopwatch.Stop();
204232

205-
await Assert.That(exception).IsTypeOf<InvalidOperationException>();
206-
await Assert.That(exception!.Message).IsEqualTo(OutputCleanupFailedMessage);
207233
await Assert.That(stopwatch.Elapsed).IsLessThan(ProcessTimeout);
208-
await Assert.That(activeReaders).IsEqualTo(2);
209-
await Assert.That(BoundedCliProcessProbe.PendingReaderCleanupCount).IsEqualTo(1);
234+
var childProcessId = int.Parse(File.ReadAllText(childProcessIdPath), CultureInfo.InvariantCulture);
235+
using (var child = Process.GetProcessById(childProcessId))
236+
{
237+
await Assert.That(child.HasExited).IsFalse();
238+
}
210239

211-
var blockedChildPath = Path.Combine(sandbox, $"blocked-{attempt}.pid");
212-
var blockedCommand = string.Concat(DetachedChildCommandPrefix, blockedChildPath,
213-
DetachedChildCommandSuffix);
214-
var blockedException = await Assert.That(() => BoundedCliProcessProbe.Run(
215-
ShellExecutableUnix,
216-
[ShellArgumentUnix, blockedCommand],
217-
environment: null,
218-
inheritEnvironmentVariables: true,
219-
timeout: ProbeTimeout,
220-
maximumOutputCharacters: 64)).ThrowsException();
240+
if (exception is TimeoutException)
241+
{
242+
await Assert.That(exception.Message).IsEqualTo(OutputTimeoutMessage);
243+
await Assert.That(activeReaders).IsEqualTo(0);
244+
await Assert.That(BoundedCliProcessProbe.PendingReaderCleanupCount).IsEqualTo(0);
245+
var subsequentProbe = BoundedCliProcessProbe.Run(
246+
ShellExecutableUnix,
247+
[ShellArgumentUnix, CompletedProbeCommandUnix],
248+
environment: null,
249+
inheritEnvironmentVariables: true,
250+
timeout: ProbeTimeout,
251+
maximumOutputCharacters: 64);
252+
await Assert.That(subsequentProbe.ExitCode).IsEqualTo(0);
253+
}
254+
else
255+
{
256+
await Assert.That(exception).IsTypeOf<InvalidOperationException>();
257+
await Assert.That(exception!.Message).IsEqualTo(OutputCleanupFailedMessage);
258+
await Assert.That(activeReaders > 0 && activeReaders <= 2).IsTrue();
259+
await Assert.That(BoundedCliProcessProbe.PendingReaderCleanupCount).IsEqualTo(1);
260+
261+
var blockedChildPath = Path.Combine(sandbox, $"blocked-{attempt}.pid");
262+
var blockedCommand = string.Concat(DetachedChildCommandPrefix, blockedChildPath,
263+
DetachedChildCommandSuffix);
264+
var blockedException = await Assert.That(() => BoundedCliProcessProbe.Run(
265+
ShellExecutableUnix,
266+
[ShellArgumentUnix, blockedCommand],
267+
environment: null,
268+
inheritEnvironmentVariables: true,
269+
timeout: ProbeTimeout,
270+
maximumOutputCharacters: 64)).ThrowsException();
271+
272+
await Assert.That(blockedException).IsTypeOf<InvalidOperationException>();
273+
await Assert.That(blockedException!.Message).IsEqualTo(ProbeBlockedMessage);
274+
await Assert.That(File.Exists(blockedChildPath)).IsFalse();
275+
}
221276

222-
await Assert.That(blockedException).IsTypeOf<InvalidOperationException>();
223-
await Assert.That(blockedException!.Message).IsEqualTo(ProbeBlockedMessage);
224-
await Assert.That(File.Exists(blockedChildPath)).IsFalse();
225277
StopChildProcess(childProcessIdPath);
226278
await readersDrained.Task.WaitAsync(ProcessTimeout);
227279
await Assert.That(activeReaders).IsEqualTo(0);

‎ClaudeCodeSharpSDK/Internal/BoundedCliProcessProbe.cs‎

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,14 @@ internal static int PendingReaderCleanupCount
3030
}
3131
}
3232

33+
internal static Task RetainProbeLeaseForTesting(Task drains)
34+
{
35+
ArgumentNullException.ThrowIfNull(drains);
36+
var probeGateLease = ProbeGateLease.Acquire(1);
37+
probeGateLease.ReleaseWhenDrained(drains);
38+
return probeGateLease.ReleaseCompletion;
39+
}
40+
3341
public static CliProcessProbeResult Run(
3442
string executablePath,
3543
IReadOnlyList<string> arguments,
@@ -312,6 +320,9 @@ private static int GetTimeoutMilliseconds(TimeSpan timeout) =>
312320
private sealed class ProbeGateLease : IDisposable
313321
{
314322
private bool _retainedForCleanup;
323+
private readonly TaskCompletionSource _releaseCompletion = new(TaskCreationOptions.RunContinuationsAsynchronously);
324+
325+
public Task ReleaseCompletion => _releaseCompletion.Task;
315326

316327
public static ProbeGateLease Acquire(int leaseTimeoutMilliseconds)
317328
{
@@ -349,10 +360,11 @@ public void ReleaseWhenDrained(Task drains)
349360
}
350361

351362
_ = drains.ContinueWith(
352-
static task =>
363+
task =>
353364
{
354365
_ = task.Exception;
355366
ReleaseProbeLease(retainedCleanup: true);
367+
_releaseCompletion.TrySetResult();
356368
},
357369
CancellationToken.None,
358370
TaskContinuationOptions.ExecuteSynchronously,

0 commit comments

Comments
 (0)