Enhance test coverage for BindingConverterResolver and BindingHookEvaluator with additional unit tests

Author: glennawatson

src/ReactiveUI/Bindings/Property/Internal/BindingConverterResolver.cs

@@ -19,7 +19,7 @@ namespace ReactiveUI;
 [RequiresUnreferencedCode("Uses RxConverters and Splat which may require dynamic type resolution")]
 internal class BindingConverterResolver : IBindingConverterResolver
 {
-    private static readonly ConcurrentDictionary<(Type fromType, Type? toType), ISetMethodBindingConverter?> _setMethodCache = new();
+    private static readonly ConcurrentDictionary<(Type fromType, Type? toType), Func<object?, object?, object?[]?, object?>?> _setMethodCache = new();
 
     /// <inheritdoc/>
     public object? GetBindingConverter(Type fromType, Type toType) =>
@@ -33,18 +33,20 @@ internal class BindingConverterResolver : IBindingConverterResolver
             return null;
         }
 
-        var converter = _setMethodCache.GetOrAdd(
+        return _setMethodCache.GetOrAdd(
             (fromType, toType),
-            static key => ResolveBestSetMethodConverter(key.fromType, key.toType));
-
-        if (converter is null)
-        {
-            return null;
-        }
-
-        // Adapt the converter's contract to the local call shape expected by SetThenGet.
-        // Note: do not store this delegate in the cache; the cache stores the converter instance.
-        return (currentValue, newValue, indexParameters) => converter.PerformSet(currentValue, newValue, indexParameters);
+            static key =>
+            {
+                var converter = ResolveBestSetMethodConverter(key.fromType, key.toType);
+                if (converter is null)
+                {
+                    return null;
+                }
+
+                // Adapt the converter's contract to the local call shape expected by SetThenGet.
+                // Cache the delegate to ensure reference equality for repeated calls.
+                return (currentValue, newValue, indexParameters) => converter.PerformSet(currentValue, newValue, indexParameters);
+            });
     }
 
     /// <summary>

src/ReactiveUI/Bindings/Property/Internal/BindingHookEvaluator.cs

@@ -41,6 +41,11 @@ public bool EvaluateBindingHooks<TViewModel, TView>(
         Func<IObservedChange<object, object?>[]> vmFetcher = vmExpression is not null
             ? (() =>
             {
+                if (viewModel is null)
+                {
+                    return [];
+                }
+
                 vmChainGetter!.TryGetAllValues(viewModel, out var fetchedValues);
                 return fetchedValues;
             })

src/tests/ReactiveUI.Tests/Bindings/Property/Unit/BindingConverterResolverTests.cs

@@ -3,15 +3,19 @@
 // The .NET Foundation licenses this file to you under the MIT license.
 // See the LICENSE file in the project root for full license information.
 
+using ReactiveUI.Builder;
+using ReactiveUI.Tests.Utilities.AppBuilder;
+
 namespace ReactiveUI.Tests;
 
 /// <summary>
 /// Unit tests for <see cref="BindingConverterResolver"/>.
 /// </summary>
 /// <remarks>
-/// These tests verify converter resolution logic without manipulating global Splat state.
-/// Tests use the real implementation and rely on converters registered in the DI container.
+/// These tests verify converter resolution logic.
+/// Tests use the Executor paradigm to manage AppBuilder state and registrations.
 /// </remarks>
+[TestExecutor<BindingConverterResolverTests.Executor>]
 public class BindingConverterResolverTests
 {
     /// <summary>
@@ -24,7 +28,7 @@ public async Task GetBindingConverter_WithRegisteredTypedConverter_ReturnsConver
         // Arrange
         var resolver = new BindingConverterResolver();
 
-        // Act - IntegerToStringTypeConverter is registered by default in RxConverters
+        // Act - IntegerToStringTypeConverter is registered by default via WithCoreServices
         var converter = resolver.GetBindingConverter(typeof(int), typeof(string));
 
         // Assert
@@ -59,10 +63,12 @@ public async Task GetSetMethodConverter_WithCaching_ReturnsSameInstance()
         var resolver = new BindingConverterResolver();
 
         // Act
-        var converter1 = resolver.GetSetMethodConverter(typeof(int), typeof(string));
-        var converter2 = resolver.GetSetMethodConverter(typeof(int), typeof(string));
+        // Use MockType which has a registered set method converter
+        var converter1 = resolver.GetSetMethodConverter(typeof(MockType), typeof(MockType));
+        var converter2 = resolver.GetSetMethodConverter(typeof(MockType), typeof(MockType));
 
-        // Assert - Should return same cached instance (or both null)
+        // Assert
+        await Assert.That(converter1).IsNotNull();
         await Assert.That(converter1).IsSameReferenceAs(converter2);
     }
 
@@ -100,4 +106,143 @@ public async Task GetBindingConverter_UsesRxConverters_WhenAvailable()
         await Assert.That(converter).IsNotNull();
         await Assert.That(converter).IsTypeOf<IntegerToStringTypeConverter>();
     }
