Skip to content

Conversation

@fabianschuiki
Copy link
Contributor

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.

Contributor Checklist

  • Did you add Scaladoc to every public function/method?
  • Did you add at least one test demonstrating the PR?
  • Did you delete any extraneous printlns/debugging code?
  • Did you specify the type of improvement?
  • Did you add appropriate documentation in docs/src?
  • Did you request a desired merge strategy?
  • Did you add text to be included in the Release Notes for this change?

Type of Improvement

  • Feature (or new API)

Desired Merge Strategy

  • Squash: The PR will be squashed and merged (choose this if you have no preference).

Release Notes

Reviewer Checklist (only modified by reviewer)

  • Did you add the appropriate labels? (Select the most appropriate one based on the "Type of Improvement")
  • Did you mark the proper milestone (Bug fix: 3.6.x, 5.x, or 6.x depending on impact, API modification or big change: 7.0)?
  • Did you review?
  • Did you check whether all relevant Contributor checkboxes have been checked?
  • Did you do one of the following when ready to merge:
    • Squash: You/ the contributor Enable auto-merge (squash), clean up the commit message, and label with Please Merge.
    • Merge: Ensure that contributor has cleaned up their commit history, then merge with Create a merge commit.

@fabianschuiki fabianschuiki added the Feature New feature, will be included in release notes label Jan 24, 2025
Copy link
Member

@seldridge seldridge left a 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.

Comment on lines +68 to +83
// If we are just listing tests, print the class name and skip this test.
if (config.list) {
println(className)
return
}
Copy link
Member

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.

Comment on lines +11 to +27
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)
Copy link
Member

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.

Copy link
Contributor Author

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@fabianschuiki fabianschuiki force-pushed the unit-test-discovery branch 2 times, most recently from 5ef0cb2 to 1682939 Compare January 27, 2025 21:38
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.
@fabianschuiki fabianschuiki merged commit ae3d735 into chipsalliance:main Jan 27, 2025
15 checks passed
@fabianschuiki fabianschuiki deleted the unit-test-discovery branch January 28, 2025 00:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Feature New feature, will be included in release notes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants