diff options
| author | Jim Lawson | 2019-01-08 07:26:20 -0800 |
|---|---|---|
| committer | Adam Izraelevitz | 2019-01-08 09:26:20 -0600 |
| commit | 5f0e893c9213464507418a532ee61347a5da26c8 (patch) | |
| tree | 5bb26fcb4f18387573524d817524fbefb844b1d7 /src | |
| parent | 37567cc65d531b5d47d13c12e9c7ac80fc4d7b1f (diff) | |
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.
Diffstat (limited to 'src')
| -rw-r--r-- | src/main/scala/firrtl/util/ClassUtils.scala | 19 | ||||
| -rw-r--r-- | src/main/scala/firrtl/util/TestOptions.scala | 13 | ||||
| -rw-r--r-- | src/test/scala/firrtlTests/UniquifySpec.scala | 13 |
3 files changed, 43 insertions, 2 deletions
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) } } |
