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

[ci] Run more on-host tests in sw_tests #15566

Merged
merged 2 commits into from
Oct 20, 2022

Conversation

dmcardle
Copy link
Contributor

I noticed that a few unit test targets outside of //sw/... were not running on CI.

This commit ensures that CI runs the following additional targets:

  • //hw/ip/rom_ctrl/util:gen_vivado_mem_image_test (after Fix OTP bitstream splicing #15163 merges)
  • //rules/scripts:bitstreams_workspace_test
  • //util:generate_compilation_db_test

Signed-off-by: Dan McArdle dmcardle@google.com

@dmcardle dmcardle requested review from milesdai and removed request for rswarbrick October 18, 2022 18:45
@dmcardle
Copy link
Contributor Author

Incidentally, I am the author of those uncovered test targets 🤦

@dmcardle dmcardle added Type:Enhancement Feature requests, enhancements Component:CI Continuous Integration (Azure Pipelines & Co.) labels Oct 18, 2022
Copy link
Contributor

@drewmacrae drewmacrae left a comment

Choose a reason for hiding this comment

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

Once you can get it to build without verilator LGTM.

@dmcardle dmcardle force-pushed the dmcardle/expand-ci-test-coverage branch from 8482234 to f309151 Compare October 18, 2022 20:29
@dmcardle dmcardle added the Status:Ready to merge PR is ready to be merged by a committer. label Oct 18, 2022
tags = ["verilator"],
tags = [
"manual",
"verilator",
Copy link
Contributor

Choose a reason for hiding this comment

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

FYI, the verilator tag wasn't filtering it out of the build because --test_tag_filters doesn't affect what's built, and it's building all the targets that match the wildcard because of --build_tests_only=false

@dmcardle dmcardle force-pushed the dmcardle/expand-ci-test-coverage branch from 064eda9 to 0d700f3 Compare October 19, 2022 19:32
I noticed that a few unit test targets outside of //sw/... were not
running on CI.

This commit ensures that CI runs the following additional targets:
* //hw/ip/rom_ctrl/util:gen_vivado_mem_image_test (after lowRISC#15163 merges)
* //rules/scripts:bitstreams_workspace_test
* //util:generate_compilation_db_test

Signed-off-by: Dan McArdle <dmcardle@google.com>
Signed-off-by: Dan McArdle <dmcardle@google.com>
@dmcardle dmcardle force-pushed the dmcardle/expand-ci-test-coverage branch from 0d700f3 to e953cf4 Compare October 19, 2022 20:34
@dmcardle
Copy link
Contributor Author

Oops. I originally added these two lines:

+      -//quality/...\
+      -//third_party/riscv-compliance/... \

Because (a) there was no whitespace between //quality/... and the \ character, and (b) YAML correctly stripped the whitespace on the next line, I effectively added one long flag: -//quality/...-//third_party/riscv-compliance/....

@drewmacrae drewmacrae merged commit 9644452 into lowRISC:master Oct 20, 2022
@dmcardle dmcardle deleted the dmcardle/expand-ci-test-coverage branch October 20, 2022 13:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component:CI Continuous Integration (Azure Pipelines & Co.) Status:Ready to merge PR is ready to be merged by a committer. Type:Enhancement Feature requests, enhancements
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants