diff options
author | Som Snytt <som.snytt@gmail.com> | 2014-11-14 11:16:06 -0800 |
---|---|---|
committer | Som Snytt <som.snytt@gmail.com> | 2014-12-06 21:29:51 -0800 |
commit | 6ba7068b9f0389812bb03eae88c31035ad73c1aa (patch) | |
tree | 6047a08b293e779d9d839b628bb19e2e2a632cc2 /test | |
parent | a1ee57da54eff4fe372e304fb5695941a70211c6 (diff) | |
download | scala-6ba7068b9f0389812bb03eae88c31035ad73c1aa.tar.gz scala-6ba7068b9f0389812bb03eae88c31035ad73c1aa.tar.bz2 scala-6ba7068b9f0389812bb03eae88c31035ad73c1aa.zip |
SI-8976 MutableList.tail.iterator.size is length
The previous behavior was to iterate over the mutated
list of arbitrary length. The previous iteration of
the iterator would also iterate the terminal element
of the list without halting.
This is fixed by capping the length of iterator.
That is OK because mutating the list by adding to it during
iteration is not recommended.
For good measure, the exhausted iterator does not hold
a reference to any remaining tail.
A test is added that will no doubt be superseded by the QCC tests.
(Quasi-Comprehensive Collections.)
The test just checks that the extra tail is not strongly
reachable from the iterator. If the garbage collector happens
to kick in and determine that the object is weakly reachable,
then the check terminates early.
Diffstat (limited to 'test')
4 files changed, 113 insertions, 6 deletions
diff --git a/test/junit/scala/collection/mutable/MutableListTest.scala b/test/junit/scala/collection/mutable/MutableListTest.scala new file mode 100644 index 0000000000..ac6d30def0 --- /dev/null +++ b/test/junit/scala/collection/mutable/MutableListTest.scala @@ -0,0 +1,37 @@ +package scala.collection.mutable + +import org.junit.runner.RunWith +import org.junit.runners.JUnit4 +import org.junit.Test +import org.junit.Assert._ + +import scala.tools.testing.AssertUtil._ + +@RunWith(classOf[JUnit4]) +class MutableListTest { + + // Tests SI-8976 + @Test def tailIteratorMustTerminateAtLength(): Unit = { + val is = MutableList(1,2,3) + val tl = is.tail + assertEquals(tl.length, tl.iterator.length) + is += 5 + assertEquals(tl.length, tl.iterator.length) + assertSameElements(tl, tl.iterator) + } + @Test def iteratorMustFailEventually(): Unit = assertThrows[NoSuchElementException] { + MutableList[Unit]().iterator.next() + } + // was: Root empty iterator held reference + @Test def iteratorMustNotHoldOntoLast(): Unit = { + val is = MutableList(Some(1), Some(2)) + val it = is.iterator + val x = Some(3) + is += x + assertNotReachable(x, it) { + it.next() + it.next() + } + assertTrue(it.isEmpty) + } +} diff --git a/test/junit/scala/tools/testing/AssertThrowsTest.scala b/test/junit/scala/tools/testing/AssertThrowsTest.scala index d91e450bac..76758f51d2 100644 --- a/test/junit/scala/tools/testing/AssertThrowsTest.scala +++ b/test/junit/scala/tools/testing/AssertThrowsTest.scala @@ -38,6 +38,6 @@ class AssertThrowsTest { } catch { case e: AssertionError => return } - assert(false, "assertThrows should error if the tested expression does not throw anything") + fail("assertThrows should error if the tested expression does not throw anything") } } diff --git a/test/junit/scala/tools/testing/AssertUtil.scala b/test/junit/scala/tools/testing/AssertUtil.scala index 83a637783f..d29f9a473f 100644 --- a/test/junit/scala/tools/testing/AssertUtil.scala +++ b/test/junit/scala/tools/testing/AssertUtil.scala @@ -2,18 +2,42 @@ package scala.tools package testing import org.junit.Assert -import Assert.fail +import Assert._ import scala.runtime.ScalaRunTime.stringOf import scala.collection.{ GenIterable, IterableLike } +import scala.collection.JavaConverters._ +import scala.collection.mutable +import java.lang.ref._ +import java.lang.reflect._ +import java.util.IdentityHashMap /** This module contains additional higher-level assert statements * that are ultimately based on junit.Assert primitives. */ object AssertUtil { - /** - * Check if throwable T (or a subclass) was thrown during evaluation of f, and that its message - * satisfies the `checkMessage` predicate. - * If any other exception will be re-thrown. + private final val timeout = 60 * 1000L // wait a minute + + private implicit class `ref helper`[A](val r: Reference[A]) extends AnyVal { + def isEmpty: Boolean = r.get == null + def nonEmpty: Boolean = !isEmpty + } + private implicit class `class helper`(val clazz: Class[_]) extends AnyVal { + def allFields: List[Field] = { + def loop(k: Class[_]): List[Field] = + if (k == null) Nil + else k.getDeclaredFields.toList ::: loop(k.getSuperclass) + loop(clazz) + } + } + private implicit class `field helper`(val f: Field) extends AnyVal { + def follow(o: AnyRef): AnyRef = { + f setAccessible true + f get o + } + } + + /** Check if throwable T (or a subclass) was thrown during evaluation of f, and that its message + * satisfies the `checkMessage` predicate. If any other exception will be re-thrown. */ def assertThrows[T <: Throwable](f: => Any, checkMessage: String => Boolean = s => true) @@ -41,4 +65,29 @@ object AssertUtil { */ def assertSameElements[A, B >: A](expected: IterableLike[A, _], actual: Iterator[B]): Unit = assertSameElements(expected, actual.toList, "") + + /** Value is not strongly reachable from roots after body is evaluated. + */ + def assertNotReachable[A <: AnyRef](a: => A, roots: AnyRef*)(body: => Unit): Unit = { + val wkref = new WeakReference(a) + def refs(root: AnyRef): mutable.Set[AnyRef] = { + val seen = new IdentityHashMap[AnyRef, Unit] + def loop(o: AnyRef): Unit = + if (wkref.nonEmpty && o != null && !seen.containsKey(o)) { + seen.put(o, ()) + for { + f <- o.getClass.allFields + if !Modifier.isStatic(f.getModifiers) + if !f.getType.isPrimitive + if !classOf[Reference[_]].isAssignableFrom(f.getType) + } loop(f follow o) + } + loop(root) + seen.keySet.asScala + } + body + for (r <- roots if wkref.nonEmpty) { + assertFalse(s"Root $r held reference", refs(r) contains wkref.get) + } + } } diff --git a/test/junit/scala/tools/testing/AssertUtilTest.scala b/test/junit/scala/tools/testing/AssertUtilTest.scala new file mode 100644 index 0000000000..03d8815ab2 --- /dev/null +++ b/test/junit/scala/tools/testing/AssertUtilTest.scala @@ -0,0 +1,21 @@ +package scala.tools +package testing + +import org.junit.Assert._ +import org.junit.Test +import org.junit.runner.RunWith +import org.junit.runners.JUnit4 +import AssertUtil._ + +import java.lang.ref._ + +@RunWith(classOf[JUnit4]) +class AssertUtilTest { + + @Test def reachableIgnoresReferences(): Unit = { + class Holder[A](val ref: SoftReference[A]) + val o = new Object + val r = new SoftReference(o) + assertNotReachable(o, new Holder(r)) { } + } +} |