From d1611f81badf54aaf460ba37f01dc98c4005d82a Mon Sep 17 00:00:00 2001
From: chick
Date: Tue, 9 Feb 2016 11:24:08 -0800
Subject: Added support for finish method of BasicTester to be overridden in a
 subclass which allows tester to have clean up and other construction code
 executed after a user code executed during constructor of that subclass

---
 src/main/scala/Chisel/testers/BasicTester.scala   |  2 +
 src/main/scala/Chisel/testers/TesterDriver.scala  | 15 ++++++-
 src/test/scala/chiselTests/DeqIOSpec.scala        | 54 +++++++----------------
 src/test/scala/chiselTests/TesterDriverSpec.scala | 10 +++++
 4 files changed, 41 insertions(+), 40 deletions(-)
 create mode 100644 src/test/scala/chiselTests/TesterDriverSpec.scala

diff --git a/src/main/scala/Chisel/testers/BasicTester.scala b/src/main/scala/Chisel/testers/BasicTester.scala
index 6807a30e..98033486 100644
--- a/src/main/scala/Chisel/testers/BasicTester.scala
+++ b/src/main/scala/Chisel/testers/BasicTester.scala
@@ -24,4 +24,6 @@ class BasicTester extends Module {
       pushCommand(Stop(Node(clock), 0))
     }
   }
+
+  def finish(): Unit = {}
 }
diff --git a/src/main/scala/Chisel/testers/TesterDriver.scala b/src/main/scala/Chisel/testers/TesterDriver.scala
index b25b160b..2b7019c9 100644
--- a/src/main/scala/Chisel/testers/TesterDriver.scala
+++ b/src/main/scala/Chisel/testers/TesterDriver.scala
@@ -23,7 +23,7 @@ object TesterDriver extends BackendCompilationUtilities {
     * frontend, and which can be turned into executables with assertions. */
   def execute(t: () => BasicTester, additionalVResources: Seq[String] = Seq()): Boolean = {
     // Invoke the chisel compiler to get the circuit's IR
-    val circuit = Driver.elaborate(t)
+    val circuit = Driver.elaborate(finishWrapper(t))
 
     // Set up a bunch of file handlers based on a random temp filename,
     // plus the quirks of Verilator's naming conventions
@@ -55,4 +55,17 @@ object TesterDriver extends BackendCompilationUtilities {
       false
     }
   }
+  /*
+   * provides a hook for testers to implement necessary control logic for tests after the
+   * implementation of the users test definition has been completed.
+   * typically the finish method will inspect the users circuit and connect the tester
+   * to the device under test
+   */
+  def finishWrapper(test: () => BasicTester): () => BasicTester = {
+    () => {
+      val tester = test()
+      tester.finish()
+      tester
+    }
+  }
 }
diff --git a/src/test/scala/chiselTests/DeqIOSpec.scala b/src/test/scala/chiselTests/DeqIOSpec.scala
index 8f7937ab..dfa63eb1 100644
--- a/src/test/scala/chiselTests/DeqIOSpec.scala
+++ b/src/test/scala/chiselTests/DeqIOSpec.scala
@@ -8,52 +8,28 @@ import Chisel.testers.BasicTester
 /**
   * Created by chick on 2/8/16.
   */
