Skip to content

Commit 9dbac6e

Browse files
chore: Stabilize ReactiveUI.Tests with state restoration and de-parallelization (#4168)
Co-authored-by: copilot-swe-agent[bot] <[email protected]> Co-authored-by: glennawatson <[email protected]>
1 parent d59db7c commit 9dbac6e

17 files changed

+2490
-0
lines changed

dotnet-install.sh

Lines changed: 1888 additions & 0 deletions
Large diffs are not rendered by default.

src/ReactiveUI.Tests/AutoPersist/AutoPersistHelperTest.cs

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,13 @@ namespace ReactiveUI.Tests;
1212
/// <summary>
1313
/// Tests the AutoPersistHelper.
1414
/// </summary>
15+
/// <remarks>
16+
/// This test fixture is marked as NonParallelizable because it uses HostTestFixture
17+
/// which depends on ICreatesObservableForProperty from the service locator.
18+
/// The service locator state must not be mutated concurrently by parallel tests.
19+
/// </remarks>
20+
[TestFixture]
21+
[NonParallelizable]
1522
public class AutoPersistHelperTest
1623
{
1724
/// <summary>

src/ReactiveUI.Tests/AwaiterTest.cs

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,13 +3,36 @@
33
// The .NET Foundation licenses this file to you under the MIT license.
44
// See the LICENSE file in the project root for full license information.
55

6+
using ReactiveUI.Tests.Infrastructure.StaticState;
7+
68
namespace ReactiveUI.Tests;
79

810
/// <summary>
911
/// Tests the awaiters.
1012
/// </summary>
13+
/// <remarks>
14+
/// This test fixture is marked as NonParallelizable because it accesses RxApp.TaskpoolScheduler,
15+
/// which is global static state. While this test only reads the scheduler, marking it as
16+
/// NonParallelizable ensures no interference with other tests that might modify scheduler state.
17+
/// </remarks>
18+
[TestFixture]
19+
[NonParallelizable]
1120
public class AwaiterTest
1221
{
22+
private RxAppSchedulersScope? _schedulersScope;
23+
24+
[SetUp]
25+
public void SetUp()
26+
{
27+
_schedulersScope = new RxAppSchedulersScope();
28+
}
29+
30+
[TearDown]
31+
public void TearDown()
32+
{
33+
_schedulersScope?.Dispose();
34+
}
35+
1336
/// <summary>
1437
/// A smoke test for Awaiters.
1538
/// </summary>

src/ReactiveUI.Tests/Commands/ReactiveCommandTest.cs

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,20 +7,41 @@
77
using DynamicData;
88
using Microsoft.Reactive.Testing;
99
using ReactiveUI.Testing;
10+
using ReactiveUI.Tests.Infrastructure.StaticState;
1011

1112
namespace ReactiveUI.Tests;
1213

1314
/// <summary>
1415
/// Tests for the ReactiveCommand class.
1516
/// </summary>
17+
/// <remarks>
18+
/// This test fixture is marked as NonParallelizable because it calls RxApp.EnsureInitialized()
19+
/// in the constructor, which initializes global static state including the service locator
20+
/// and schedulers. This state must not be concurrently initialized by parallel tests.
21+
/// </remarks>
1622
[TestFixture]
23+
[NonParallelizable]
1724
public class ReactiveCommandTest
1825
{
26+
private RxAppSchedulersScope? _schedulersScope;
27+
1928
public ReactiveCommandTest()
2029
{
2130
RxApp.EnsureInitialized();
2231
}
2332

33+
[SetUp]
34+
public void SetUp()
35+
{
36+
_schedulersScope = new RxAppSchedulersScope();
37+
}
38+
39+
[TearDown]
40+
public void TearDown()
41+
{
42+
_schedulersScope?.Dispose();
43+
}
44+
2445
/// <summary>
2546
/// A test that determines whether this instance [can execute changed is available via ICommand].
2647
/// </summary>
Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,69 @@
1+
// Copyright (c) 2025 .NET Foundation and Contributors. All rights reserved.
2+
// Licensed to the .NET Foundation under one or more agreements.
3+
// The .NET Foundation licenses this file to you under the MIT license.
4+
// See the LICENSE file in the project root for full license information.
5+
6+
namespace ReactiveUI.Tests.Infrastructure.StaticState;
7+
8+
/// <summary>
9+
/// A disposable scope that snapshots and restores MessageBus.Current static state.
10+
/// Use this in test fixtures that read or modify MessageBus.Current to ensure
11+
/// static state is properly restored after tests complete.
12+
/// </summary>
13+
/// <remarks>
14+
/// This helper is necessary because MessageBus.Current maintains a static/global reference
15+
/// that can leak between parallel test executions, causing intermittent failures.
16+
/// Tests using this scope should also be marked with [NonParallelizable] to prevent
17+
/// concurrent modifications to the shared state.
18+
/// </remarks>
19+
/// <example>
20+
/// <code>
21+
/// [TestFixture]
22+
/// [NonParallelizable]
23+
/// public class MyTests
24+
/// {
25+
/// private MessageBusScope _messageBusScope;
26+
///
27+
/// [SetUp]
28+
/// public void SetUp()
29+
/// {
30+
/// _messageBusScope = new MessageBusScope();
31+
/// // Now safe to use MessageBus.Current
32+
/// }
33+
///
34+
/// [TearDown]
35+
/// public void TearDown()
36+
/// {
37+
/// _messageBusScope?.Dispose();
38+
/// }
39+
/// }
40+
/// </code>
41+
/// </example>
42+
public sealed class MessageBusScope : IDisposable
43+
{
44+
private readonly IMessageBus _previousMessageBus;
45+
private bool _disposed;
46+
47+
/// <summary>
48+
/// Initializes a new instance of the <see cref="MessageBusScope"/> class.
49+
/// Snapshots the current MessageBus.Current state.
50+
/// </summary>
51+
public MessageBusScope()
52+
{
53+
_previousMessageBus = MessageBus.Current;
54+
}
55+
56+
/// <summary>
57+
/// Restores the MessageBus.Current state to what it was when this scope was created.
58+
/// </summary>
59+
public void Dispose()
60+
{
61+
if (_disposed)
62+
{
63+
return;
64+
}
65+
66+
MessageBus.Current = _previousMessageBus;
67+
_disposed = true;
68+
}
69+
}
Lines changed: 144 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,144 @@
1+
# Static State Test Isolation
2+
3+
This directory contains helper classes for managing static/global state in ReactiveUI tests.
4+
5+
## Problem
6+
7+
ReactiveUI uses several static/global entry points for configuration:
8+
- `RxApp.MainThreadScheduler` and `RxApp.TaskpoolScheduler`
9+
- `RxApp.EnsureInitialized()` (initializes the service locator)
10+
- `MessageBus.Current`
11+
- `Locator.Current` / `Locator.CurrentMutable` (from Splat)
12+
13+
When tests access or modify these static states, they can cause interference between test executions, leading to intermittent failures and hidden state pollution.
14+
15+
## Solution: NonParallelizable + State Restoration
16+
17+
Tests that use static state should be marked `[NonParallelizable]` **AND** use state restoration scopes to ensure clean state between tests.
18+
19+
### Available Helper Scopes
20+
21+
#### 1. RxAppSchedulersScope
22+
23+
Snapshots and restores `RxApp.MainThreadScheduler` and `RxApp.TaskpoolScheduler`.
24+
25+
**Usage:**
26+
```csharp
27+
[TestFixture]
28+
[NonParallelizable]
29+
public class MyTests
30+
{
31+
private RxAppSchedulersScope? _schedulersScope;
32+
33+
[SetUp]
34+
public void SetUp()
35+
{
36+
_schedulersScope = new RxAppSchedulersScope();
37+
// Now safe to modify RxApp schedulers
38+
}
39+
40+
[TearDown]
41+
public void TearDown()
42+
{
43+
_schedulersScope?.Dispose();
44+
}
45+
}
46+
```
47+
48+
#### 2. MessageBusScope
49+
50+
Snapshots and restores `MessageBus.Current`.
51+
52+
**Usage:**
53+
```csharp
54+
[TestFixture]
55+
[NonParallelizable]
56+
public class MyTests
57+
{
58+
private MessageBusScope? _messageBusScope;
59+
60+
[SetUp]
61+
public void SetUp()
62+
{
63+
_messageBusScope = new MessageBusScope();
64+
// Now safe to use or replace MessageBus.Current
65+
}
66+
67+
[TearDown]
68+
public void TearDown()
69+
{
70+
_messageBusScope?.Dispose();
71+
}
72+
}
73+
```
74+
75+
#### 3. StaticStateScope (Generic)
76+
77+
A generic helper for capturing and restoring arbitrary static state using getter/setter pairs.
78+
79+
**Usage:**
80+
```csharp
81+
[TestFixture]
82+
[NonParallelizable]
83+
public class MyTests
84+
{
85+
private StaticStateScope? _stateScope;
86+
87+
[SetUp]
88+
public void SetUp()
89+
{
90+
_stateScope = new StaticStateScope(
91+
() => MyClass.StaticProperty,
92+
(object? value) => MyClass.StaticProperty = value,
93+
() => AnotherClass.StaticField,
94+
(object? value) => AnotherClass.StaticField = value);
95+
}
96+
97+
[TearDown]
98+
public void TearDown()
99+
{
100+
_stateScope?.Dispose();
101+
}
102+
}
103+
```
104+
105+
## When to Use These Scopes
106+
107+
**Always use state restoration scopes when:**
108+
109+
1. The test calls `RxApp.EnsureInitialized()`
110+
2. The test modifies `RxApp` properties (schedulers, SuspensionHost, etc.)
111+
3. The test modifies `MessageBus.Current`
112+
4. The test accesses or modifies `Locator.CurrentMutable`
113+
5. The test creates instances that depend on service locator registrations
114+
115+
**Why both NonParallelizable AND state restoration?**
116+
117+
- `[NonParallelizable]` prevents concurrent access issues
118+
- State restoration ensures clean state even when tests run sequentially
119+
- This prevents hidden state pollution that can cause issues in future test runs
120+
121+
## Important Notes
122+
123+
1. **Always mark test fixtures as `[NonParallelizable]`** when using these scopes
124+
2. **State restoration does NOT make tests safe for parallel execution** - it only ensures cleanup
125+
3. **For Splat's Locator**: Due to API complexity, use `StaticStateScope` or manual cleanup if needed
126+
127+
## Test Fixtures Already Marked as NonParallelizable
128+
129+
The following test fixtures are marked `[NonParallelizable]` because they use static/global state:
130+
131+
- `AutoPersistHelperTest` - uses HostTestFixture which depends on service locator
132+
- `MessageBusTest` - uses Locator.CurrentMutable and MessageBus.Current
133+
- `RandomTests` - uses RxApp, Locator.CurrentMutable, MessageBus.Current
134+
- `RxAppTest` - accesses RxApp.MainThreadScheduler
135+
- `ReactiveCommandTest` - calls RxApp.EnsureInitialized() in constructor
136+
- `PocoObservableForPropertyTests` - calls RxApp.EnsureInitialized()
137+
- `AwaiterTest` - accesses RxApp.TaskpoolScheduler
138+
- `RxAppDependencyObjectTests` - calls RxApp.EnsureInitialized() and accesses Locator.Current
139+
- `RoutedViewHostTests` - uses Locator.CurrentMutable to register services
140+
- `ViewModelViewHostTests` - uses Locator.CurrentMutable.Register()
141+
- `WpfCommandBindingImplementationTests` - registers test logger in Locator
142+
- `DefaultPropertyBindingTests` - calls RxApp.EnsureInitialized() in constructor
143+
144+
Each of these fixtures has XML documentation explaining the specific reason for being NonParallelizable.
Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,72 @@
1+
// Copyright (c) 2025 .NET Foundation and Contributors. All rights reserved.
2+
// Licensed to the .NET Foundation under one or more agreements.
3+
// The .NET Foundation licenses this file to you under the MIT license.
4+
// See the LICENSE file in the project root for full license information.
5+
6+
namespace ReactiveUI.Tests.Infrastructure.StaticState;
7+
8+
/// <summary>
9+
/// A disposable scope that snapshots and restores RxApp scheduler state.
10+
/// Use this in test fixtures that modify RxApp.MainThreadScheduler or RxApp.TaskpoolScheduler
11+
/// to ensure static state is properly restored after tests complete.
12+
/// </summary>
13+
/// <remarks>
14+
/// This helper is necessary because RxApp maintains static/global scheduler references
15+
/// that can leak between parallel test executions, causing intermittent failures.
16+
/// Tests using this scope should also be marked with [NonParallelizable] to prevent
17+
/// concurrent modifications to the shared state.
18+
/// </remarks>
19+
/// <example>
20+
/// <code>
21+
/// [TestFixture]
22+
/// [NonParallelizable]
23+
/// public class MyTests
24+
/// {
25+
/// private RxAppSchedulersScope _schedulersScope;
26+
///
27+
/// [SetUp]
28+
/// public void SetUp()
29+
/// {
30+
/// _schedulersScope = new RxAppSchedulersScope();
31+
/// // Now safe to modify RxApp schedulers
32+
/// }
33+
///
34+
/// [TearDown]
35+
/// public void TearDown()
36+
/// {
37+
/// _schedulersScope?.Dispose();
38+
/// }
39+
/// }
40+
/// </code>
41+
/// </example>
42+
public sealed class RxAppSchedulersScope : IDisposable
43+
{
44+
private readonly IScheduler _mainThreadScheduler;
45+
private readonly IScheduler _taskpoolScheduler;
46+
private bool _disposed;
47+
48+
/// <summary>
49+
/// Initializes a new instance of the <see cref="RxAppSchedulersScope"/> class.
50+
/// Snapshots the current RxApp scheduler state.
51+
/// </summary>
52+
public RxAppSchedulersScope()
53+
{
54+
_mainThreadScheduler = RxApp.MainThreadScheduler;
55+
_taskpoolScheduler = RxApp.TaskpoolScheduler;
56+
}
57+
58+
/// <summary>
59+
/// Restores the RxApp scheduler state to what it was when this scope was created.
60+
/// </summary>
61+
public void Dispose()
62+
{
63+
if (_disposed)
64+
{
65+
return;
66+
}
67+
68+
RxApp.MainThreadScheduler = _mainThreadScheduler;
69+
RxApp.TaskpoolScheduler = _taskpoolScheduler;
70+
_disposed = true;
71+
}
72+
}

0 commit comments

Comments
 (0)