Merge pull request #10686 from som-snytt/tweak/12944-hashset-issubset

Tweak HashSet tests
scala · Feb 12, 2024 · 17c0c74 · 17c0c74
2 parents 2d30e4b + 5adc521
commit 17c0c74
Show file tree

Hide file tree

Showing 2 changed files with 65 additions and 62 deletions.
diff --git a/test/junit/scala/collection/immutable/HashSetTest.scala b/test/junit/scala/collection/immutable/HashSetTest.scala
@@ -305,6 +305,67 @@ class HashSetAllocationTest extends AllocationTest {
   }
 }
 class HashSetTest {
+  @Test def t7326(): Unit = {
+    def testCorrectness(): Unit = {
+      // a key that has many hashCode collisions
+      case class Collision(i: Int) { override def hashCode = i / 5 }
+
+      def subsetTest[T](emptyA:Set[T], emptyB:Set[T], mkKey:Int => T, n:Int): Unit = {
+        val outside = mkKey(n + 1)
+        for(i <- 0 to n) {
+          val a = emptyA ++ (0 until i).map(mkKey)
+          // every set must be a subset of itself
+          assertTrue("A set must be the subset of itself", a.subsetOf(a))
+          for(k <- 0 to i) {
+            // k <= i, so b is definitely a subset
+            val b = emptyB ++ (0 until k).map(mkKey)
+            // c has less elements than a, but contains a value that is not in a
+            // so it is not a subset, but that is not immediately obvious due to size
+            val c = b + outside
+            assertTrue(s"$b must be a subset of $a", b.subsetOf(a))
+            assertTrue(s"$c must not be a subset of $a", !c.subsetOf(a))
+          }
+        }
+      }
+
+      // test the HashSet/HashSet case
+      subsetTest(HashSet.empty[Int], HashSet.empty[Int], identity, 100)
+
+      // test the HashSet/other set case
+      subsetTest(HashSet.empty[Int], ListSet.empty[Int], identity, 100)
+
+      // test the HashSet/HashSet case for Collision keys
+      subsetTest(HashSet.empty[Collision], HashSet.empty[Collision], Collision, 100)
+
+      // test the HashSet/other set case for Collision keys
+      subsetTest(HashSet.empty[Collision], ListSet.empty[Collision], Collision, 100)
+    }
+
+    /**
+      * A main performance benefit of the new subsetOf is that we do not have to call hashCode during subsetOf
+      * since we already have the hash codes in the HashSet1 nodes.
+      */
+    def testNoHashCodeInvocationsDuringSubsetOf() = {
+      var count = 0
+
+      case class HashCodeCounter(i:Int) {
+        override def hashCode = {
+          count += 1
+          i
+        }
+      }
+
+      val a = HashSet.empty ++ (0 until 100).map(HashCodeCounter)
+      val b = HashSet.empty ++ (0 until 50).map(HashCodeCounter)
+      val count0 = count
+      val result = b.subsetOf(a)
+      assertTrue("key.hashCode must not be called during subsetOf of two HashSets", count == count0)
+      result
+    }
+    testCorrectness()
+    testNoHashCodeInvocationsDuringSubsetOf()
+  }
+
   @Test def `t12944 subset case NxD is false`: Unit = {
 
     // Bryan Cranston, John Stuart Mill
@@ -319,4 +380,8 @@ class HashSetTest {
   }
   @Test def `nonempty subset of empty is false`: Unit =
     assertFalse(HashSet("bryan", "cranston", "john", "stuart", "mill").subsetOf(HashSet.empty[String]))
+  @Test def `verify not totally broken`: Unit = {
+    assertTrue(HashSet.empty[String].subsetOf(HashSet("bryan", "cranston", "john", "stuart", "mill")))
+    assertTrue(HashSet("john").subsetOf(HashSet("bryan", "cranston", "john", "stuart", "mill")))
+  }
 }
diff --git a/test/junit/scala/collection/immutable/SetTest.scala b/test/junit/scala/collection/immutable/SetTest.scala
@@ -99,68 +99,6 @@ class SetTest {
     assertEquals(Set(1), s4)
   }
 
-  @Test
-  def t7326(): Unit = {
-    def testCorrectness(): Unit = {
-      // a key that has many hashCode collisions
-      case class Collision(i: Int) { override def hashCode = i / 5 }
-
-      def subsetTest[T](emptyA:Set[T], emptyB:Set[T], mkKey:Int => T, n:Int): Unit = {
-        val outside = mkKey(n + 1)
-        for(i <- 0 to n) {
-          val a = emptyA ++ (0 until i).map(mkKey)
-          // every set must be a subset of itself
-          require(a.subsetOf(a), "A set must be the subset of itself")
-          for(k <- 0 to i) {
-            // k <= i, so b is definitely a subset
-            val b = emptyB ++ (0 until k).map(mkKey)
-            // c has less elements than a, but contains a value that is not in a
-            // so it is not a subset, but that is not immediately obvious due to size
-            val c = b + outside
-            require(b.subsetOf(a), s"$b must be a subset of $a")
-            require(!c.subsetOf(a), s"$c must not be a subset of $a")
-          }
-        }
-      }
-
-      // test the HashSet/HashSet case
-      subsetTest(HashSet.empty[Int], HashSet.empty[Int], identity, 100)
-
-      // test the HashSet/other set case
-      subsetTest(HashSet.empty[Int], ListSet.empty[Int], identity, 100)
-
-      // test the HashSet/HashSet case for Collision keys
-      subsetTest(HashSet.empty[Collision], HashSet.empty[Collision], Collision, 100)
-
-      // test the HashSet/other set case for Collision keys
-      subsetTest(HashSet.empty[Collision], ListSet.empty[Collision], Collision, 100)
-    }
-
-    /**
-      * A main performance benefit of the new subsetOf is that we do not have to call hashCode during subsetOf
-      * since we already have the hash codes in the HashSet1 nodes.
-      */
-    def testNoHashCodeInvocationsDuringSubsetOf() = {
-      var count = 0
-
-      case class HashCodeCounter(i:Int) {
-        override def hashCode = {
-          count += 1
-          i
-        }
-      }
-
-      val a = HashSet.empty ++ (0 until 100).map(HashCodeCounter)
-      val b = HashSet.empty ++ (0 until 50).map(HashCodeCounter)
-      val count0 = count
-      val result = b.subsetOf(a)
-      require(count == count0, "key.hashCode must not be called during subsetOf of two HashSets")
-      result
-    }
-    testCorrectness()
-    testNoHashCodeInvocationsDuringSubsetOf()
-  }
-
   @Test def pr10238(): Unit = {
     assertEquals(BitSet(0), BitSet(0, 128) diff BitSet(128))