-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Include cts_runner tests in CTS jobs #8547
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
base: trunk
Are you sure you want to change the base?
Conversation
.github/workflows/cts.yml
Outdated
| - name: run CTS | ||
| - name: Test cts_runner | ||
| shell: bash | ||
| run: cargo --locked xtask test --llvm-cov -p cts_runner |
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.
I don't think it's particularly interesting or useful for these to contribute to coverage, but it seems like building with --llvm-cov maximizes build reuse between this and the next step.
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.
Never mind, --llvm-cov doesn't get along with deno.
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.
Also, I don't think we actually share artifacts between xtask and the workspace?
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.
Why wouldn't they be shared?
I actually think my original concern about needing llvm-cov for build reuse wasn't accurate, the compilation of the dependencies appears to be mostly reused anyways, I guess because they aren't instrumented for coverage.
The CTS job doesn't currently install cargo-nextest. In one of the intermediate versions of the job, I added it, but the current version doesn't have it, and doesn't use the test xtask. I could add it back if it seemed like consistency amongst our test jobs is that important.
There are only 3 tests in cts_runner, so there's not a lot at stake here.
8a26612 to
9e38f97
Compare
9e38f97 to
cbb7787
Compare
|
From today's maintainers' meeting:
🙏🏻💦 |
Stumbling on #3164 this morning reminded me that these tests weren't being run in CI.
Testing
Adds already passing tests to CI so they don't regress.
Squash or Rebase? Squash