From 5f0e893c9213464507418a532ee61347a5da26c8 Mon Sep 17 00:00:00 2001
From: Jim Lawson
Date: Tue, 8 Jan 2019 07:26:20 -0800
Subject: Avoid enforcing time constrains during coverage tests. (#989)

This fixes issue #988
I tried one alternative to this fix: record the time to do a *no rename* run (`depth = 0`) and check that the time to do the *deep rename* (`depth = 500`) was a reasonable multiple of the *no rename* test. Unfortunately, the discrepancies were all over the map, sometime as much three orders of magnitude difference.
I decided the current fix was the simplest - don't enforce timing checks if we're doing coverage testing, although determining the latter is brittle.---
 src/main/scala/firrtl/util/ClassUtils.scala   | 19 +++++++++++++++++++
 src/main/scala/firrtl/util/TestOptions.scala  | 13 +++++++++++++
 src/test/scala/firrtlTests/UniquifySpec.scala | 13 +++++++++++--
 3 files changed, 43 insertions(+), 2 deletions(-)
 create mode 100644 src/main/scala/firrtl/util/ClassUtils.scala
 create mode 100644 src/main/scala/firrtl/util/TestOptions.scala

(limited to 'src')

diff --git a/src/main/scala/firrtl/util/ClassUtils.scala b/src/main/scala/firrtl/util/ClassUtils.scala
new file mode 100644
index 00000000..1b388035
--- /dev/null
+++ b/src/main/scala/firrtl/util/ClassUtils.scala
@@ -0,0 +1,19 @@
+package firrtl.util
+
+object ClassUtils {
+  /** Determine if a named class is loaded.
+    *
+    * @param name - name of the class: "foo.bar" or "org.foo.bar"
+    * @return true if the class has been loaded (is accessible), false otherwise.
+    */
+  def isClassLoaded(name: String): Boolean = {
+    val found = try {
+      Class.forName(name, false, getClass.getClassLoader) != null
+    } catch {
+      case e: ClassNotFoundException => false
+      case x: Throwable => throw x
+    }
+//    println(s"isClassLoaded: %s $name".format(if (found) "found" else "didn't find"))
+    found
+  }
+}
diff --git a/src/main/scala/firrtl/util/TestOptions.scala b/src/main/scala/firrtl/util/TestOptions.scala
new file mode 100644
index 00000000..9ee99f8c
--- /dev/null
+++ b/src/main/scala/firrtl/util/TestOptions.scala
@@ -0,0 +1,13 @@
+package firrtl.util
+
+import ClassUtils.isClassLoaded
+
+object TestOptions {
+  // Our timing is inaccurate if we're running tests under coverage.
+  // If any of the classes known to be associated with evaluating coverage are loaded,
+  //  assume we're running tests under coverage.
+  // NOTE: We assume we need only ask the class loader that loaded us.
+  // If it was loaded by another class loader (outside of our hierarchy), it wouldn't be available to us.
+  val coverageClasses = List("scoverage.Platform", "com.intellij.rt.coverage.instrumentation.TouchCounter")
+  val accurateTiming = !coverageClasses.exists(isClassLoaded(_))
+}
diff --git a/src/test/scala/firrtlTests/UniquifySpec.scala b/src/test/scala/firrtlTests/UniquifySpec.scala
index bf0586f3..561f0a84 100644
--- a/src/test/scala/firrtlTests/UniquifySpec.scala
+++ b/src/test/scala/firrtlTests/UniquifySpec.scala
@@ -12,6 +12,7 @@ import firrtl._
 import firrtl.annotations._
 import firrtl.annotations.TargetToken._
 import firrtl.transforms.DontTouchAnnotation
+import firrtl.util.TestOptions
 
 class UniquifySpec extends FirrtlFlatSpec {
 
@@ -285,6 +286,12 @@ class UniquifySpec extends FirrtlFlatSpec {
   }
 
   it should "quickly rename deep bundles" in {
+    // We use a fixed time to determine if this test passed or failed.
+    // This test would pass under normal conditions, but would fail during coverage tests.
+    // Since executions times vary significantly under coverage testing, we check a global
+    //  to see if timing measurements are accurate enough to enforce the timing checks.
+    val maxMs = 8000.0
+
     def mkType(i: Int): String = {
       if(i == 0) "UInt<8>" else s"{x: ${mkType(i - 1)}}"
     }
@@ -299,7 +306,9 @@ class UniquifySpec extends FirrtlFlatSpec {
          |    out <= in
          |""".stripMargin
 
-    val (ms, _) = Utils.time(compileToVerilog(input))
-    (ms < 8000) shouldBe true
+    val (renameMs, _) = Utils.time(compileToVerilog(input))
+
+    if (TestOptions.accurateTiming)
+      renameMs shouldBe < (maxMs)
   }
 }
-- 
cgit v1.2.3