From 54e67889f10b39323fb729808b8cd22b4c832910 Mon Sep 17 00:00:00 2001 From: chick Date: Thu, 1 Aug 2019 09:21:52 -0700 Subject: Followup to PR #1142 Fixes a threading bug in where lazy reading of file caused a problem for multithreaded access to the that was read. Changes all uses of io.Source to use new API getText getLines getTextResource getLinesResouce Make style to only import FileUtils and not its methods So code is more explicit as e.g. FileUtils.getText() --- .../firrtl/stage/phases/tests/DriverCompatibilitySpec.scala | 2 -- src/test/scala/firrtlTests/AnnotationTests.scala | 6 +++--- src/test/scala/firrtlTests/DriverSpec.scala | 8 ++++---- src/test/scala/firrtlTests/FirrtlSpec.scala | 1 - src/test/scala/firrtlTests/MultiThreadingSpec.scala | 9 +++++---- src/test/scala/firrtlTests/ProtoBufSpec.scala | 7 +++++-- src/test/scala/firrtlTests/ReplSeqMemTests.scala | 4 ++-- src/test/scala/firrtlTests/stage/FirrtlMainSpec.scala | 4 ++-- .../scala/firrtlTests/transforms/BlackBoxSourceHelperSpec.scala | 5 ++--- src/test/scala/firrtlTests/transforms/TopWiringTest.scala | 1 - 10 files changed, 23 insertions(+), 24 deletions(-) (limited to 'src/test') diff --git a/src/test/scala/firrtl/stage/phases/tests/DriverCompatibilitySpec.scala b/src/test/scala/firrtl/stage/phases/tests/DriverCompatibilitySpec.scala index 77316feb..d06344af 100644 --- a/src/test/scala/firrtl/stage/phases/tests/DriverCompatibilitySpec.scala +++ b/src/test/scala/firrtl/stage/phases/tests/DriverCompatibilitySpec.scala @@ -4,8 +4,6 @@ package firrtl.stage.phases.tests import org.scalatest.{FlatSpec, Matchers, PrivateMethodTester} -import scala.io.Source - import java.io.File import firrtl._ diff --git a/src/test/scala/firrtlTests/AnnotationTests.scala b/src/test/scala/firrtlTests/AnnotationTests.scala index 6b492148..7b944470 100644 --- a/src/test/scala/firrtlTests/AnnotationTests.scala +++ b/src/test/scala/firrtlTests/AnnotationTests.scala @@ -7,6 +7,7 @@ import java.io.{File, FileWriter, Writer} import firrtl.annotations.AnnotationYamlProtocol._ import firrtl.annotations._ import firrtl._ +import firrtl.FileUtils import firrtl.transforms.OptimizableExtModuleAnnotation import firrtl.passes.InlineAnnotation import firrtl.passes.memlib.PinAnnotation @@ -472,8 +473,7 @@ class LegacyAnnotationTests extends AnnotationTests { Annotation(ModuleName(mod, CircuitName("Top")), classOf[Transform], "some value") "LegacyAnnotations" should "be readable from file" in { - val annotationStream = getClass.getResourceAsStream("/annotations/SampleAnnotations.anno") - val annotationsYaml = scala.io.Source.fromInputStream(annotationStream).getLines().mkString("\n").parseYaml + val annotationsYaml = FileUtils.getTextResource("/annotations/SampleAnnotations.anno").parseYaml val annotationArray = annotationsYaml.convertTo[Array[LegacyAnnotation]] annotationArray.length should be (9) annotationArray(0).targetString should be ("ModC") @@ -538,7 +538,7 @@ class JsonAnnotationTests extends AnnotationTests with BackendCompilationUtiliti writer.write(JsonProtocol.serialize(annos)) writer.close() - val text = io.Source.fromFile(annoFile).getLines().mkString("\n") + val text = FileUtils.getText(annoFile) annoFile.delete() val readAnnos = JsonProtocol.deserializeTry(text).get diff --git a/src/test/scala/firrtlTests/DriverSpec.scala b/src/test/scala/firrtlTests/DriverSpec.scala index 1cb991c8..b4f61dfd 100644 --- a/src/test/scala/firrtlTests/DriverSpec.scala +++ b/src/test/scala/firrtlTests/DriverSpec.scala @@ -9,10 +9,10 @@ import firrtl.passes.{InlineAnnotation, InlineInstances} import firrtl.passes.memlib.{InferReadWrite, InferReadWriteAnnotation, ReplSeqMem, ReplSeqMemAnnotation} import firrtl.transforms.BlackBoxTargetDirAnno import firrtl._ +import firrtl.FileUtils import firrtl.annotations._ import firrtl.util.BackendCompilationUtilities -import scala.io.Source import scala.util.{Failure, Success, Try} class ExceptingTransform extends Transform { @@ -307,7 +307,7 @@ class DriverSpec extends FreeSpec with Matchers with BackendCompilationUtilities copyResourceToFile(s"/annotations/$annoFilename", annotationsTestFile) import net.jcazevedo.moultingyaml._ - val text = io.Source.fromFile(annotationsTestFile).mkString + val text = FileUtils.getText(annotationsTestFile) val yamlAnnos = text.parseYaml match { case YamlArray(xs) => xs } @@ -467,8 +467,8 @@ class DriverSpec extends FreeSpec with Matchers with BackendCompilationUtilities Driver.execute(args) } "Both paths do the same thing" in { - val s1 = Source.fromFile(verilogFromFir).mkString - val s2 = Source.fromFile(verilogFromPb).mkString + val s1 = FileUtils.getText(verilogFromFir) + val s2 = FileUtils.getText(verilogFromPb) s1 should equal (s2) } } diff --git a/src/test/scala/firrtlTests/FirrtlSpec.scala b/src/test/scala/firrtlTests/FirrtlSpec.scala index c110109c..efbb27de 100644 --- a/src/test/scala/firrtlTests/FirrtlSpec.scala +++ b/src/test/scala/firrtlTests/FirrtlSpec.scala @@ -11,7 +11,6 @@ import scala.sys.process._ import org.scalatest._ import org.scalatest.prop._ -import scala.io.Source import firrtl._ import firrtl.ir._ import firrtl.Parser.{IgnoreInfo, UseInfo} diff --git a/src/test/scala/firrtlTests/MultiThreadingSpec.scala b/src/test/scala/firrtlTests/MultiThreadingSpec.scala index 444faebc..72c66e93 100644 --- a/src/test/scala/firrtlTests/MultiThreadingSpec.scala +++ b/src/test/scala/firrtlTests/MultiThreadingSpec.scala @@ -2,10 +2,11 @@ package firrtlTests -import firrtl.{ChirrtlForm, CircuitState, Compiler, annotations} +import firrtl.FileUtils +import firrtl.{ChirrtlForm, CircuitState} -import scala.concurrent.{Future, Await, ExecutionContext} import scala.concurrent.duration.Duration +import scala.concurrent.{Await, ExecutionContext, Future} class MultiThreadingSpec extends FirrtlPropSpec { @@ -27,8 +28,8 @@ class MultiThreadingSpec extends FirrtlPropSpec { val numThreads = 64 // arbitrary // Begin the actual test - val inputStream = getClass().getResourceAsStream(inputFilePath) - val inputStrings = io.Source.fromInputStream(inputStream).getLines().toList + + val inputStrings = FileUtils.getLinesResource(inputFilePath) import ExecutionContext.Implicits.global try { // Use try-catch because error can manifest in many ways diff --git a/src/test/scala/firrtlTests/ProtoBufSpec.scala b/src/test/scala/firrtlTests/ProtoBufSpec.scala index 7a0c3eeb..3d46e291 100644 --- a/src/test/scala/firrtlTests/ProtoBufSpec.scala +++ b/src/test/scala/firrtlTests/ProtoBufSpec.scala @@ -30,8 +30,11 @@ class ProtoBufSpec extends FirrtlFlatSpec { for (FirrtlResourceTest(name, dir) <- firrtlResourceTests) { s"$name" should "work with Protobuf serialization and deserialization" in { - val stream = getClass.getResourceAsStream(s"$dir/$name.fir") - val circuit = parse(scala.io.Source.fromInputStream(stream).getLines.mkString("\n")) +// val stream = getClass.getResourceAsStream(s"$dir/$name.fir") +// val circuit = parse(scala.io.Source.fromInputStream(stream).getLines.mkString("\n")) +// stream.close() + + val circuit = parse(FileUtils.getTextResource(s"$dir/$name.fir")) // Test ToProto and FromProto val protobuf = proto.ToProto.convert(circuit) diff --git a/src/test/scala/firrtlTests/ReplSeqMemTests.scala b/src/test/scala/firrtlTests/ReplSeqMemTests.scala index a1f27958..9a5a2f72 100644 --- a/src/test/scala/firrtlTests/ReplSeqMemTests.scala +++ b/src/test/scala/firrtlTests/ReplSeqMemTests.scala @@ -7,6 +7,7 @@ import firrtl.ir._ import firrtl.passes._ import firrtl.transforms._ import firrtl.passes.memlib._ +import firrtl.FileUtils import annotations._ import FirrtlCheckers._ @@ -29,8 +30,7 @@ class ReplSeqMemSpec extends SimpleTransformSpec { def checkMemConf(filename: String, mems: Set[MemConf]) { // Read the mem conf - val file = scala.io.Source.fromFile(filename) - val text = try file.mkString finally file.close() + val text = FileUtils.getText(filename) // Verify that this does not throw an exception val fromConf = MemConf.fromString(text) // Verify the mems in the conf are the same as the expected ones diff --git a/src/test/scala/firrtlTests/stage/FirrtlMainSpec.scala b/src/test/scala/firrtlTests/stage/FirrtlMainSpec.scala index 0d1120fc..e6129550 100644 --- a/src/test/scala/firrtlTests/stage/FirrtlMainSpec.scala +++ b/src/test/scala/firrtlTests/stage/FirrtlMainSpec.scala @@ -6,7 +6,7 @@ import org.scalatest.{FeatureSpec, GivenWhenThen, Matchers} import java.io.{File, PrintWriter} -import scala.io.Source +import firrtl.FileUtils import firrtl.stage.FirrtlMain import firrtl.util.BackendCompilationUtilities @@ -312,7 +312,7 @@ class FirrtlMainSpec extends FeatureSpec with GivenWhenThen with Matchers with f Then("the implicit annotation file should NOT be read") val annoFileOut = new File(td.dir + "/Top.out.anno.json") - val annotationJson = Source.fromFile(annoFileOut).mkString + val annotationJson = FileUtils.getText(annoFileOut) annotationJson should not include ("InlineInstances") And("no warning should be printed") diff --git a/src/test/scala/firrtlTests/transforms/BlackBoxSourceHelperSpec.scala b/src/test/scala/firrtlTests/transforms/BlackBoxSourceHelperSpec.scala index 089e837a..6e63317b 100644 --- a/src/test/scala/firrtlTests/transforms/BlackBoxSourceHelperSpec.scala +++ b/src/test/scala/firrtlTests/transforms/BlackBoxSourceHelperSpec.scala @@ -6,6 +6,7 @@ import firrtl.annotations.{Annotation, CircuitName, ModuleName} import firrtl.transforms._ import firrtl.{FIRRTLException, Transform, VerilogCompiler, VerilogEmitter} import firrtlTests.{HighTransformSpec, LowTransformSpec} +import firrtl.FileUtils import org.scalacheck.Test.Failed import org.scalatest.{FreeSpec, Matchers, Succeeded} @@ -132,9 +133,7 @@ class BlacklBoxSourceHelperTransformSpec extends LowTransformSpec { // but our file list should not include the verilog header file. val fileListFile = new java.io.File(s"test_run_dir/${BlackBoxSourceHelper.defaultFileListName}") fileListFile.exists should be (true) - val fileListFileSource = io.Source.fromFile(fileListFile) - val fileList = fileListFileSource.getLines.mkString - fileListFileSource.close() + val fileList = FileUtils.getText(fileListFile) fileList.contains("ParameterizedViaHeaderAdderExtModule.v") should be (true) fileList.contains("VerilogHeaderFile.vh") should be (false) } diff --git a/src/test/scala/firrtlTests/transforms/TopWiringTest.scala b/src/test/scala/firrtlTests/transforms/TopWiringTest.scala index 16dffd66..9dd290f8 100644 --- a/src/test/scala/firrtlTests/transforms/TopWiringTest.scala +++ b/src/test/scala/firrtlTests/transforms/TopWiringTest.scala @@ -6,7 +6,6 @@ package transforms import org.scalatest.FlatSpec import org.scalatest.Matchers import org.scalatest.junit.JUnitRunner -import scala.io.Source import java.io._ import firrtl._ -- cgit v1.2.3 From 2bf399c240938ba51069348f986fa5d65135a808 Mon Sep 17 00:00:00 2001 From: chick Date: Thu, 1 Aug 2019 17:04:45 -0700 Subject: Followup to PR #1142 - use scala.io.Source instead of io.Source - .toList cleaner way to force stream to be read. - clear old commented out code in ProtoBufSpec --- src/test/scala/firrtlTests/ProtoBufSpec.scala | 6 ------ 1 file changed, 6 deletions(-) (limited to 'src/test') diff --git a/src/test/scala/firrtlTests/ProtoBufSpec.scala b/src/test/scala/firrtlTests/ProtoBufSpec.scala index 3d46e291..526a194c 100644 --- a/src/test/scala/firrtlTests/ProtoBufSpec.scala +++ b/src/test/scala/firrtlTests/ProtoBufSpec.scala @@ -2,8 +2,6 @@ package firrtlTests -import java.io.{ByteArrayInputStream, ByteArrayOutputStream} - import firrtl.FirrtlProtos.Firrtl import firrtl._ import firrtl.ir._ @@ -30,10 +28,6 @@ class ProtoBufSpec extends FirrtlFlatSpec { for (FirrtlResourceTest(name, dir) <- firrtlResourceTests) { s"$name" should "work with Protobuf serialization and deserialization" in { -// val stream = getClass.getResourceAsStream(s"$dir/$name.fir") -// val circuit = parse(scala.io.Source.fromInputStream(stream).getLines.mkString("\n")) -// stream.close() - val circuit = parse(FileUtils.getTextResource(s"$dir/$name.fir")) // Test ToProto and FromProto -- cgit v1.2.3