Files
dotnet__skills/eng/skill-validator/tests/Shared/ConcurrencyLimiterTests.cs
Viktor Hofer 3fff1f2573 Skill-validator: perf, cancellation, and reliability improvements (#482)
* Skill-validator: perf, cancellation, and reliability improvements

Performance:
- Cache skill-validator-dist.tar.gz in CI (evaluation.yml and skill-check.yml)
  to skip .NET SDK setup, build, and tar on cache hit. Evaluate and comment-on-pr
  jobs restore from cache with artifact download fallback.
- Replace SemaphoreSlim with Lock in SessionDatabase for lighter-weight
  synchronous locking.

Cancellation:
- Thread CancellationToken from System.CommandLine entry point through the
  entire evaluate call chain: Run -> EvaluateTarget -> EvaluateSkill/EvaluateAgent
  -> ExecuteScenario/ExecuteAgentScenario -> ExecuteRun/ExecuteAgentRun, plus
  EvaluateSkillNoise -> ExecuteNoiseTest.
- Add CancellationToken parameter to AgentRunner.RunAgent, Judge.JudgeRun,
  PairwiseJudge.Judge, and OverfittingJudge.Analyze, forwarding to
  RetryHelper and LlmSession.SendAsync respectively.
- Pass CancellationToken to all ConcurrencyLimiter.RunAsync call sites so
  semaphore waits are cancellable.
- Re-throw OperationCanceledException at the top-level skill evaluation loop
  so Ctrl+C / CI cancellation propagates instead of being swallowed as a
  skill failure.

Reliability:
- Smart timeline truncation in Judge and PairwiseJudge: error events
  (session.error, runner.error) are always preserved during truncation
  regardless of position, with a summary marker showing event type breakdown.
- Add jitter (±25%) to RetryHelper exponential backoff to prevent thundering
  herd on rate-limit recovery.
- Add CancellationToken parameter to ConcurrencyLimiter.RunAsync<T>.
- Add ConcurrencyLimiter tests (6 tests) and RetryHelper jitter tests (2 tests).

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* Address PR review feedback

- RetryHelper: re-clamp delay after jitter so it never exceeds the
  remaining budget or MaxSingleDelayMs.
- Judge/PairwiseJudge: handle edge case where error count alone exceeds
  MaxTimelineEvents by trimming errors with a summary instead of
  unconditionally appending all of them.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* Add high-variance detection and refactor FailureKind to enum

- Add Statistics.CoefficientOfVariation() for measuring score consistency
- Flag scenarios with CV > 0.5 as high-variance in ScenarioComparison
- Show console warning and markdown footnote for high-variance scenarios
- Include high-variance scenario names in Comparator verdict reason
- Refactor FailureKind from magic strings to strongly-typed enum with
  JsonStringEnumMemberName attributes for backwards-compatible JSON output
- Add 6 new CV unit tests

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

---------

Co-authored-by: Viktor Hofer <vihofer@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
2026-03-31 13:58:28 +02:00

83 lines
3.0 KiB
C#

using SkillValidator.Shared;
namespace SkillValidator.Tests;
public class ConcurrencyLimiterTests
{
[Fact]
public async Task RunAsync_ReturnsResult()
{
using var limiter = new ConcurrencyLimiter(2);
var result = await limiter.RunAsync(() => Task.FromResult(42), TestContext.Current.CancellationToken);
Assert.Equal(42, result);
}
[Fact]
public async Task RunAsync_PropagatesExceptions()
{
using var limiter = new ConcurrencyLimiter(2);
await Assert.ThrowsAsync<InvalidOperationException>(() =>
limiter.RunAsync<int>(() => throw new InvalidOperationException("boom"), TestContext.Current.CancellationToken));
}
[Fact]
public async Task RunAsync_LimitsConcurrency()
{
using var limiter = new ConcurrencyLimiter(2);
var concurrentCount = 0;
var maxConcurrent = 0;
var lockObj = new object();
var tasks = Enumerable.Range(0, 10).Select(_ =>
limiter.RunAsync(async () =>
{
lock (lockObj)
{
concurrentCount++;
maxConcurrent = Math.Max(maxConcurrent, concurrentCount);
}
await Task.Delay(50, TestContext.Current.CancellationToken);
lock (lockObj) { concurrentCount--; }
return 1;
}, TestContext.Current.CancellationToken));
await Task.WhenAll(tasks);
Assert.True(maxConcurrent <= 2, $"Max concurrency was {maxConcurrent}, expected ≤ 2");
Assert.True(maxConcurrent >= 1, $"Max concurrency was {maxConcurrent}, expected ≥ 1");
}
[Fact]
public async Task RunAsync_ConcurrentFailures_AllSurfaced()
{
using var limiter = new ConcurrencyLimiter(5);
var tasks = Enumerable.Range(0, 5).Select(i =>
limiter.RunAsync<int>(() =>
throw new InvalidOperationException($"fail-{i}"), TestContext.Current.CancellationToken));
var ex = await Assert.ThrowsAsync<InvalidOperationException>(() => Task.WhenAll(tasks));
Assert.Contains("fail-", ex.Message);
}
[Fact]
public async Task RunAsync_SemaphoreReleasedOnFailure()
{
using var limiter = new ConcurrencyLimiter(1);
// First call throws
await Assert.ThrowsAsync<InvalidOperationException>(() =>
limiter.RunAsync<int>(() => throw new InvalidOperationException("first"), TestContext.Current.CancellationToken));
// Second call should still work (semaphore was released in finally)
var result = await limiter.RunAsync(() => Task.FromResult(42), TestContext.Current.CancellationToken);
Assert.Equal(42, result);
}
[Fact]
public async Task RunAsync_RespectsMinimumConcurrencyOfOne()
{
using var limiter = new ConcurrencyLimiter(0); // should clamp to 1
var result = await limiter.RunAsync(() => Task.FromResult("ok"), TestContext.Current.CancellationToken);
Assert.Equal("ok", result);
}
}