ci: slightly faster rust checks #5048
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
We were previously running an isolated pd & cometbft instance specifically for the smoke tests, via process-compose. We already have a rather robust dev-env that also leverages process-compose, but includes auxiliary services, like metrics, postgres, and pindexer. That dev env isn't exercised anywhere in CI, so let's reuse it as part of the smoke test suite, which both reduces code duplication in the test harness scripts, and opens the door to adding integration tests for pindexer.
ci: use separate target-dir for check In order to disallow warnings, we set `RUSTFLAGS="-D warnings"` when running `cargo check`. Doing so, however, busts the cache, making it impossible to reuse artifacts already built for other work. Let's give just the check action its own cache dir, which will allow both to exist side by side. Breaks out the compat and feature checks into a separate job. Also makes a few small tweaks to improve runtime throughout: * ci: don't run release workflow on PRs This substnatially reduces the amount of checks per PR, which makes the entire test run more readable at a glance. * ci: fix buf lint via forks When running in a private fork, e.g. for testing CI, the buf lint check will fail, because the GITHUB_REPOSITORY var points to the private repo URL. Instead, let's just check against the public protocol repo: that's really the answer we want. Doing so will display warnings if protocol buffer definitions break compatibility against the published main branch. And renames the workflows to be more concise and readable at a glance.
Updates the pmonitor integration tests to use a feature gate, rather than the old "ignore" pragma. Implements a TODO in the pmonitor checks, to poll for ready state, rather than guessing and sleeping.
conorsch
added
the
A-CI/CD
|
The status checks being renamed will require an update to the branch-protection rules. The two (2) that were required were the buf checks. I can handle that in follow-up after merge. |
2 tasks
conorsch
added a commit
that referenced
this pull request
Feb 5, 2025
## Describe your changes Follow-up to #5009, in which these tests were introduced, and #5048, which refactored the smoke tests to make additions like this easier. ## Issue ticket number and link See above. ## Testing and review The tests were already merged, this is just enabling them in CI, so CI passing, specifically on the smoke-test job, is enough. ## Checklist before requesting a review - [x] I have added guiding text to explain how a reviewer should test these changes. - [x] If this code contains consensus-breaking changes, I have added the "consensus-breaking" label. Otherwise, I declare my belief that there are not consensus-breaking changes, for the following reason: > tests/ci only, no changes to application code
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Describe your changes
A bit of spring cleaning for the CI suite. These changes aim at faster run times on PRs, mostly via reusing common tooling, and reducing setup time. These changes don't affect which tests run, out of an abundance of caution, because we have ongoing development in a feature branch right now (#5010).
The longest-running CI check is currently the check-all-features job, which clocks in at ~14m with these changes. That's an improvement over the ~20m or so it's been taking on other PRs. For full details on the changes, see the commit messages; to summarize:
just check, to make CI failures more easily replicable locallyOverall the checks seem to complete >5m faster than before, which is good enough to merge as progress. I'm deferring more substantive changes like kicking long-running jobs to schedules, rather than on-every-PR, until we have a chance to observe these improvements over time, and until after the LQT push.
Example of CI job display before changes
Example of CI job display after changes
Issue ticket number and link
N/A
Checklist before requesting a review
If this code contains consensus-breaking changes, I have added the "consensus-breaking" label. Otherwise, I declare my belief that there are not consensus-breaking changes, for the following reason: