From 1fa175d3280d4c97970ed42e159629e89ac783f9 Mon Sep 17 00:00:00 2001 From: Jake Meiergerd Date: Fri, 31 Jul 2026 00:28:58 -0500 Subject: [PATCH] Added an additional test covering the original issue reported in #1136, and removed a now-redundant test. This test was actually developed as part of an entirely different fix for #1136. We opted to take the fix implemented in #1141, but still wanted to keep the test here. Fixed incorrect usage of `ObservableCacheEx.Switch()` instead of `Observable.Switch()`, within ObservableCache.Connect()`, which was causing completion notifications to be dropped, and hurting performance. --- .../SuspendNotificationsFixture.UnitTests.cs | 52 +++++++++++++------ .../Cache/SuspendNotificationsFixture.cs | 1 - 2 files changed, 36 insertions(+), 17 deletions(-) diff --git a/src/DynamicData.Tests/Cache/SuspendNotificationsFixture.UnitTests.cs b/src/DynamicData.Tests/Cache/SuspendNotificationsFixture.UnitTests.cs index 9b3dec0e5..be0bb6145 100644 --- a/src/DynamicData.Tests/Cache/SuspendNotificationsFixture.UnitTests.cs +++ b/src/DynamicData.Tests/Cache/SuspendNotificationsFixture.UnitTests.cs @@ -9,6 +9,7 @@ using Xunit; using DynamicData.Kernel; +using DynamicData.Tests.Utilities; namespace DynamicData.Tests.Cache; @@ -46,6 +47,41 @@ public void NotificationsCanBeSuspended() _results.IsCompleted.Should().BeFalse("IsCompleted should not have fired"); } + // https://github.com/reactivemarbles/DynamicData/issues/1136 + [Fact] + public void SuspendedConnectCompletesCorrectly() + { + using var source = new SourceCache(static item => item); + + using var suspension = source.SuspendNotifications(); + + source.AddOrUpdate(1); + + using var subscription = source.Connect() + .RecordCacheItems(out var results); + + results.Error.Should().BeNull("no errors should have occurred"); + results.RecordedChangeSets.Should().BeEmpty("notifications should have been suspended"); + + suspension.Dispose(); + + results.Error.Should().BeNull("no errors should have occurred"); + results.RecordedChangeSets.Should().ContainSingle("notifications should have been resumed"); + results.RecordedItemsByKey.Should().BeEquivalentTo(source.KeyValues, "all changes should have propagated to the new subscriber"); + + source.AddOrUpdate(2); + + results.Error.Should().BeNull("no errors should have occurred"); + results.RecordedChangeSets.Skip(1).Count().Should().Be(1, "a single additional source operation was performed"); + results.RecordedItemsByKey.Should().BeEquivalentTo(source.KeyValues, "all changes should have propagated to the new subscriber"); + + source.Dispose(); + + results.Error.Should().BeNull("no errors should have occurred"); + results.RecordedChangeSets.Skip(2).Should().BeEmpty("no additional source operations were performed"); + results.HasCompleted.Should().BeTrue("the source has been disposed"); + } + [Fact] public void SuspendingNotificationsDoesNotImpactPreview() { @@ -512,22 +548,6 @@ public void OnErrorFiresIfCacheFailsAfterResumingWhileConnectionWasSuspended() results.Data.Count.Should().Be(1, "the data written before the failure should have arrived"); } - [Fact] - public void OnCompletedFiresIfCacheDisposedAfterResumingWhileWatchWasSuspended() - { - // The tests above cover failure. Completion has to reach an activated watch too, and - // this covers the path through the watch itself rather than through the suspension gate. - var suspend = _source.SuspendNotifications(); - var isCompleted = false; - using var subscription = _source.Watch(1).Subscribe(static _ => { }, () => isCompleted = true); - _source.AddOrUpdate(1); - - suspend.Dispose(); - _source.Dispose(); - - isCompleted.Should().BeTrue("a watch deferred by a suspension should still complete when the source does"); - } - public void Dispose() { _source.Dispose(); diff --git a/src/DynamicData.Tests/Cache/SuspendNotificationsFixture.cs b/src/DynamicData.Tests/Cache/SuspendNotificationsFixture.cs index e490eb968..816b3a12c 100644 --- a/src/DynamicData.Tests/Cache/SuspendNotificationsFixture.cs +++ b/src/DynamicData.Tests/Cache/SuspendNotificationsFixture.cs @@ -1,4 +1,3 @@ namespace DynamicData.Tests.Cache; public static partial class SuspendNotificationsFixture; -