Add unit tests for BindableCollection modifications

Stylet/BindableCollection.cs

@@ -187,7 +187,7 @@ namespace Stylet
         {
             Execute.OnUIThreadSync(() =>
             {
-                this.OnCollectionChanging(new NotifyCollectionChangedEventArgs(NotifyCollectionChangedAction.Replace, this[index], item, index));
+                this.OnCollectionChanging(new NotifyCollectionChangedEventArgs(NotifyCollectionChangedAction.Replace, item, this[index], index));
                 base.SetItem(index, item);
             });
         }

StyletUnitTests/BindableCollectionTests.cs

@@ -92,12 +92,16 @@ namespace StyletUnitTests
         }
 
         [Test]
-        public void AddRangeFiresPropertyChanged()
+        public void AddRangeFiresPropertyChangedAfterAddingItems()
         {
             var collection = new BindableCollection<Element>(new[] { new Element(), new Element() });
 
             var changedProperties = new List<string>();
-            ((INotifyPropertyChanged)collection).PropertyChanged += (o, e) => changedProperties.Add(e.PropertyName);
+            ((INotifyPropertyChanged)collection).PropertyChanged += (o, e) =>
+            {
+                Assert.AreEqual(4, collection.Count);
+                changedProperties.Add(e.PropertyName);
+            };
  
             collection.AddRange(new[] { new Element(), new Element() });
 
@@ -105,12 +109,35 @@ namespace StyletUnitTests
         }
 
         [Test]
-        public void AddRangeFiresCollectionChanged()
+        public void AddRangeFiresCollectionChangingBeforeAddingItems()
+        {
+            var collection = new BindableCollection<Element>();
+
+            var changedEvents = new List<NotifyCollectionChangedEventArgs>();
+            collection.CollectionChanging += (o, e) =>
+            {
+                changedEvents.Add(e);
+                Assert.AreEqual(0, collection.Count);
+            };
+
+            collection.AddRange(new[] { new Element() });
+
+            Assert.AreEqual(1, changedEvents.Count);
+            var changedEvent = changedEvents[0];
+            Assert.AreEqual(NotifyCollectionChangedAction.Reset, changedEvent.Action);
+        }
+
+        [Test]
+        public void AddRangeFiresCollectionChangedAfterAddingItems()
         {
             var collection = new BindableCollection<Element>(new[] { new Element(), new Element() });
 
             var changedEvents = new List<NotifyCollectionChangedEventArgs>();
-            collection.CollectionChanged += (o, e) => changedEvents.Add(e);
+            collection.CollectionChanged += (o, e) =>
+            {
+                Assert.AreEqual(4, collection.Count);
+                changedEvents.Add(e);
+            };
 
             var elementsToAdd = new[] { new Element(), new Element() };
             collection.AddRange(elementsToAdd);
@@ -121,13 +148,17 @@ namespace StyletUnitTests
         }
 
         [Test]
-        public void RemoveRangeFiresPropertyChanged()
+        public void RemoveRangeFiresPropertyChangedAfterRemovingItems()
         {
             var itemsToRemove = new[] { new Element(), new Element() };
             var collection = new BindableCollection<Element>(new[] { new Element() }.Concat(itemsToRemove));
 
             var changedProperties = new List<string>();
-            ((INotifyPropertyChanged)collection).PropertyChanged += (o, e) => changedProperties.Add(e.PropertyName);
+            ((INotifyPropertyChanged)collection).PropertyChanged += (o, e) =>
+            {
+                Assert.AreEqual(1, collection.Count);
+                changedProperties.Add(e.PropertyName);
+            };
 
             collection.RemoveRange(itemsToRemove);
 
@@ -135,13 +166,36 @@ namespace StyletUnitTests
         }
 
         [Test]
-        public void RemoveRangeFiresCollectionChanged()
+        public void RemoveRangeFiresCollectionChangingBeforeRemovingItems()
+        {
+            var collection = new BindableCollection<Element>() { new Element() };
+
+            var changedEvents = new List<NotifyCollectionChangedEventArgs>();
+            collection.CollectionChanging += (o, e) =>
+            {
+                changedEvents.Add(e);
+                Assert.AreEqual(1, collection.Count);
+            };
+
+            collection.RemoveRange(new[] { new Element() });
+
+            Assert.AreEqual(1, changedEvents.Count);
+            var changedEvent = changedEvents[0];
+            Assert.AreEqual(NotifyCollectionChangedAction.Reset, changedEvent.Action);
+        }
+
+        [Test]
+        public void RemoveRangeFiresCollectionChangedAfterRemovingItems()
         {
             var itemsToRemove = new[] { new Element(), new Element() };
             var collection = new BindableCollection<Element>(new[] { new Element() }.Concat(itemsToRemove));
 
             var changedEvents = new List<NotifyCollectionChangedEventArgs>();
-            collection.CollectionChanged += (o, e) => changedEvents.Add(e);
+            collection.CollectionChanged += (o, e) =>
+            {
+                Assert.AreEqual(1, collection.Count);
+                changedEvents.Add(e);
+            };
 
             collection.RemoveRange(itemsToRemove);
 
@@ -163,6 +217,21 @@ namespace StyletUnitTests
             Assert.That(changedProperties, Is.EquivalentTo(new[] { "Count", "Item[]" }));
         }
 
+        [Test]
+        public void RefreshFiresCollectionChanging()
+        {
+            var collection = new BindableCollection<Element>(new[] { new Element() });
+
+            var changedEvents = new List<NotifyCollectionChangedEventArgs>();
+            collection.CollectionChanging += (o, e) => changedEvents.Add(e);
+
+            collection.Refresh();
+
+            Assert.AreEqual(1, changedEvents.Count);
+            var changedEvent = changedEvents[0];
+            Assert.AreEqual(NotifyCollectionChangedAction.Reset, changedEvent.Action);
+        }
+
         [Test]
         public void RefreshFiresCollectionChanged()
         {
@@ -185,6 +254,8 @@ namespace StyletUnitTests
 
             bool propertyChangedRaised = false;
             ((INotifyPropertyChanged)collection).PropertyChanged += (o, e) => propertyChangedRaised = true;
+            bool collectionChangingRaised = false;
+            collection.CollectionChanging += (o, e) => collectionChangingRaised = true;
             bool collectionChangedRaised = false;
             collection.CollectionChanged += (o, e) => collectionChangedRaised = true;
 
@@ -194,15 +265,36 @@ namespace StyletUnitTests
             collection.Refresh();
 
             Assert.False(propertyChangedRaised);
+            Assert.False(collectionChangingRaised);
             Assert.False(collectionChangedRaised);
             Assert.NotNull(dispatcher.SendAction);
 
             dispatcher.SendAction();
 
             Assert.True(propertyChangedRaised);
+            Assert.True(collectionChangingRaised);
             Assert.True(collectionChangedRaised);
         }
 
+        [Test]
+        public void InsertItemRaisesCollectionChangingBeforeItemInserted()
+        {
+            var element = new Element();
+            var collection = new BindableCollection<Element>();
+
+            // We assert elsewhere that this is raised
+            collection.CollectionChanging += (o, e) =>
+            {
+                Assert.AreEqual(0, collection.Count);
+
+                Assert.AreEqual(NotifyCollectionChangedAction.Add, e.Action);
+                Assert.That(e.NewItems, Is.EquivalentTo(new[] { element }));
+                Assert.AreEqual(0, e.NewStartingIndex);
+            };
+
+            collection.Add(element);
+        }
+
         [Test]
         public void InsertItemUsesDispatcherToInsertItem()
         {
@@ -228,6 +320,8 @@ namespace StyletUnitTests
 
             bool propertyChangedRaised = false;
             ((INotifyPropertyChanged)collection).PropertyChanged += (o, e) => propertyChangedRaised = true;
+            bool collectionChangingRaised = false;
+            collection.CollectionChanging += (o, e) => collectionChangingRaised = true;
             bool collectionChangedRaised = false;
             collection.CollectionChanged += (o, e) => collectionChangedRaised = true;
 
@@ -237,15 +331,39 @@ namespace StyletUnitTests
             collection.Add(new Element());
 
             Assert.False(propertyChangedRaised);
+            Assert.False(collectionChangingRaised);
             Assert.False(collectionChangedRaised);
             Assert.NotNull(dispatcher.SendAction);
 
             dispatcher.SendAction();
 
             Assert.True(propertyChangedRaised);
+            Assert.True(collectionChangingRaised);
             Assert.True(collectionChangedRaised);
         }
 
