Skip to content

Conversation

seldridge
Copy link
Member

This is three commits that should be reviewed independently. This builds towards and then adds a HasConfigMap trait which allows for -D<key>=<value> options to be exposed to chisel3.simulator.scalatest.ChiselTest. In order to do this, and without starting to add functionality directly to ChiselSim, the existing TestingDirectory trait is refactored to be made stackable.

Release Notes

Add the ability for users to pass command-line options, via -D<key>=<value>, to any chisel3.simulator.scalatest.ChiselTest test for them to customize the execution of the test.

Copy link

linux-foundation-easycla bot commented Mar 5, 2025

CLA Signed

The committers listed above are authorized under a signed CLA.

@seldridge seldridge requested a review from jackkoenig March 5, 2025 20:19
@seldridge seldridge added the Feature New feature, will be included in release notes label Mar 5, 2025
@seldridge
Copy link
Member Author

Testing this a bit more, I may need another approach as the configMap is only valid inside the test and not outside it. If you are trying to use a -D option to set a seed and have that used in a test name, this won't work. There are a couple of other ways to get at Scalatest's config map that I'm going to look at.

@seldridge
Copy link
Member Author

Testing this a bit more, I may need another approach as the configMap is only valid inside the test and not outside it. If you are trying to use a -D option to set a seed and have that used in a test name, this won't work. There are a couple of other ways to get at Scalatest's config map that I'm going to look at.

I was able to workaround this problem with #4774. That provides the ability to get a new HasTestingDirectory with a subdirectory. This can be used to manually change the directory of calls to simulate (or other methods which need a HasTestingDirectory type class implementation).

Add a new, stackable trait called `HasConfigMap` which exposes Scalatest's
`configMap` [[1]] in an easy-to-use way.  This allows for users to pass
arguments on the command line to Scalatest via the `-D<key>=<foo>` option
and have access to them in a `Map[String, Any]` in their test.

While I am generally not hugely in favor of this pattern, as it can be
abused to make tests pass or fail via options when I would prefer that
tests were self-contained, there are some internal usages of this for
doing things like changing the simulator, turning on waveforms, or
extending the length of randomized testing.  Given that, it's a useful
feature to have and I would prefer if users went rolling their own
solutions to this.

[1]: https://www.scalatest.org/scaladoc/3.1.0/org/scalatest/ConfigMap.html

Signed-off-by: Schuyler Eldridge <schuyler.eldridge@sifive.com>
Add a `configMap` to all `ChiselSim` tests so that users have a mechanism
to get `-D<key>=<value>` command line options to their tests.

Signed-off-by: Schuyler Eldridge <schuyler.eldridge@sifive.com>
@jarias-lfx
Copy link

/easycla

Copy link
Contributor

@jackkoenig jackkoenig left a comment

Choose a reason for hiding this comment

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

LGTM but a couple of suggestions

@seldridge seldridge force-pushed the dev/seldridge/add-use-HasConfigMap branch from 943e726 to ef325d3 Compare March 6, 2025 16:37
@seldridge seldridge enabled auto-merge (squash) March 6, 2025 16:38
@seldridge seldridge merged commit 05c87bc into main Mar 6, 2025
14 checks passed
@seldridge seldridge deleted the dev/seldridge/add-use-HasConfigMap branch March 6, 2025 16:57
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.

3 participants