ChiselTest Compatibility Layer for Chisel 7#5221
ChiselTest Compatibility Layer for Chisel 7#5221EhsanKhodadad wants to merge 4 commits intochipsalliance:mainfrom
Conversation
|
Howdy @EhsanKhodadad, thank you for the contribution! Can you sign the CLA? |
Hi Jack, thank you for your comment. |
jackkoenig
left a comment
jackkoenig
left a comment
There was a problem hiding this comment.
Thank you again for doing this and for following up on the CLA. I have some comments.
I don't have a very good way to see how well this work in migrating from Chisel 6 with chiseltest to Chisel 7 with this. Do you have some example open-source code where we can see the code diff to perhaps use as a motivating example?
| * This package provides example modules and utilities for testing with the | ||
| * ChiselTest compatibility layer on ChiselSim. | ||
| */ | ||
| package object examples { |
There was a problem hiding this comment.
I don't think of this examples stuff should live in main, I think it should be over in test/scala/chiselTests.
| * These are compatibility stubs that allow code to compile but may not provide | ||
| * full formal verification functionality. | ||
| */ | ||
| package object formal { |
There was a problem hiding this comment.
I'm a bit confused by this, not only is it not supported but this doesn't work right? I think it would be better for it not to compile than to vacuously pass.
| object ChiselTestCompat { | ||
| import chisel3.simulator.EphemeralSimulator._ | ||
|
|
||
| /** | ||
| * Implicit class to add ChiselTest-style operations to Data types | ||
| */ | ||
| implicit class testableData[T <: Data](val x: T) extends AnyVal { |
There was a problem hiding this comment.
Should these maybe be in package object chiseltest so they're available by default when one does a wildcard import? Or is there a reason to keep them separate?
| object DecoupledDriver { | ||
| // For compatibility with imports like: import chiseltest.DecoupledDriver._ | ||
| // The actual implicit classes are defined in the package object | ||
| } |
There was a problem hiding this comment.
I think DecoupledDriver was a class in chiseltest, not an object, so what does this do? import chiseltest.DecoupledDriver._ shouldn't be legal in chiseltest.
|
|
||
| ## ChiselTest Compatibility Layer | ||
|
|
||
| For projects with a large test suite, Chisel now includes a **ChiselTest compatibility layer** at `chiseltest.*` that provides a drop-in replacement for most ChiselTest APIs. This allows you to keep your existing test code while running on ChiselSim. |
There was a problem hiding this comment.
| For projects with a large test suite, Chisel now includes a **ChiselTest compatibility layer** at `chiseltest.*` that provides a drop-in replacement for most ChiselTest APIs. This allows you to keep your existing test code while running on ChiselSim. | |
| For projects with a large test suite, Chisel now includes a **ChiselTest compatibility layer** at `chiseltest._` that provides a drop-in replacement for most ChiselTest APIs. This allows you to keep your existing test code while running on ChiselSim. |
Let's keep the wildcard import suggestion Scala 2-style
|
|
||
| Simply keep your existing imports and test structure: | ||
|
|
||
| ```scala |
There was a problem hiding this comment.
Can this be mdoc to make sure it compiles (like the examples above)?
| ```scala | |
| ```scala mdoc |
2 participants
Contributor Checklist
docs/src?Type of Improvement
Desired Merge Strategy
Release Notes
Reviewer Checklist (only modified by reviewer)
3.6.x,5.x, or6.xdepending on impact, API modification or big change:7.0)?Enable auto-merge (squash)and clean up the commit message.Create a merge commit.