-
Notifications
You must be signed in to change notification settings - Fork 647
Add UnitTest marker and test discovery utility #4642
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add UnitTest marker and test discovery utility #4642
Conversation
seldridge
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Nitty comments throughout and feel free to address them as you see fit.
As an aside: what is interesting about this PR is that it may have also defined a notion of a "package", i.e., a collection of modules that should all be built together.
| // If we are just listing tests, print the class name and skip this test. | ||
| if (config.list) { | ||
| println(className) | ||
| return | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sending this to stdout seems right.
| def check(args: Seq[String])(checkOut: String, checkErr: String): Unit = { | ||
| val outStream = new ByteArrayOutputStream() | ||
| val errStream = new ByteArrayOutputStream() | ||
| Console.withOut(new PrintStream(outStream)) { | ||
| Console.withErr(new PrintStream(errStream)) { | ||
| UnitTests.main(args.toArray) | ||
| } | ||
| } | ||
| if (!checkOut.isEmpty) fileCheckString(outStream.toString)(checkOut) | ||
| if (!checkErr.isEmpty) fileCheckString(errStream.toString)(checkErr) | ||
| } | ||
|
|
||
| def checkOutAndErr(args: String*)(checkOut: String, checkErr: String): Unit = check(args)(checkOut, checkErr) | ||
|
|
||
| def checkOut(args: String*)(checkOut: String): Unit = check(args)(checkOut, "") | ||
| def checkErr(args: String*)(checkErr: String): Unit = check(args)("", checkErr) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is nice.
FWIW, I have had difficulty defining methods like this in the past because Scalatest seems to run things multi-process, but share stdout and stderr. If you start seeing intermittent test failures, this may be the cause.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Interesting 🤔 I was hoping that the withOut and withErr would somehow setup a thread-local redirect of these streams 🤞. Haven't seen a failure yet. But better be on the lookout for these.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
5ef0cb2 to
1682939
Compare
Add the `UnitTest` trait as a marker for classes and objects that should
be automatically discovered and constructed/executed as unit tests for
hardware written in Chisel.
Also add a new `chisel3.UnitTests` main which discovers all unit tests
in the classpath and generates the FIRRTL output for them. The discovery
mechanism is roughly the same as implemented in ScalaTest, which scans
the classpath for JAR files and directories and then tries to guess
class names based on the encountered file structure. This feels pretty
brittle, but has been working reliably for ScalaTest for a long time,
so chances are this will work for Chisel too.
The utility offers a few options to list all available unit tests, or
filter them according to inclusion or exclusion regular expressions.
This allows unit tests of only the current package or a subset of the
current project to be discovered and built. This is particularly
useful if a Chisel project depends on libraries such as rocket-chip or
by necessesity Chisel itself, whose unit tests are likely not of
interest to that project.
Classes and objects can be marked as unit tests as follows:
package magic
class TestA extends UnitTest {
Definition(new MyAdder(1))
Definition(new MyAdder(42))
// ...
}
object TestB extends UnitTest {
for (i <- 1 to 4)
Definition(new MyMultiplier(i))
// ...
}
The `chisel3.UnitTests` utility can then be execute, for example through
mill or a standalone binary:
# List all unit tests:
> mill chisel[].runMain chisel3.UnitTests -l
magic.TestA
magic.TestB
some.other.package.TestX
# Produce FIRRTL output for all unit tests:
> mill chisel[].runMain chisel3.UnitTests -o tests.fir
# Filter tests according to a regex:
> mill chisel[].runMain chisel3.UnitTests -l -f '^magic\.' -x TestA
magic.TestB
As a convenience, Chisel modules can directly be marked as unit test as
long as they do not have any constructor arguments. Doing so will
automatically create a new `Definition` of the module without
instantiating it. This can be useful for modules marked as public that
serve as test harness, or by using a `FormalTest` marker to execute the
module as a formal test.
class MyTestHarness extends RawModule with UnitTest with Public {
// ...
}
class MyFormalTest extends RawModule with UnitTest {
// ...
FormalTest(this)
}
This is a first building block towards adding hardware unit tests to
Chisel in a similar style as `AnyFlatSpec` and friends in ScalaTest,
`#[test]` in Rust, lit tests in LLVM, or gtest in C++.
At a later stage, we may want to automate the execution of unit tests
as formal or simulation checks, similar to how we have helper methods
to run a Chisel generator all the way through to Verilog.
1682939 to
8c49e6d
Compare
Add the
UnitTesttrait as a marker for classes and objects that should be automatically discovered and constructed/executed as unit tests for hardware written in Chisel.Also add a new
chisel3.UnitTestsmain which discovers all unit tests in the classpath and generates the FIRRTL output for them. The discovery mechanism is roughly the same as implemented in ScalaTest, which scans the classpath for JAR files and directories and then tries to guess class names based on the encountered file structure. This feels pretty brittle, but has been working reliably for ScalaTest for a long time, so chances are this will work for Chisel too.The utility offers a few options to list all available unit tests, or filter them according to inclusion or exclusion regular expressions. This allows unit tests of only the current package or a subset of the current project to be discovered and built. This is particularly useful if a Chisel project depends on libraries such as rocket-chip or by necessesity Chisel itself, whose unit tests are likely not of interest to that project.
Classes and objects can be marked as unit tests as follows:
The
chisel3.UnitTestsutility can then be execute, for example through mill or a standalone binary:As a convenience, Chisel modules can directly be marked as unit test as long as they do not have any constructor arguments. Doing so will automatically create a new
Definitionof the module without instantiating it. This can be useful for modules marked as public that serve as test harness, or by using aFormalTestmarker to execute the module as a formal test.This is a first building block towards adding hardware unit tests to Chisel in a similar style as
AnyFlatSpecand friends in ScalaTest,#[test]in Rust, lit tests in LLVM, or gtest in C++.At a later stage, we may want to automate the execution of unit tests as formal or simulation checks, similar to how we have helper methods to run a Chisel generator all the way through to Verilog.
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), clean up the commit message, and label withPlease Merge.Create a merge commit.