Skip to content

Commit 4f09a71

Browse files
Fix unit stress test isolation flakes (#393)
Serialize persisted-state singleton tests and ensure UseCommandTests drains its expected unobserved task exception so unrelated unobserved-exception tests do not count it under stress. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 31c0b90 commit 4f09a71

5 files changed

Lines changed: 61 additions & 21 deletions

File tree

tests/Reactor.Tests/ComponentModelIntegrationTests.cs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,9 +10,9 @@ namespace Microsoft.UI.Reactor.Tests;
1010
/// component lifecycle (BeginRender → Render → FlushEffects → RunCleanups)
1111
/// through a shared ContextScope.
1212
/// </summary>
13-
// Shares the process-wide ApplicationPersistedScope.Default with
14-
// PersistedStateTests; both Clear() it in setup/teardown, so xUnit must
15-
// serialize them or a Clear() can race with another class's Set/TryGet.
13+
// Shares the process-wide ApplicationPersistedScope.Default with other
14+
// persistence tests; setup/teardown Clear() must not race another class's
15+
// Set/TryGet against the singleton.
1616
[Collection("PersistedStateCache")]
1717
public class ComponentModelIntegrationTests : IDisposable
1818
{

tests/Reactor.Tests/PersistedStateTests.cs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,9 +6,9 @@ namespace Microsoft.UI.Reactor.Tests;
66
/// <summary>
77
/// Phase 4 tests: PersistedStateCache unit tests and UsePersisted hook tests.
88
/// </summary>
9-
// Shares the process-wide ApplicationPersistedScope.Default with
10-
// ComponentModelIntegrationTests; both Clear() it in setup/teardown, so xUnit
11-
// must serialize them or a Clear() can race with another class's Set/TryGet.
9+
// Shares the process-wide ApplicationPersistedScope.Default with other
10+
// persistence tests; setup/teardown Clear() must not race another class's
11+
// Set/TryGet against the singleton.
1212
[Collection("PersistedStateCache")]
1313
public class PersistedStateTests : IDisposable
1414
{

tests/Reactor.Tests/TestIsolationCollections.cs

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,3 +19,12 @@ public sealed class ConsoleTestsCollection { }
1919
/// </summary>
2020
[CollectionDefinition("UnobservedTaskException", DisableParallelization = true)]
2121
public sealed class UnobservedTaskExceptionCollection { }
22+
23+
/// <summary>
24+
/// xUnit collection marker for tests that mutate
25+
/// <see cref="Microsoft.UI.Reactor.Core.ApplicationPersistedScope.Default"/>.
26+
/// The singleton is process-wide, so tests that clear or write it must not run
27+
/// concurrently with other tests that assert values remain present.
28+
/// </summary>
29+
[CollectionDefinition("PersistedStateCache", DisableParallelization = true)]
30+
public sealed class PersistedStateCacheCollection { }

tests/Reactor.Tests/UseCommandTests.cs

Lines changed: 45 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ namespace Microsoft.UI.Reactor.Tests;
66
/// <summary>
77
/// Tests for UseCommand hook — sync passthrough, async lifecycle, re-entrance guards.
88
/// </summary>
9+
[Collection("UnobservedTaskException")]
910
public class UseCommandTests
1011
{
1112
private static RenderContext CreateContext()
@@ -106,31 +107,60 @@ public async Task IsExecuting_Becomes_True_During_Execution()
106107
}
107108

108109
[Fact]
110+
[Trait("Category", "Threading")]
109111
public async Task Error_In_ExecuteAsync_Still_Resets_IsExecuting()
110112
{
111113
// Use a semaphore to observe re-render requests from setIsExecuting.
112114
// Task.Yield() doesn't reliably interleave with thread pool work items
113115
// under xUnit's synchronization context.
114-
var stateChanged = new SemaphoreSlim(0);
115-
var ctx = new RenderContext();
116-
ctx.BeginRender(() => stateChanged.Release());
117-
var cmd = new Command
116+
int unobserved = 0;
117+
EventHandler<UnobservedTaskExceptionEventArgs> handler = (_, e) =>
118118
{
119-
Label = "Save",
120-
ExecuteAsync = () => throw new InvalidOperationException("test error")
119+
if (e.Exception.InnerExceptions.Any(ex =>
120+
ex is InvalidOperationException { Message: "test error" }))
121+
{
122+
Interlocked.Increment(ref unobserved);
123+
e.SetObserved();
124+
}
121125
};
126+
TaskScheduler.UnobservedTaskException += handler;
127+
var stateChanged = new SemaphoreSlim(0);
128+
try
129+
{
130+
var ctx = new RenderContext();
131+
ctx.BeginRender(() => stateChanged.Release());
132+
var cmd = new Command
133+
{
134+
Label = "Save",
135+
ExecuteAsync = () => throw new InvalidOperationException("test error")
136+
};
122137

123-
var result = ctx.UseCommand(cmd);
124-
result.Execute!();
138+
var result = ctx.UseCommand(cmd);
139+
result.Execute!();
125140

126-
// Execute! synchronously sets IsExecuting=true (1st release), then Task.Run
127-
// catches the error and sets IsExecuting=false in finally (2nd release).
128-
await stateChanged.WaitAsync(TimeSpan.FromSeconds(5));
129-
await stateChanged.WaitAsync(TimeSpan.FromSeconds(5));
141+
// Execute! synchronously sets IsExecuting=true (1st release), then Task.Run
142+
// lets the error fault the background task while still resetting
143+
// IsExecuting=false in finally (2nd release).
144+
await stateChanged.WaitAsync(TimeSpan.FromSeconds(5));
145+
await stateChanged.WaitAsync(TimeSpan.FromSeconds(5));
130146

131-
Rerender(ctx);
132-
var result2 = ctx.UseCommand(cmd);
133-
Assert.False(result2.IsExecuting);
147+
Rerender(ctx);
148+
var result2 = ctx.UseCommand(cmd);
149+
Assert.False(result2.IsExecuting);
150+
151+
for (int i = 0; i < 10 && Volatile.Read(ref unobserved) == 0; i++)
152+
{
153+
GC.Collect();
154+
GC.WaitForPendingFinalizers();
155+
await Task.Delay(10);
156+
}
157+
Assert.Equal(1, Volatile.Read(ref unobserved));
158+
}
159+
finally
160+
{
161+
TaskScheduler.UnobservedTaskException -= handler;
162+
stateChanged.Dispose();
163+
}
134164
}
135165

136166
// ════════════════════════════════════════════════════════════════

tests/Reactor.Tests/WindowPersistedScopeIsolationTests.cs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ namespace Microsoft.UI.Reactor.Tests;
1111
/// type is the same instance <see cref="ReactorWindow.PersistedScope"/>
1212
/// surfaces, so isolation is a property of the scope object itself.
1313
/// </summary>
14+
[Collection("PersistedStateCache")]
1415
public class WindowPersistedScopeIsolationTests
1516
{
1617
[Fact]

0 commit comments

Comments
 (0)