Skip to content
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 sbt test to regressions #2511

Merged

Conversation

seldridge
Copy link
Member

This adds the few Scalatest tests to the regression suite of both Travis and GitHub Actions.

This is largely cargo cult-ed from existing tests, but I expect @richardxia can straighten me out.

Related issue: None

Type of change: other enhancement

Impact: no functional change

Development Phase: implementation

Release Notes

None.

@richardxia
Copy link
Member

The GitHub Actions changes look about right to me, but it looks like they failed because we haven't done the recursive submodule init. I think the Makefiles currently handle this, which is why the other tests didn't need to explicitly do the submodule init step.

One thing you could try is adding a Makefile target that runs the sbt test command but that depends on the git submodule checkout step. We have the git submodule checkouts split into a few different targets so that we don't have to clone all of GCC all the time, so I think the one that you want to target to grab the other dependencies like Chisel is this one:

stamps/other-submodules.stamp:

@seldridge seldridge force-pushed the quis-custodiet-ipsos-custodes branch from 5be76e2 to 8c6fbcc Compare June 9, 2020 16:48
Add `sbt test` to regressions.

Signed-off-by: Schuyler Eldridge <schuyler.eldridge@ibm.com>
@seldridge seldridge force-pushed the quis-custodiet-ipsos-custodes branch from 8c6fbcc to d8d5d0d Compare June 9, 2020 19:18
@seldridge
Copy link
Member Author

Ping @richardxia. Would you be able to take a look? Your suggestion seems to work great.

Copy link
Member

@richardxia richardxia left a comment

Choose a reason for hiding this comment

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

Ping @richardxia. Would you be able to take a look? Your suggestion seems to work great.

Yep, LGTM! Glad that it worked out!

@richardxia richardxia merged commit 1e0db63 into chipsalliance:master Jun 10, 2020
@seldridge seldridge deleted the quis-custodiet-ipsos-custodes branch June 10, 2020 01:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants