Merge pull request #2325 from AArnott/AddBulkRemovalFromEmptyTests

Add tests for bulk removal from empty collections

Author: stephentoub

src/System.Collections.Immutable/tests/ImmutableListTest.cs

@@ -374,6 +374,25 @@ public void RemoveNonExistentKeepsReference()
             Assert.Same(list, list.Remove(3));
         }
 
+        /// <summary>
+        /// Verifies that RemoveRange does not enumerate its argument if the list is empty
+        /// and therefore could not possibly have any elements to remove anyway.
+        /// </summary>
+        /// <remarks>
+        /// While this would seem an implementation detail and simply an optimization,
+        /// it turns out that changing this behavior now *could* represent a breaking change
+        /// because if the enumerable were to throw an exception, that exception would not be
+        /// observed previously, but would start to be thrown if this behavior changed.
+        /// So this is a test to lock the behavior in place.
+        /// </remarks>
+        /// <seealso cref="ImmutableSetTest.ExceptDoesEnumerateSequenceIfThisIsEmpty"/>
+        [Fact]
+        public void RemoveRangeDoesNotEnumerateSequenceIfThisIsEmpty()
+        {
+            var list = ImmutableList<int>.Empty;
+            list.RemoveRange(Enumerable.Range(1, 1).Select(n => { Assert.False(true, "Sequence should not have been enumerated."); return n; }));
+        }
+
         [Fact]
         public void RemoveAtTest()
         {

src/System.Collections.Immutable/tests/ImmutableSetTest.cs

@@ -58,6 +58,25 @@ public void ExceptTest()
             this.ExceptTestHelper(Empty<int>().Add(1).Add(3).Add(5).Add(7), 3, 7);
         }
 
+        /// <summary>
+        /// Verifies that Except *does* enumerate its argument if the collection is empty.
+        /// </summary>
+        /// <remarks>
+        /// While this would seem an implementation detail and simply lack of an optimization,
+        /// it turns out that changing this behavior now *could* represent a breaking change
+        /// because if the enumerable were to throw an exception, that exception would be
+        /// observed previously, but would no longer be thrown if this behavior changed.
+        /// So this is a test to lock the behavior in place or be thoughtful if adding the optimization.
+        /// </remarks>
+        /// <seealso cref="ImmutableListTest.RemoveRangeDoesNotEnumerateSequenceIfThisIsEmpty"/>
+        [Fact]
+        public void ExceptDoesEnumerateSequenceIfThisIsEmpty()
+        {
+            bool enumerated = false;
+            Empty<int>().Except(Enumerable.Range(1, 1).Select(n => { enumerated = true; return n; }));
+            Assert.True(enumerated);
+        }
+
         [Fact]
         public void SymmetricExceptTest()
         {