+        [Test]
+        public void SetItemRaisesollectionChangingBeforeItemSet()
+        {
+            var oldElement = new Element();
+            var newElement = new Element();
+            var collection = new BindableCollection<Element>() { oldElement };
+
+            // We assert elsewhere that this is raised
+            collection.CollectionChanging += (o, e) =>
+            {
+                Assert.AreEqual(oldElement, collection[0]);
+
+                Assert.AreEqual(NotifyCollectionChangedAction.Replace, e.Action);
+                Assert.That(e.NewItems, Is.EquivalentTo(new[] { newElement }));
+                Assert.AreEqual(0, e.NewStartingIndex);
+                Assert.That(e.OldItems, Is.EquivalentTo(new[] { oldElement }));
+                Assert.AreEqual(0, e.OldStartingIndex);
+            };
+
+            collection[0] = newElement;
+        }
+
         [Test]
         public void SetItemUsesDispatcherToSetItems()
         {
@@ -273,6 +391,8 @@ namespace StyletUnitTests
 
             bool propertyChangedRaised = false;
             ((INotifyPropertyChanged)collection).PropertyChanged += (o, e) => propertyChangedRaised = true;
+            bool collectionChangingRaised = false;
+            collection.CollectionChanging += (o, e) => collectionChangingRaised = true;
             bool collectionChangedRaised = false;
             collection.CollectionChanged += (o, e) => collectionChangedRaised = true;
 
@@ -282,15 +402,36 @@ namespace StyletUnitTests
             collection[0] = new Element();
 
             Assert.False(propertyChangedRaised);
+            Assert.False(collectionChangingRaised);
             Assert.False(collectionChangedRaised);
             Assert.NotNull(dispatcher.SendAction);
 
             dispatcher.SendAction();
 
             Assert.True(propertyChangedRaised);
+            Assert.True(collectionChangingRaised);
             Assert.True(collectionChangedRaised);
         }
 
+        [Test]
+        public void RemoveItemRaisesCollectionChangingBeforeRemovingItem()
+        {
+            var element = new Element();
+            var collection = new BindableCollection<Element>() { element };
+
+            // We assert elsewhere that this is raised
+            collection.CollectionChanging += (o, e) =>
+            {
+                Assert.AreEqual(1, collection.Count);
+
+                Assert.AreEqual(NotifyCollectionChangedAction.Remove, e.Action);
+                Assert.That(e.OldItems, Is.EquivalentTo(new[] { element }));
+                Assert.AreEqual(0, e.OldStartingIndex);
+            };
+
+            collection.Remove(element);
+        }
+
         [Test]
         public void RemoveItemUsesDispatcherToRemoveItems()
         {
@@ -316,6 +457,8 @@ namespace StyletUnitTests
 
             bool propertyChangedRaised = false;
             ((INotifyPropertyChanged)collection).PropertyChanged += (o, e) => propertyChangedRaised = true;
+            bool collectionChangingRaised = false;
+            collection.CollectionChanging += (o, e) => collectionChangingRaised = true;
             bool collectionChangedRaised = false;
             collection.CollectionChanged += (o, e) => collectionChangedRaised = true;
 
@@ -325,15 +468,33 @@ namespace StyletUnitTests
             collection.RemoveAt(0);
 
             Assert.False(propertyChangedRaised);
+            Assert.False(collectionChangingRaised);
             Assert.False(collectionChangedRaised);
             Assert.NotNull(dispatcher.SendAction);
 
             dispatcher.SendAction();
 
             Assert.True(propertyChangedRaised);
+            Assert.True(collectionChangingRaised);
             Assert.True(collectionChangedRaised);
         }
 
+        [Test]
+        public void ClearItemsRaisesCollectionChangingBeforeClearingItems()
+        {
+            var collection = new BindableCollection<Element>() { new Element() };
+
+            // We assert elsewhere that this event is raised
+            collection.CollectionChanging += (o, e) =>
+            {
+                Assert.AreEqual(1, collection.Count);
+
+                Assert.AreEqual(NotifyCollectionChangedAction.Reset, e.Action);
+            };
+
+            collection.Clear();
+        }
+
         [Test]
         public void ClearItemsUsesDispatcherToClearItems()
         {
@@ -359,6 +520,8 @@ namespace StyletUnitTests
 
             bool propertyChangedRaised = false;
             ((INotifyPropertyChanged)collection).PropertyChanged += (o, e) => propertyChangedRaised = true;
+            bool collectionChangingRaised = false;
+            collection.CollectionChanging += (o, e) => collectionChangingRaised = true;
             bool collectionChangedRaised = false;
             collection.CollectionChanged += (o, e) => collectionChangedRaised = true;
 
@@ -368,12 +531,14 @@ namespace StyletUnitTests
             collection.Clear();
 
             Assert.False(propertyChangedRaised);
+            Assert.False(collectionChangingRaised);
             Assert.False(collectionChangedRaised);
             Assert.NotNull(dispatcher.SendAction);
 
             dispatcher.SendAction();
 
             Assert.True(propertyChangedRaised);
+            Assert.True(collectionChangingRaised);
             Assert.True(collectionChangedRaised);
         }
     }