ci: multi Bitcoin Core RPC testing#6927
ci: multi Bitcoin Core RPC testing#6927federico-stacks merged 24 commits intostacks-network:developfrom
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #6927 +/- ##
===========================================
+ Coverage 54.68% 55.80% +1.12%
===========================================
Files 412 412
Lines 218662 219958 +1296
Branches 338 338
===========================================
+ Hits 119571 122756 +3185
+ Misses 99091 97202 -1889 see 257 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
|
From the CI side, the changes appear fine - but the question i have is more around rate-limits from dockerhub. https://docs.docker.com/docker-hub/usage/ I think the question here is - will gha pull these images from a new ip for each runner, or would they use a shared egress? or is there some agreement so gha isn't affected at all? 🤔 if the above is possible, we do have some options though (ex: building the docker image ourselves per PR, or in a bitcoin fork where the binaries come from - we could push to ghcr) |
Fair point.
Here the official reference: https://docs.github.com/en/actions/reference/limits#docker-hubs-rate-limit-for-github-actions So we should be safe from hitting those limits in our current setup. |
|
@wileyj just in case you miss the Additional Info section. I would like to know your thoughts about the option to run this workflow nightly? Or do you prefer start like this and improve later or in a follow-up PR? |
i don't have a strong opinion for either, but when i looked at the job - it takes roughly ~10 minutes in total. based on that, i think it's fine to run for each PR vs nightly (i would reserver nightly jobs for longer tasks or non-blocking workflows). this workflow specifically, i feel has a good signal - if the rpc breaks for a major version of bitcoin, it's something we should know about (and today it relies on manuallly testing it). i say keep it the way you wrote it. if it causes pain later, it may be adjusted. edit: there is one thing to consider actually - if kept as-is, and for example and older version of btc causes a test failure due to a deprecated rpc or something, then the entire ci workflow will appear as "failed". one option to prevent a (visual only) failed ci is to trigger this workflow at the same time as the main ci does, i.e. |
OK. My concern wasn’t about performance, but rather about continuously validating Bitcoin RPC compatibility across all supported major versions, even when there are no active pull requests. That said, I’m fine starting with the current setup (running on each PR). If we later feel that continuous validation is important enough, we can always switch it to a nightly schedule.
That might actually be desirable. If something breaks on the RPC side, it’s probably something we should address promptly. Also, if we run this workflow in parallel with the main CI, wouldn’t we need to rebuild the binaries separately increasing load on the github runners for each PR? (Or eventually we should try to serialize the workflows) If you don’t have a strong opinion here, I’d lean toward keeping the workflow as-is for now. |
Pull Request Test Coverage Report for Build 23488680821Details
💛 - Coveralls |
|
This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
Description
This PR introduces a new CI workflow designed to run our Bitcoin RPC test suite against multiple versions of Bitcoin Core, ensuring continued compatibility across supported major releases.
What's new:
testcontainersas a development dependency to run a containerized Bitcoin Core service, encapsulated in theBitcoinCoreContainerstruct.BitcoinCoreContainer, replacing the previousBitcoinCoreController.bitcoin-rpc-tests.yml(triggered fromci.yml), which:Note: To improve maintainability and testability, the Docker image discovery script has been extracted to
.github/workflow/scripts/docker-bitcoin-majors.Job execution example:
Applicable issues
Additional info (benefits, drawbacks, caveats)
> Alternative execution strategy (nightly runs)
As a potential improvement, we could consider moving the multi-version compatibility workflow to a scheduled nightly run instead of executing it on every PR. This would allow us to continuously validate Bitcoin RPC compatibility across all supported major versions, even in the absence of active pull requests.
In this model, the CI pipeline for PRs could be simplified to run the test suite only against the default Bitcoin Core version (as is currently done for other integration tests), while the full dynamically discovered version matrix would execute nightly.
> Other approaches evaluated
rstestwith compile-time configuration:> Generic CI improvement
In the
bitcoin-rpc-tests.ymlworkflow,stacks-network/actions/stacks-core/testenv@mainis reused to set up the test environment. This action also restores a cached Bitcoin binary, which is not used in this job since tests rely exclusively on the containerized Bitcoin Core instance.Although the cache restore only adds a few seconds to the runtime (so no big issue there), we could streamline the workflow by skipping the Bitcoin cache if not explicitly requested.
Checklist
docs/property-testing.md)rpc/openapi.yamlfor RPC endpoints,event-dispatcher.mdfor new events)clarity-benchmarkingrepo