-class UsesDeqIOInfo extends Bundle {
-  val test_width = 32
+class FinishTester extends BasicTester {
+  var finish_was_run = false
 
-  val info_data = UInt(width = test_width)
+  override def finish(): Unit = {
+    finish_was_run = true
+  }
 }
 
-class UsesDeqIO extends Module {
-  val io = new Bundle {
-    val in = new DeqIO(new UsesDeqIOInfo)
-    val out = new EnqIO(new UsesDeqIOInfo)
+class FinishTesterSpec extends ChiselFlatSpec {
+  class DummyCircuit extends Module {
+    val io = new Bundle {
+      val in = Bool(INPUT)
+      val out = Bool(OUTPUT)
+    }
   }
-}
 
-class DeqIOSpec extends ChiselFlatSpec {
   runTester {
-    new BasicTester {
-      val dut = new UsesDeqIO
-
-      "DeqIO" should "set the direction of it's parameter to INPUT" in {
-        assert(dut.io.in.bits.info_data.dir === INPUT)
-      }
-      "DeqIO" should "create a valid input and ready output" in {
-        assert(dut.io.in.valid.dir === INPUT)
-        assert(dut.io.in.ready.dir === OUTPUT)
-      }
-      "EnqIO" should "set the  direction of it's parameter OUTPUT" in {
-        assert(dut.io.out.bits.info_data.dir === OUTPUT)
-      }
-      "EnqIO" should "create a valid input and ready output" in {
-        assert(dut.io.out.valid.dir === OUTPUT)
-        assert(dut.io.out.ready.dir === INPUT)
-      }
-
-      val in_clone = dut.io.in.cloneType
-      val out_clone = dut.io.out.cloneType
-
-      "A deqIO device" should "clone itself with it's directions intact" in {
-        assert(dut.io.in.bits.info_data.dir == in_clone.bits.info_data.dir)
-        assert(dut.io.in.ready.dir == in_clone.ready.dir)
-        assert(dut.io.in.valid.dir == in_clone.valid.dir)
-      }
+    new FinishTester {
+      val dut = new DummyCircuit
 
-      "A enqIO device" should "clone itself with it's directions intact" in {
-        assert(dut.io.out.bits.info_data.dir == out_clone.bits.info_data.dir)
-        assert(dut.io.out.ready.dir == out_clone.ready.dir)
-        assert(dut.io.out.valid.dir == out_clone.valid.dir)
+      "Extending BasicTester" should "allow developer to have finish method run automatically" in {
+        assert(finish_was_run)
       }
     }
   }
diff --git a/src/test/scala/chiselTests/TesterDriverSpec.scala b/src/test/scala/chiselTests/TesterDriverSpec.scala
new file mode 100644
index 00000000..ea1692f9
--- /dev/null
+++ b/src/test/scala/chiselTests/TesterDriverSpec.scala
@@ -0,0 +1,10 @@
+// See LICENSE for license details.
+
+package chiselTests
+
+/**
+  * Created by chick on 2/9/16.
+  */
+class TesterDriverSpec {
+
+}
-- 
cgit v1.2.3


From 29dc51f8a1e1ebb3072fd6e7bd5fba7c331d029b Mon Sep 17 00:00:00 2001
From: chick
Date: Tue, 9 Feb 2016 11:32:19 -0800
Subject: fixed test for finish method being put in the wrong place

---
 src/test/scala/chiselTests/DeqIOSpec.scala        | 54 ++++++++++++++++-------
 src/test/scala/chiselTests/TesterDriverSpec.scala | 32 +++++++++++++-
 2 files changed, 70 insertions(+), 16 deletions(-)

diff --git a/src/test/scala/chiselTests/DeqIOSpec.scala b/src/test/scala/chiselTests/DeqIOSpec.scala
index dfa63eb1..8f7937ab 100644
--- a/src/test/scala/chiselTests/DeqIOSpec.scala
+++ b/src/test/scala/chiselTests/DeqIOSpec.scala
@@ -8,28 +8,52 @@ import Chisel.testers.BasicTester
 /**
   * Created by chick on 2/8/16.
   */
-class FinishTester extends BasicTester {
-  var finish_was_run = false
+class UsesDeqIOInfo extends Bundle {
+  val test_width = 32
 
-  override def finish(): Unit = {
-    finish_was_run = true
-  }
+  val info_data = UInt(width = test_width)
 }
 
-class FinishTesterSpec extends ChiselFlatSpec {
-  class DummyCircuit extends Module {
-    val io = new Bundle {
-      val in = Bool(INPUT)
-      val out = Bool(OUTPUT)
-    }
+class UsesDeqIO extends Module {
+  val io = new Bundle {
+    val in = new DeqIO(new UsesDeqIOInfo)
+    val out = new EnqIO(new UsesDeqIOInfo)
   }
+}
 
+class DeqIOSpec extends ChiselFlatSpec {
   runTester {
-    new FinishTester {
-      val dut = new DummyCircuit
+    new BasicTester {
+      val dut = new UsesDeqIO
+
+      "DeqIO" should "set the direction of it's parameter to INPUT" in {
+        assert(dut.io.in.bits.info_data.dir === INPUT)
+      }
+      "DeqIO" should "create a valid input and ready output" in {
+        assert(dut.io.in.valid.dir === INPUT)
+        assert(dut.io.in.ready.dir === OUTPUT)
+      }
+      "EnqIO" should "set the  direction of it's parameter OUTPUT" in {
+        assert(dut.io.out.bits.info_data.dir === OUTPUT)
+      }
+      "EnqIO" should "create a valid input and ready output" in {
+        assert(dut.io.out.valid.dir === OUTPUT)
+        assert(dut.io.out.ready.dir === INPUT)
+      }
+
+      val in_clone = dut.io.in.cloneType
+      val out_clone = dut.io.out.cloneType
+
+      "A deqIO device" should "clone itself with it's directions intact" in {
+        assert(dut.io.in.bits.info_data.dir == in_clone.bits.info_data.dir)
+        assert(dut.io.in.ready.dir == in_clone.ready.dir)
+        assert(dut.io.in.valid.dir == in_clone.valid.dir)
+      }
 
-      "Extending BasicTester" should "allow developer to have finish method run automatically" in {
-        assert(finish_was_run)
+      "A enqIO device" should "clone itself with it's directions intact" in {
+        assert(dut.io.out.bits.info_data.dir == out_clone.bits.info_data.dir)
+        assert(dut.io.out.ready.dir == out_clone.ready.dir)
+        assert(dut.io.out.valid.dir == out_clone.valid.dir)
       }
     }
   }
diff --git a/src/test/scala/chiselTests/TesterDriverSpec.scala b/src/test/scala/chiselTests/TesterDriverSpec.scala
index ea1692f9..6271d382 100644
--- a/src/test/scala/chiselTests/TesterDriverSpec.scala
+++ b/src/test/scala/chiselTests/TesterDriverSpec.scala
@@ -5,6 +5,36 @@ package chiselTests
 /**
   * Created by chick on 2/9/16.
   */
-class TesterDriverSpec {
+import Chisel._
+import Chisel.testers.BasicTester
 
+/**
+  * Created by chick on 2/8/16.
+  */
+class FinishTester extends BasicTester {
+  var finish_was_run = false
+
+  override def finish(): Unit = {
+    finish_was_run = true
+  }
+}
+
+class TesterDriverSpec extends ChiselFlatSpec {
+  class DummyCircuit extends Module {
+    val io = new Bundle {
+      val in = Bool(INPUT)
+      val out = Bool(OUTPUT)
+    }
+  }
+
+  runTester {
+    new FinishTester {
+      val dut = new DummyCircuit
+
+      "Extending BasicTester" should "allow developer to have finish method run automatically" in {
+        assert(finish_was_run)
+      }
+    }
+  }
 }
+
-- 
cgit v1.2.3


From 82da362037a227a1c16eb56ead69f3f73cea6c4f Mon Sep 17 00:00:00 2001
From: chick
Date: Wed, 10 Feb 2016 12:11:51 -0800
Subject: Added some comments describing potential use of the finish method
 hook clean up the test example

---
 src/main/scala/Chisel/testers/BasicTester.scala   |  5 ++++
 src/test/scala/chiselTests/TesterDriverSpec.scala | 32 +++++++++++++----------
 2 files changed, 23 insertions(+), 14 deletions(-)

diff --git a/src/main/scala/Chisel/testers/BasicTester.scala b/src/main/scala/Chisel/testers/BasicTester.scala
index 98033486..8f4d60f4 100644
--- a/src/main/scala/Chisel/testers/BasicTester.scala
+++ b/src/main/scala/Chisel/testers/BasicTester.scala
@@ -25,5 +25,10 @@ class BasicTester extends Module {
     }
   }
 
+  /** Called this class or a subclass's constructor has finished giving
+    * developers of chisel testers a post construction hook.
+    * For example, a decoupled tester subclassing BasicTester could override finish in order to
+    * add flow control logic around loading the device under test's input io from a Vec of values
+    */
   def finish(): Unit = {}
 }
diff --git a/src/test/scala/chiselTests/TesterDriverSpec.scala b/src/test/scala/chiselTests/TesterDriverSpec.scala
index 6271d382..bfe8602a 100644
--- a/src/test/scala/chiselTests/TesterDriverSpec.scala
+++ b/src/test/scala/chiselTests/TesterDriverSpec.scala
@@ -2,39 +2,43 @@
 
 package chiselTests
 
-/**
-  * Created by chick on 2/9/16.
-  */
 import Chisel._
 import Chisel.testers.BasicTester
 
-/**
-  * Created by chick on 2/8/16.
+/** Extends basic tester with a finish method.
+  *
   */
 class FinishTester extends BasicTester {
   var finish_was_run = false
 
   override def finish(): Unit = {
-    finish_was_run = true
+     finish_was_run = true
   }
 }
 
 class TesterDriverSpec extends ChiselFlatSpec {
   class DummyCircuit extends Module {
     val io = new Bundle {
-      val in = Bool(INPUT)
-      val out = Bool(OUTPUT)
+      val in = UInt(INPUT, width = 1)
+      val out = UInt(OUTPUT, width = 1)
     }
+
+    io.out := io.in
   }
 
-  runTester {
-    new FinishTester {
-      val dut = new DummyCircuit
+  class DummyTester extends FinishTester {
+    val dut = new DummyCircuit
+
+    dut.io.in := UInt(1)
+    Chisel.assert(dut.io.out === UInt(1))
 
-      "Extending BasicTester" should "allow developer to have finish method run automatically" in {
-        assert(finish_was_run)
-      }
+    "Extending BasicTester" should "allow developer to have finish method run automatically" in {
+      assert(finish_was_run)
     }
   }
+
+  runTester {
+    new DummyTester
+  }
 }
 
-- 
cgit v1.2.3


From c9d9a313b39fa1d43f794c85ec31d8deb847dc9c Mon Sep 17 00:00:00 2001
From: chick
Date: Wed, 10 Feb 2016 13:49:13 -0800
Subject: The TesterDriverSpec has been made stronger by altering circuit in
 finish method

---
 src/test/scala/chiselTests/TesterDriverSpec.scala | 56 ++++++++++++++---------
 1 file changed, 34 insertions(+), 22 deletions(-)

diff --git a/src/test/scala/chiselTests/TesterDriverSpec.scala b/src/test/scala/chiselTests/TesterDriverSpec.scala
index bfe8602a..d7cb0305 100644
--- a/src/test/scala/chiselTests/TesterDriverSpec.scala
+++ b/src/test/scala/chiselTests/TesterDriverSpec.scala
@@ -5,40 +5,52 @@ package chiselTests
 import Chisel._
 import Chisel.testers.BasicTester
 
-/** Extends basic tester with a finish method.
-  *
+/** Extend basic tester with a finish method.  TesterDriver will call the
+  * finish method after the Tester's constructor has completed
+  * -
+  * In this example we use last connect semantics to alter the circuit after
+  * the constructor has completed
   */
 class FinishTester extends BasicTester {
-  var finish_was_run = false
+  val test_wire_width = 2
+  val test_wire_override_value = 3
+
+  val test_wire = Wire(UInt(1, width = test_wire_width))
+
+  test_wire := UInt(1, width = test_wire_width)
+  // though we just test_wire to 1, the assert below will be true because
+  // the finish will override it
+  assert(test_wire === UInt(test_wire_override_value))
 
   override def finish(): Unit = {
-     finish_was_run = true
+    test_wire := UInt(test_wire_override_value, width = test_wire_width)
   }
 }
 
-class TesterDriverSpec extends ChiselFlatSpec {
-  class DummyCircuit extends Module {
-    val io = new Bundle {
-      val in = UInt(INPUT, width = 1)
-      val out = UInt(OUTPUT, width = 1)
-    }
-
-    io.out := io.in
+class DummyCircuit extends Module {
+  val io = new Bundle {
+    val in = UInt(INPUT, width = 1)
+    val out = UInt(OUTPUT, width = 1)
   }
 
-  class DummyTester extends FinishTester {
-    val dut = new DummyCircuit
+  io.out := io.in
+}
+
+class DummyTester extends FinishTester {
+  val dut = Module(new DummyCircuit)
 
-    dut.io.in := UInt(1)
-    Chisel.assert(dut.io.out === UInt(1))
+  dut.io.in := UInt(1)
+  Chisel.assert(dut.io.out === UInt(1))
 
-    "Extending BasicTester" should "allow developer to have finish method run automatically" in {
-      assert(finish_was_run)
-    }
-  }
+  stop()
+}
 
-  runTester {
-    new DummyTester
+class TesterDriverSpec extends ChiselFlatSpec {
+  "TesterDriver calls a BasicTester subclass's finish method which" should
+    "allow modifications of test circuit after tester constructor is done" in {
+    assertTesterPasses {
+      new DummyTester
+    }
   }
 }
 
-- 
cgit v1.2.3


From a9355ba3784e3c5ae611f096a48b4ee5c78464ad Mon Sep 17 00:00:00 2001
From: chick
Date: Wed, 10 Feb 2016 14:29:49 -0800
Subject: TesterDriverSpec is simpler and cleaner, extraneous circuit has been
 removed. Cleanup comment for finish method in BasicTester, and finishWrapper
 in TesterDriver.

---
 src/main/scala/Chisel/testers/BasicTester.scala   |  8 ++--
 src/main/scala/Chisel/testers/TesterDriver.scala  | 10 ++---
 src/test/scala/chiselTests/TesterDriverSpec.scala | 48 +++++++++--------------
 3 files changed, 26 insertions(+), 40 deletions(-)

diff --git a/src/main/scala/Chisel/testers/BasicTester.scala b/src/main/scala/Chisel/testers/BasicTester.scala
index 8f4d60f4..d3e9e7c8 100644
--- a/src/main/scala/Chisel/testers/BasicTester.scala
+++ b/src/main/scala/Chisel/testers/BasicTester.scala
@@ -25,10 +25,10 @@ class BasicTester extends Module {
     }
   }
 
-  /** Called this class or a subclass's constructor has finished giving
-    * developers of chisel testers a post construction hook.
-    * For example, a decoupled tester subclassing BasicTester could override finish in order to
-    * add flow control logic around loading the device under test's input io from a Vec of values
+  /** The finish method provides a hook that subclasses of BasicTester can use to
+    * alter a circuit after their constructor has been called.
+    * For example, a specialized tester subclassing BasicTester could override finish in order to
+    * add flow control logic for a decoupled io port of a device under test
     */
   def finish(): Unit = {}
 }
diff --git a/src/main/scala/Chisel/testers/TesterDriver.scala b/src/main/scala/Chisel/testers/TesterDriver.scala
index 2b7019c9..4547f48f 100644
--- a/src/main/scala/Chisel/testers/TesterDriver.scala
+++ b/src/main/scala/Chisel/testers/TesterDriver.scala
@@ -55,12 +55,10 @@ object TesterDriver extends BackendCompilationUtilities {
       false
     }
   }
-  /*
-   * provides a hook for testers to implement necessary control logic for tests after the
-   * implementation of the users test definition has been completed.
-   * typically the finish method will inspect the users circuit and connect the tester
-   * to the device under test
-   */
+  /**
+    * Calls the finish method of an BasicTester or a class that extends it.
+    * The finish method is a hook for code that augments the circuit built in the constructor.
+    */
   def finishWrapper(test: () => BasicTester): () => BasicTester = {
     () => {
       val tester = test()
diff --git a/src/test/scala/chiselTests/TesterDriverSpec.scala b/src/test/scala/chiselTests/TesterDriverSpec.scala
index d7cb0305..dfdd07cc 100644
--- a/src/test/scala/chiselTests/TesterDriverSpec.scala
+++ b/src/test/scala/chiselTests/TesterDriverSpec.scala
@@ -5,51 +5,39 @@ package chiselTests
 import Chisel._
 import Chisel.testers.BasicTester
 
-/** Extend basic tester with a finish method.  TesterDriver will call the
-  * finish method after the Tester's constructor has completed
-  * -
-  * In this example we use last connect semantics to alter the circuit after
-  * the constructor has completed
+/** Extend BasicTester with a simple circuit and finish method.  TesterDriver will call the
+  * finish method after the FinishTester's constructor has completed, which will alter the
+  * circuit after the constructor has finished.
   */
 class FinishTester extends BasicTester {
   val test_wire_width = 2
   val test_wire_override_value = 3
 
+  val counter = Counter(1)
+
+  when(counter.inc()) {
+    stop()
+  }
+
   val test_wire = Wire(UInt(1, width = test_wire_width))
 
-  test_wire := UInt(1, width = test_wire_width)
-  // though we just test_wire to 1, the assert below will be true because
-  // the finish will override it
+  // though we just set test_wire to 1, the assert below will pass because
+  // the finish will change it's value
   assert(test_wire === UInt(test_wire_override_value))
 
+  /** In finish we use last connect semantics to alter the test_wire in the circuit
+    * with a new value
+    */
   override def finish(): Unit = {
-    test_wire := UInt(test_wire_override_value, width = test_wire_width)
-  }
-}
-
-class DummyCircuit extends Module {
-  val io = new Bundle {
-    val in = UInt(INPUT, width = 1)
-    val out = UInt(OUTPUT, width = 1)
+    test_wire := UInt(test_wire_override_value)
   }
-
-  io.out := io.in
-}
-
-class DummyTester extends FinishTester {
-  val dut = Module(new DummyCircuit)
-
-  dut.io.in := UInt(1)
-  Chisel.assert(dut.io.out === UInt(1))
-
-  stop()
 }
 
 class TesterDriverSpec extends ChiselFlatSpec {
-  "TesterDriver calls a BasicTester subclass's finish method which" should
-    "allow modifications of test circuit after tester constructor is done" in {
+  "TesterDriver calls BasicTester's finish method which" should
+    "allow modifications of test circuit after the tester's constructor is done" in {
     assertTesterPasses {
-      new DummyTester
+      new FinishTester
     }
   }
 }
-- 
cgit v1.2.3