Skip to content

Commit 22d5bb5

Browse files
authored
Fix intermittient test failures that use Locator, introduced a scope (#4251)
This pull request introduces a new `LocatorScope` helper to safely isolate and restore the global service locator state (`Locator.Current`) during tests, and applies it to several test fixtures that modify the locator. This change ensures better test reliability and prevents state leakage between tests. Additionally, the documentation is updated to reflect the new approach. **Test isolation improvements:** * Added a new `LocatorScope` class in `ReactiveUI.Tests.Infrastructure.StaticState` to snapshot and restore `Locator.Current` during tests, preventing global state leakage and making tests more reliable. * Updated test fixtures (`CommandBindingTests`, `MessageBusTest`, `RoutedViewHostTests`, `ViewModelViewHostTests`) to use `LocatorScope` in their setup and teardown methods, ensuring the service locator is properly isolated for each test run. [[1]](diffhunk://#diff-cb950c3f874cb7e5c5a49fd2bedc2fa3e51511aa6db8462f270eef23f11e0028R13-R48) [[2]](diffhunk://#diff-f7f03456bd65136087c07d8367d29b062028508f41480f8a69365c4c88bf9f0dR20-R33) [[3]](diffhunk://#diff-d9dbb1c8d2fee1ef13344fb77470509a36bcb26ae3e9d613b5af99375386f9d0R27-R40) [[4]](diffhunk://#diff-5580daabcb200c9598e649dfc4cf40d21a8c3600f93763afda29711563b0387cR26-R39) **Test metadata and documentation:** * Marked `WhenAnyObservableTests` as `[NotInParallel]` and added XML documentation explaining the need for test isolation due to reliance on the service locator. * Updated using statements to import the new `LocatorScope` where required. [[1]](diffhunk://#diff-d9dbb1c8d2fee1ef13344fb77470509a36bcb26ae3e9d613b5af99375386f9d0R8) [[2]](diffhunk://#diff-5580daabcb200c9598e649dfc4cf40d21a8c3600f93763afda29711563b0387cR8) **Documentation updates:** * Removed the outdated `StaticState/README.md` and replaced its guidance with the new `LocatorScope` approach for managing static state in tests.
1 parent 69b64a1 commit 22d5bb5

File tree

8 files changed

+135
-145
lines changed

8 files changed

+135
-145
lines changed

src/tests/ReactiveUI.NonParallel.Tests/CommandBinding/CommandBindingTests.cs

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,21 +10,42 @@
1010
using System.Threading.Tasks;
1111
using System.Windows.Input;
1212
using ReactiveUI;
13+
using ReactiveUI.Tests.Infrastructure.StaticState;
1314
using Splat;
1415

1516
namespace ReactiveUI.NonParallel.Tests
1617
{
18+
/// <summary>
19+
/// Tests for command binding.
20+
/// </summary>
21+
/// <remarks>
22+
/// This test fixture is marked as NotInParallel because tests call
23+
/// Locator.CurrentMutable to register ICreatesCommandBinding implementations,
24+
/// which mutate global service locator state.
25+
/// </remarks>
26+
[NotInParallel]
1727
public class CommandBindingTests
1828
{
19-
public CommandBindingTests()
29+
private LocatorScope? _locatorScope;
30+
31+
[Before(Test)]
32+
public void SetUp()
2033
{
34+
_locatorScope = new LocatorScope();
35+
2136
Locator.CurrentMutable.InitializeSplat();
2237
Locator.CurrentMutable.RegisterConstant(new CreatesCommandBindingViaEvent(), typeof(ICreatesCommandBinding));
2338

2439
// Register a custom binder to test binder resolution
2540
Locator.CurrentMutable.RegisterConstant(new FakeCustomBinder(), typeof(ICreatesCommandBinding));
2641
}
2742

43+
[After(Test)]
44+
public void TearDown()
45+
{
46+
_locatorScope?.Dispose();
47+
}
48+
2849
[Test]
2950
public async Task CommandBinderImplementation_Should_Bind_Command_To_Event()
3051
{

src/tests/ReactiveUI.NonParallel.Tests/MessageBusTest.cs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,17 +17,20 @@ namespace ReactiveUI.Tests.Core;
1717
public class MessageBusTest : IDisposable
1818
{
1919
private MessageBusScope? _messageBusScope;
20+
private LocatorScope? _locatorScope;
2021

2122
[Before(Test)]
2223
public void SetUp()
2324
{
25+
_locatorScope = new LocatorScope();
2426
_messageBusScope = new MessageBusScope();
2527
}
2628

2729
[After(Test)]
2830
public void TearDown()
2931
{
3032
_messageBusScope?.Dispose();
33+
_locatorScope?.Dispose();
3134
}
3235

3336
/// <summary>

src/tests/ReactiveUI.NonParallel.Tests/Platforms/windows-xaml/RoutedViewHostTests.cs

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55

66
using System.Windows;
77
using DynamicData;
8+
using ReactiveUI.Tests.Infrastructure.StaticState;
89
using ReactiveUI.Tests.Wpf;
910

1011
using TUnit.Core.Executors;
@@ -23,6 +24,20 @@ namespace ReactiveUI.Tests;
2324
[NotInParallel]
2425
public class RoutedViewHostTests
2526
{
27+
private LocatorScope? _locatorScope;
28+
29+
[Before(Test)]
30+
public void SetUp()
31+
{
32+
_locatorScope = new LocatorScope();
33+
}
34+
35+
[After(Test)]
36+
public void TearDown()
37+
{
38+
_locatorScope?.Dispose();
39+
}
40+
2641
[Test]
2742
[TestExecutor<STAThreadExecutor>]
2843
public async Task RoutedViewHostDefaultContentNotNull()

src/tests/ReactiveUI.NonParallel.Tests/Platforms/windows-xaml/ViewModelViewHostTests.cs

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55

66
using System.Windows;
77
using DynamicData;
8+
using ReactiveUI.Tests.Infrastructure.StaticState;
89
using ReactiveUI.Tests.Wpf;
910

1011
using TUnit.Core.Executors;
@@ -22,6 +23,20 @@ namespace ReactiveUI.Tests;
2223
[NotInParallel]
2324
public class ViewModelViewHostTests
2425
{
26+
private LocatorScope? _locatorScope;
27+
28+
[Before(Test)]
29+
public void SetUp()
30+
{
31+
_locatorScope = new LocatorScope();
32+
}
33+
34+
[After(Test)]
35+
public void TearDown()
36+
{
37+
_locatorScope?.Dispose();
38+
}
39+
2540
[Test]
2641
[TestExecutor<STAThreadExecutor>]
2742
public async Task ViewModelViewHostDefaultContentNotNull()

src/tests/ReactiveUI.Tests/WhenAny/TestWhenAnyObsViewModel.cs renamed to src/tests/ReactiveUI.NonParallel.Tests/WhenAny/TestWhenAnyObsViewModel.cs

File renamed without changes.

src/tests/ReactiveUI.Tests/WhenAny/WhenAnyObservableTests.cs renamed to src/tests/ReactiveUI.NonParallel.Tests/WhenAny/WhenAnyObservableTests.cs

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,14 @@
77

88
namespace ReactiveUI.Tests;
99

10+
/// <summary>
11+
/// Tests for WhenAnyObservable functionality.
12+
/// This test class is marked as NotInParallel because WhenAnyObservable relies on
13+
/// the service locator (Locator.Current) to find ICreatesObservableForProperty implementations.
14+
/// When tests run in parallel, they can interfere with each other's service locator state,
15+
/// causing intermittent failures with "Could not find a ICreatesObservableForProperty" errors.
16+
/// </summary>
17+
[NotInParallel]
1018
public class WhenAnyObservableTests
1119
{
1220
/// <summary>
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+
using Splat;
7+
8+
namespace ReactiveUI.Tests.Infrastructure.StaticState;
9+
10+
/// <summary>
11+
/// A disposable scope that snapshots and restores Splat's Locator.Current static state.
12+
/// Use this in test fixtures that read or modify Locator.CurrentMutable to ensure
13+
/// static state is properly restored after tests complete.
14+
/// </summary>
15+
/// <remarks>
16+
/// This helper is necessary because Splat's Locator maintains a static/global reference
17+
/// that can leak between test executions, causing intermittent failures.
18+
/// Tests using this scope should also be marked with [NotInParallel] to prevent
19+
/// concurrent modifications to the shared state.
20+
/// </remarks>
21+
/// <example>
22+
/// <code>
23+
/// [NotInParallel]
24+
/// public class MyTests
25+
/// {
26+
/// private LocatorScope? _locatorScope;
27+
///
28+
/// [Before(Test)]
29+
/// public void SetUp()
30+
/// {
31+
/// _locatorScope = new LocatorScope();
32+
/// // Now safe to use Locator.CurrentMutable
33+
/// }
34+
///
35+
/// [After(Test)]
36+
/// public void TearDown()
37+
/// {
38+
/// _locatorScope?.Dispose();
39+
/// }
40+
/// }
41+
/// </code>
42+
/// </example>
43+
public sealed class LocatorScope : IDisposable
44+
{
45+
private readonly IReadonlyDependencyResolver _previousLocator;
46+
47+
/// <summary>
48+
/// Initializes a new instance of the <see cref="LocatorScope"/> class.
49+
/// Captures the current Locator state and sets up a fresh locator for testing.
50+
/// </summary>
51+
public LocatorScope()
52+
{
53+
// Save the current locator so we can restore it later
54+
_previousLocator = Locator.Current;
55+
56+
// Replace with a new locator that tests can modify
57+
var newLocator = new ModernDependencyResolver();
58+
newLocator.InitializeSplat();
59+
newLocator.InitializeReactiveUI();
60+
Locator.SetLocator(newLocator);
61+
}
62+
63+
/// <summary>
64+
/// Restores the Locator to its previous state.
65+
/// </summary>
66+
public void Dispose()
67+
{
68+
// Restore the previous locator
69+
// Cast is safe because we saved it from Locator.Current
70+
Locator.SetLocator((IDependencyResolver)_previousLocator);
71+
}
72+
}

src/tests/ReactiveUI.Tests/Infrastructure/StaticState/README.md

Lines changed: 0 additions & 144 deletions
This file was deleted.

0 commit comments

Comments
 (0)