diff --git a/.claude/rules/csharp.md b/.claude/rules/csharp.md index f0a5f9070..143866c58 100644 --- a/.claude/rules/csharp.md +++ b/.claude/rules/csharp.md @@ -11,9 +11,9 @@ This rule file summarizes the C#-specific policies for this repository. ## Toolchain -1. **Formatting — CSharpier**: All C# source files must be formatted with CSharpier. Do not use `dotnet format`. Command: `dotnet tool run csharpier .` or `csharpier .` -2. **Linting — .NET Analyzers**: C# code must pass Roslyn/.NET analyzer diagnostics. Command: `msbuild .sln /t:Build /p:Configuration=Debug /p:Platform="Any CPU" /p:EnableNETAnalyzers=true /p:EnforceCodeStyleInBuild=true` -3. **Type Checking — Nullable Analysis**: Enable nullable reference types and fail on warnings. Command: `msbuild .sln /t:Build /p:Configuration=Debug /p:Platform="Any CPU" /p:Nullable=enable /p:TreatWarningsAsErrors=true` +1. **Formatting — CSharpier**: All C# source files must be formatted with CSharpier. Do not use `dotnet format`. Run `dotnet tool restore` first when the manifest tool has not been restored. Apply formatting with `dotnet tool run csharpier format .` and verify read-only with `dotnet tool run csharpier check .`. Always invoke through `dotnet tool run` so the manifest-pinned CSharpier version is used. +2. **Linting — .NET Analyzers**: C# code must pass Roslyn/.NET analyzer diagnostics. Command: `msbuild .sln /t:Rebuild /m /p:Configuration=Debug "/p:Platform=Any CPU" /p:EnableNETAnalyzers=true /p:EnforceCodeStyleInBuild=true`. `/t:Rebuild` is intentional for a warm local worktree: `/t:Build` can skip `CoreCompile` through MSBuild incrementality and exit 0 without running analyzers. CI may retain `/t:Build` on a cold checkout. +3. **Type Checking — Nullable Analysis**: Compiler and nullable-flow diagnostics must pass with warnings as errors. Command: `msbuild .sln /t:Rebuild /m /p:Configuration=Debug "/p:Platform=Any CPU" /p:TreatWarningsAsErrors=true`. `/t:Rebuild` is required locally so compiler and nullable-flow diagnostics actually run. Projects opt into nullable per file with `#nullable enable`; do not pass `/p:Nullable=enable`, which opts every unannotated file in at once. 4. **Testing — MSTest + Moq + FluentAssertions**: Run tests with: `vstest.console.exe /EnableCodeCoverage` Run the toolchain in order: format → lint → type-check → test. Restart from step 1 if any step fails or changes files. @@ -80,7 +80,7 @@ This repository adopts a fixed set of FIVE static-analysis packages, wired into ### Severity-first ordering invariant -All new analyzer rule severities are configured in `.editorconfig` at `severity = suggestion` (never `warning`/`error`) BEFORE any `` item is wired into a project. This is required because the type-check toolchain step runs `msbuild ... /p:Nullable=enable /p:TreatWarningsAsErrors=true`, which promotes any `warning`-severity analyzer diagnostic to a build error. Keeping new analyzer diagnostics at `suggestion` (message level) prevents the analyzer adoption from breaking the protected nullable gate. +All new analyzer rule severities are configured in `.editorconfig` at `severity = suggestion` (never `warning`/`error`) BEFORE any `` item is wired into a project. This is required because the type-check toolchain step runs `msbuild ... /p:TreatWarningsAsErrors=true`, which promotes any `warning`-severity analyzer diagnostic to a build error. Keeping new analyzer diagnostics at `suggestion` (message level) prevents the analyzer adoption from breaking the protected nullable gate. ### Deferred analyzer — SecurityCodeScan.VS2019 diff --git a/.claude/skills/csharp-qa-gate/SKILL.md b/.claude/skills/csharp-qa-gate/SKILL.md index 43ded1e93..0d3b005a0 100644 --- a/.claude/skills/csharp-qa-gate/SKILL.md +++ b/.claude/skills/csharp-qa-gate/SKILL.md @@ -27,11 +27,13 @@ Before invoking this gate, the agent must have: Run the full toolchain in this exact order. If any step fails or modifies files, fix the issue and restart from step 1. Do not stop the loop until all four steps complete without errors in a single pass. -1. `dotnet tool run csharpier .` -2. `msbuild .sln /t:Build /p:Configuration=Debug /p:Platform="Any CPU" /p:EnableNETAnalyzers=true /p:EnforceCodeStyleInBuild=true` -3. `msbuild .sln /t:Build /p:Configuration=Debug /p:Platform="Any CPU" /p:Nullable=enable /p:TreatWarningsAsErrors=true` +1. `dotnet tool restore` (when the manifest tool has not been restored), then `dotnet tool run csharpier format .` to apply formatting and `dotnet tool run csharpier check .` to verify read-only. +2. `msbuild .sln /t:Rebuild /m /p:Configuration=Debug "/p:Platform=Any CPU" /p:EnableNETAnalyzers=true /p:EnforceCodeStyleInBuild=true` +3. `msbuild .sln /t:Rebuild /m /p:Configuration=Debug "/p:Platform=Any CPU" /p:TreatWarningsAsErrors=true` 4. `vstest.console.exe /EnableCodeCoverage` +`/t:Rebuild /m` is intentional for the local gate: on a warm worktree `/t:Build` can skip `CoreCompile` through MSBuild incrementality and exit 0 without running analyzers or the compiler. CI may retain `/t:Build` on a cold checkout. Projects opt into nullable per file with `#nullable enable`; do not pass `/p:Nullable=enable`. + If the environment prevents running any tool, stop and report the change as **unverified**. Do not declare completion. ## Delta Requirements (Zero-Regression Hard Gate) diff --git a/UtilitiesCS.Test/Threading/TimeOutTask_AdditionalTests.cs b/UtilitiesCS.Test/Threading/TimeOutTask_AdditionalTests.cs index 71d2c69d6..281f91f94 100644 --- a/UtilitiesCS.Test/Threading/TimeOutTask_AdditionalTests.cs +++ b/UtilitiesCS.Test/Threading/TimeOutTask_AdditionalTests.cs @@ -2,18 +2,29 @@ using System.Threading; using System.Threading.Tasks; using FluentAssertions; +using Microsoft.Extensions.Time.Testing; using Microsoft.VisualStudio.TestTools.UnitTesting; namespace UtilitiesCS.Test { public partial class TimeOutTask_Tests { + /// + /// The source-completes-later tests race the timeout timer against the source completion. + /// Driving TimeoutAfter from a that is never advanced + /// makes the outcome deterministic: the timer cannot fire, so the source result is always + /// the one marshalled to the proxy. A real-clock timeout leaves the outcome dependent on + /// how long the runner thread is preempted between arming the timer and completing the + /// source, which is what failed under CI parallelism plus coverage instrumentation. + /// + private static FakeTimeProvider FrozenClock() => new FakeTimeProvider(); + [TestMethod] public async Task TimeoutAfter_GenericTask_ShouldPropagateFaultedSourceException_WhenSourceFaultsLater() { // Arrange var source = new TaskCompletionSource(); - var proxy = source.Task.TimeoutAfter(100); + var proxy = source.Task.TimeoutAfter(100, FrozenClock()); // Act source.SetException(new InvalidOperationException("boom")); @@ -28,7 +39,7 @@ public async Task TimeoutAfter_GenericTask_ShouldPropagateCancellation_WhenSourc { // Arrange var source = new TaskCompletionSource(); - var proxy = source.Task.TimeoutAfter(100); + var proxy = source.Task.TimeoutAfter(100, FrozenClock()); // Act source.SetCanceled(); @@ -43,7 +54,7 @@ public async Task TimeoutAfter_NonGenericTask_ShouldPropagateFaultedSourceExcept { // Arrange var source = new TaskCompletionSource(); - var proxy = ((Task)source.Task).TimeoutAfter(100); + var proxy = ((Task)source.Task).TimeoutAfter(100, FrozenClock()); // Act source.SetException(new InvalidOperationException("boom")); @@ -58,7 +69,7 @@ public async Task TimeoutAfter_NonGenericTask_ShouldPropagateCancellation_WhenSo { // Arrange var source = new TaskCompletionSource(); - var proxy = ((Task)source.Task).TimeoutAfter(100); + var proxy = ((Task)source.Task).TimeoutAfter(100, FrozenClock()); // Act source.SetCanceled(); @@ -68,6 +79,38 @@ public async Task TimeoutAfter_NonGenericTask_ShouldPropagateCancellation_WhenSo await act.Should().ThrowAsync(); } + [TestMethod] + public async Task TimeoutAfter_GenericTask_ShouldFaultWithTimeout_WhenInjectedClockPassesTheDeadline() + { + // Arrange + var clock = FrozenClock(); + var source = new TaskCompletionSource(); + var proxy = source.Task.TimeoutAfter(100, clock); + + // Act + clock.Advance(TimeSpan.FromMilliseconds(100)); + + // Assert + Func act = async () => await proxy; + await act.Should().ThrowAsync(); + } + + [TestMethod] + public async Task TimeoutAfter_NonGenericTask_ShouldFaultWithTimeout_WhenInjectedClockPassesTheDeadline() + { + // Arrange + var clock = FrozenClock(); + var source = new TaskCompletionSource(); + var proxy = ((Task)source.Task).TimeoutAfter(100, clock); + + // Act + clock.Advance(TimeSpan.FromMilliseconds(100)); + + // Assert + Func act = async () => await proxy; + await act.Should().ThrowAsync(); + } + [TestMethod] public async Task RunWithTimeout_Func_ShouldReturnDefault_WhenTaskIsCanceledWithoutRetries() { diff --git a/UtilitiesCS/Threading/TimeOutTask.cs b/UtilitiesCS/Threading/TimeOutTask.cs index f9e97dcf6..22cefecc6 100644 --- a/UtilitiesCS/Threading/TimeOutTask.cs +++ b/UtilitiesCS/Threading/TimeOutTask.cs @@ -835,10 +835,16 @@ int repeatAttempts /// /// /// + /// + /// Clock used to arm the timeout timer. When null, is used + /// (production); tests pass a FakeTimeProvider so the timeout fires only when the fake + /// clock is advanced, making the timeout-versus-completion race deterministic. + /// /// public static Task TimeoutAfter( this Task task, - int millisecondsTimeout + int millisecondsTimeout, + TimeProvider? timeProvider = null ) { // Short-circuit #1: infinite timeout or task already completed @@ -861,7 +867,7 @@ int millisecondsTimeout } // Set up a timer to complete after the specified timeout period - Timer timer = new Timer( + ITimer timer = (timeProvider ?? TimeProvider.System).CreateTimer( state => { // Recover your state information @@ -871,8 +877,8 @@ int millisecondsTimeout myTcs.TrySetException(new TimeoutException()); }, tcs, - millisecondsTimeout, - Timeout.Infinite + TimeSpan.FromMilliseconds(millisecondsTimeout), + Timeout.InfiniteTimeSpan ); // Wire up the logic for what happens when source task completes @@ -880,7 +886,7 @@ int millisecondsTimeout (antecedent, state) => { // Recover our state data - var tuple = (Tuple>)state; + var tuple = (Tuple>)state; // Cancel the Timer tuple.Item1.Dispose(); @@ -915,7 +921,18 @@ public static Task TimeoutAfter(this Task task, int millisecondsTimeout, int rep return result!; } - public static Task TimeoutAfter(this Task task, int millisecondsTimeout) + /// + /// + /// + /// Clock used to arm the timeout timer. When null, is used + /// (production); tests pass a FakeTimeProvider so the timeout fires only when the fake + /// clock is advanced, making the timeout-versus-completion race deterministic. + /// + public static Task TimeoutAfter( + this Task task, + int millisecondsTimeout, + TimeProvider? timeProvider = null + ) { // Short-circuit #1: infinite timeout or task already completed if (task.IsCompleted || (millisecondsTimeout == Timeout.Infinite)) @@ -937,7 +954,7 @@ public static Task TimeoutAfter(this Task task, int millisecondsTimeout) } // Set up a timer to complete after the specified timeout period - Timer timer = new Timer( + ITimer timer = (timeProvider ?? TimeProvider.System).CreateTimer( state => { // Recover your state information @@ -947,8 +964,8 @@ public static Task TimeoutAfter(this Task task, int millisecondsTimeout) myTcs.TrySetException(new TimeoutException()); }, tcs, - millisecondsTimeout, - Timeout.Infinite + TimeSpan.FromMilliseconds(millisecondsTimeout), + Timeout.InfiniteTimeSpan ); // Wire up the logic for what happens when source task completes @@ -956,7 +973,7 @@ public static Task TimeoutAfter(this Task task, int millisecondsTimeout) (antecedent, state) => { // Recover our state data - var tuple = (Tuple>)state; + var tuple = (Tuple>)state; // Cancel the Timer tuple.Item1.Dispose();