From 081780006503d1f0e0dd6e65fe9e410688071b47 Mon Sep 17 00:00:00 2001 From: Josh Suereth Date: Mon, 11 Aug 2014 14:54:33 -0400 Subject: [PATCH 1/2] Fixes flaky no-such-element exception from bad generation of random tests. --- util/collection/src/test/scala/SettingsTest.scala | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/util/collection/src/test/scala/SettingsTest.scala b/util/collection/src/test/scala/SettingsTest.scala index fb5511c99..2f3d9d4ae 100644 --- a/util/collection/src/test/scala/SettingsTest.scala +++ b/util/collection/src/test/scala/SettingsTest.scala @@ -41,10 +41,11 @@ object SettingsTest extends Properties("settings") { final def derivedSettings(nr: Int): Prop = { val genScopedKeys = { - val attrKeys = mkAttrKeys[Int](nr) + val attrKeys = mkAttrKeys[Int](nr).filter(!_.isEmpty) attrKeys map (_ map (ak => ScopedKey(Scope(0), ak))) - } + }.label("scopedKeys") forAll(genScopedKeys) { scopedKeys => + // Note; It's evil to grab last IF you haven't verified the set can't be empty. val last = scopedKeys.last val derivedSettings: Seq[Setting[Int]] = ( for { @@ -65,10 +66,10 @@ object SettingsTest extends Properties("settings") { val nonEmptyAlphaStr = nonEmptyListOf(alphaChar).map(_.mkString).suchThat(_.forall(_.isLetter)) - for { + (for { list <- Gen.listOfN(nr, nonEmptyAlphaStr) suchThat (l => l.size == l.distinct.size) item <- list - } yield AttributeKey[T](item) + } yield AttributeKey[T](item)).label(s"mkAttrKeys($nr)") } property("Derived setting(s) replace DerivedSetting in the Seq[Setting[_]]") = derivedKeepsPosition From a151ac9c948ccd0fbb6a5618ee1e52a1ead27822 Mon Sep 17 00:00:00 2001 From: Josh Suereth Date: Mon, 11 Aug 2014 16:13:26 -0400 Subject: [PATCH 2/2] Ok, this is actually the flaky issue with the test. We use the ch key for testing, so it can't be part of the autogenerated set. --- .../src/test/scala/SettingsTest.scala | 41 ++++++++++++------- 1 file changed, 27 insertions(+), 14 deletions(-) diff --git a/util/collection/src/test/scala/SettingsTest.scala b/util/collection/src/test/scala/SettingsTest.scala index 2f3d9d4ae..dbad035c6 100644 --- a/util/collection/src/test/scala/SettingsTest.scala +++ b/util/collection/src/test/scala/SettingsTest.scala @@ -37,26 +37,39 @@ object SettingsTest extends Properties("settings") { evaluate(setting(chk, iterate(top)) :: Nil); true } - property("Derived setting chain depending on (prev derived, normal setting)") = forAllNoShrink(Gen.choose(1, 100)) { derivedSettings } + property("Derived setting chain depending on (prev derived, normal setting)") = forAllNoShrink(Gen.choose(1, 100).label("numSettings")) { derivedSettings } final def derivedSettings(nr: Int): Prop = { val genScopedKeys = { - val attrKeys = mkAttrKeys[Int](nr).filter(!_.isEmpty) + // We wan + // t to generate lists of keys that DO NOT inclue the "ch" key we use to check thigns. + val attrKeys = mkAttrKeys[Int](nr).filter(_.forall(_.label != "ch")) attrKeys map (_ map (ak => ScopedKey(Scope(0), ak))) - }.label("scopedKeys") + }.label("scopedKeys").filter(!_.isEmpty) forAll(genScopedKeys) { scopedKeys => - // Note; It's evil to grab last IF you haven't verified the set can't be empty. - val last = scopedKeys.last - val derivedSettings: Seq[Setting[Int]] = ( - for { - List(scoped0, scoped1) <- chk :: scopedKeys sliding 2 - nextInit = if (scoped0 == chk) chk - else (scoped0 zipWith chk) { (p, _) => p + 1 } - } yield derive(setting(scoped1, nextInit)) - ).toSeq + try { + // Note; It's evil to grab last IF you haven't verified the set can't be empty. + val last = scopedKeys.last + val derivedSettings: Seq[Setting[Int]] = ( + for { + List(scoped0, scoped1) <- chk :: scopedKeys sliding 2 + nextInit = if (scoped0 == chk) chk + else (scoped0 zipWith chk) { (p, _) => p + 1 } + } yield derive(setting(scoped1, nextInit)) + ).toSeq - { checkKey(last, Some(nr - 1), evaluate(setting(chk, value(0)) +: derivedSettings)) :| "Not derived?" } && - { checkKey(last, None, evaluate(derivedSettings)) :| "Should not be derived" } + { + // Note: This causes a cycle refernec error, quite frequently. + checkKey(last, Some(nr - 1), evaluate(setting(chk, value(0)) +: derivedSettings)) :| "Not derived?" + } && { + checkKey(last, None, evaluate(derivedSettings)) :| "Should not be derived" + } + } catch { + case t: Throwable => + // TODO - For debugging only. + t.printStackTrace(System.err) + throw t + } } }