-}
+
+    /// <summary>
+    /// Verifies that GetBindingConverter falls back to Splat if not in RxConverters.
+    /// </summary>
+    /// <returns>A <see cref="Task"/> representing the asynchronous unit test.</returns>
+    [Test]
+    public async Task GetBindingConverter_FallsBackToSplat_WhenRxConvertersFails()
+    {
+        // Arrange
+        var resolver = new BindingConverterResolver();
+
+        // Act
+        // MockType converter is registered in Splat (via Executor) but NOT in RxConverters (standard list)
+        var converter = resolver.GetBindingConverter(typeof(MockType), typeof(MockType));
+
+        // Assert
+        await Assert.That(converter).IsNotNull();
+        await Assert.That(converter).IsTypeOf<MockBindingTypeConverter>();
+    }
+
+    /// <summary>
+    /// Verifies that GetSetMethodConverter returns a registered converter.
+    /// </summary>
+    /// <returns>A <see cref="Task"/> representing the asynchronous unit test.</returns>
+    [Test]
+    public async Task GetSetMethodConverter_ReturnsConverter_WhenRegistered()
+    {
+        // Arrange
+        var resolver = new BindingConverterResolver();
+
+        // Act
+        var converterFunc = resolver.GetSetMethodConverter(typeof(MockType), typeof(MockType));
+
+        // Assert
+        await Assert.That(converterFunc).IsNotNull();
+
+        // Invoke to verify
+        var result = converterFunc!(new MockType(), new MockType(), null);
+        await Assert.That(result).IsEqualTo("SetPerformed");
+    }
+
+    /// <summary>
+    /// Verifies that GetSetMethodConverter returns null when no converter is registered.
+    /// </summary>
+    /// <returns>A <see cref="Task"/> representing the asynchronous unit test.</returns>
+    [Test]
+    public async Task GetSetMethodConverter_WithUnregisteredType_ReturnsNull()
+    {
+        // Arrange
+        var resolver = new BindingConverterResolver();
+
+        // Act
+        var converterFunc = resolver.GetSetMethodConverter(typeof(System.Net.IPAddress), typeof(System.Numerics.BigInteger));
+
+        // Assert
+        await Assert.That(converterFunc).IsNull();
+    }
+
+    /// <summary>
+    /// Verifies that GetSetMethodConverter handles null toType gracefully.
+    /// </summary>
+    /// <returns>A <see cref="Task"/> representing the asynchronous unit test.</returns>
+    [Test]
+    public async Task GetSetMethodConverter_WithNullToType_HandlesGracefully()
+    {
+        // Arrange
+        var resolver = new BindingConverterResolver();
+
+        // Act
+        var converterFunc = resolver.GetSetMethodConverter(typeof(MockType), null);
+
+        // Assert
+        // The MockSetMethodBindingConverter returns affinity 0 for null toType, so converter should be null
+        // The test verifies the method handles null toType gracefully without throwing
+        await Assert.That(converterFunc).IsNull();
+    }
+
+    /// <summary>
+    /// Verifies that GetBindingConverter handles null services gracefully.
+    /// </summary>
+    /// <returns>A <see cref="Task"/> representing the asynchronous unit test.</returns>
+    [Test]
+    public async Task GetBindingConverter_WithNoRxConverters_FallsBackToSplat()
+    {
+        // Arrange
+        var resolver = new BindingConverterResolver();
+
+        // Act - Get a converter that should be found in Splat
+        var converter = resolver.GetBindingConverter(typeof(MockType), typeof(MockType));
+
+        // Assert
+        await Assert.That(converter).IsNotNull();
+        await Assert.That(converter).IsTypeOf<MockBindingTypeConverter>();
+    }
+
+    /// <summary>
+    /// Test executor that registers mock converters.
+    /// </summary>
+    public class Executor : BaseAppBuilderTestExecutor
+    {
+        /// <inheritdoc/>
+        protected override void ConfigureAppBuilder(IReactiveUIBuilder builder, TestContext context)
+        {
+            ArgumentNullException.ThrowIfNull(builder);
+            ArgumentNullException.ThrowIfNull(context);
+
+            builder
+                .WithRegistration(r => r.RegisterConstant<IBindingTypeConverter>(new MockBindingTypeConverter()))
+                .WithRegistration(r => r.RegisterConstant<ISetMethodBindingConverter>(new MockSetMethodBindingConverter()))
+                .WithCoreServices();
+        }
+    }
+
+    private class MockType
+    {
+    }
+
+    private class MockBindingTypeConverter : IBindingTypeConverter
+    {
+        public Type FromType => typeof(MockType);
+
+        public Type ToType => typeof(MockType);
+
+        public int GetAffinityForObjects() => 100;
+
+        public bool TryConvertTyped(object? from, object? conversionHint, out object? result)
+        {
+            result = null;
+            return false;
+        }
+    }
+
+    private class MockSetMethodBindingConverter : ISetMethodBindingConverter
+    {
+        public int GetAffinityForObjects(Type? fromType, Type? toType) =>
+            fromType == typeof(MockType) && toType == typeof(MockType) ? 100 : 0;
+
+        public object? PerformSet(object? current, object? newValue, object?[]? arguments) => "SetPerformed";
+    }
+}