feat: use `testcontainers` for integration tests by cylewitruk · Pull Request #1572 · stacks-sbtc/sbtc

cylewitruk · 2025-04-03T06:42:56Z

Description

Supersedes: #1223

Probably 80% of the changes here are updating tests to use testcontainers vs. regtest::initialize_blockchain().

Changes

Introduces the testcontainers crate and implements images for bitcoin core and emily & co. I stuffed these into a separate crate sbtc-docker-testing at the moment, I'm on the fence if we should bake it into the sbtc crate or not...
Refactors the sbtc::testing::regtest module a bit to get rid of static references (it made sense when there was only a single bitcoind...).
Refactors [almost] all integration tests.
Includes some refactoring of the ZMQ tests due to trying to figure out what was going on with them. They ended up in a "better/more robust" state than they were in to begin with, so I left them the way they are.
Adds a nextest.toml and updates the Makefile commands to use it. This lets us specify specific test filters that nextest will use to group tests, run certain tests serially, using more/less threads, etc.
- Configuration file documentation here
- Test groups and mutual exclusion docs here
Updated CI (WIP)

Note that the current structure is:

sbtc-docker-testing crate: contains the base testcontainers integration
sbtc crate: builds on sbtc-docker-testing adding "common" regtest functionality (faucet, etc). Needed here to support the integration tests in this crate.
signer crate: builds on both sbtc and sbtc-docker-testing, adding additional support for e.g. BitcoinCoreClient, BitcoinCoreMessageStream etc. which are local to this crate.

Testing Information

Make sure you're running the latest nextest (cargo install nextest --locked) as some of the config file options are only available in newer versions.
Otherwise, just use the normal make testing commands as usual.

P.S. still working on getting CI to work properly

Checklist:

I have performed a self-review of my code
My changes generate no new warnings
New and existing unit tests pass locally with my changes
Any dependent changes have been merged and published in downstream modules

Copilot

Pull Request Overview

This PR introduces the use of the testcontainers crate for integration tests by creating a dedicated crate (sbtc-docker-testing) and refactors existing regtest and integration test modules to support asynchronous testing with Tokio. It also updates the CI configuration and Nextest settings.

Introduced sbtc-docker-testing for container image management and integration testing.
Refactored the sbtc regtest module and integration tests to remove static references and leverage async tests.
Updated Cargo.toml and CI workflows to include the new testing crate and nextest configuration.

Reviewed Changes

Copilot reviewed 36 out of 38 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
signer/src/testing/docker.rs	Added Docker test utilities for BitcoinCore.
signer/src/bitcoin/rpc.rs	Added AsRef and Deref implementations for BitcoinCoreClient.
signer/Cargo.toml	Updated testing features to include sbtc-docker-testing.
sbtc/tests/integration/validation.rs	Refactored integration tests to use async Tokio tests with testcontainers.
sbtc/src/testing/regtest.rs	Refactored regtest module and faucet initialization.
sbtc/Cargo.toml	Added sbtc-docker-testing dependency to the testing feature.
nextest.toml	Added configuration for test groups and overrides.
docker/docker-compose.test.yml	Commented out the bitcoind service configuration.
docker-testing/*	Added new testcontainers integration modules (images, logging, error).
Cargo.toml	Added docker-testing as a workspace member and dependency.
.github/workflows/on-push.yaml	Updated CI configuration with the new nextest version.

Files not reviewed (2)

.github/actions/docker.override.conf: Language not supported
Makefile: Language not supported

Comments suppressed due to low confidence (1)

sbtc/src/testing/regtest.rs:211

The signature for Faucet::new has been changed from taking a static reference to owning a Client, which might affect usage elsewhere. Confirm that this change is intentional and update documentation or related code if needed.

fn new(secret_key: &str, kind: AddressType, rpc: Client) -> Self {

Copilot · 2025-04-03T06:43:32Z

docker-testing/src/images/mod.rs

+fn container_name(name: &str) -> String {
+    let suffix: String = rand::thread_rng()
+        .sample_iter(&Alphanumeric)
+        .take(12)


The documentation for container_name states an 8-character random suffix, but the code generates a 12-character suffix. Please update the comment or modify the code to ensure consistency.

moodmosaic · 2025-04-18T13:45:22Z

docker-testing/src/images/mod.rs

+        }
+    })
+    .await
+    .expect("ZMQ endpooint not accessible within allotted timeout");


Suggested change

.expect("ZMQ endpooint not accessible within allotted timeout");

.expect("ZMQ endpoint not accessible within allotted timeout");

moodmosaic · 2025-04-18T13:46:13Z

docker-testing/src/images/mod.rs

+/// # Returns
+/// * `Ok(())` if connection was established successfully
+/// * `Err(Error)` if timeout occurred before successful connection
+pub async fn wait_for_tcp_connectivity(host: &str, port: u16, timeout: Duration) {


Why not use a circuit breaker here (like failsafe-rs or resilient) with exponential back-offs instead of manually handling timeouts?

We want the tests to be able to continue ASAP, so exponential back-off isn't really applicable, at least in the current use case. This is almost instantaneous in the current usages as well, so bringing in another dependency for something tokio already gives us isn't very appealing.

moodmosaic · 2025-04-18T13:54:40Z

docker-testing/src/images/mod.rs

+        .map(char::from)
+        .collect();
+
+    format!("sbtc-test-{name}-{suffix}")


Could we include the test name in the container name for easier debugging?

Current: sbtc-test-bitcoind-abc123

Maybe: sbtc-test-bitcoind-test_get_block_works-abc123?

In this example, test_get_block_works is the test name (can probably be derived).

Using a derive macro, sure, but otherwise we're stuck either providing the name as a string, or using a (questionably reliable) hack like any::type_name<fn()>() and stripping module prefix/suffixes...

djordon · 2025-04-18T14:59:53Z

sbtc/Cargo.toml

+more-asserts.workspace = true
+sbtc = { path = ".", features = ["testing"] }
+sbtc-docker-testing.workspace = true
+tokio.workspace = true


nit: new line here

djordon · 2025-04-18T15:00:04Z

sbtc/Cargo.toml

 assert_matches.workspace = true
-more-asserts.workspace = true
+more-asserts.workspace = true
+sbtc = { path = ".", features = ["testing"] }


? Is this necessary?

djordon · 2025-04-18T15:10:29Z

docker-testing/src/logging.rs

+        }
+
+        // Return an immediately-resolved future
+        async move {}.boxed()


I think you can return std::future::ready(()) here.

cylewitruk added 14 commits March 25, 2025 10:45

add testcontainers bitcoind and update a few tests

d566a58

add nextest config

a09de72

update all tests

eb5ffc4

Merge branch 'main' into feat/docker-integration-tests

59d482c

wip

619ca2c

Merge branch 'main' into feat/docker-integration-tests

6b17443

works

1924ee8

Merge branch 'main' into feat/docker-integration-tests

f6cfc40

comments

19454b4

Merge branch 'main' into feat/docker-integration-tests

ee5a2bf

fixes after merge main

4d4da07

bump nextest version

37cc0c6

use tab in makefile instead of spaces

35a7e5e

maybe this isn't needed anymore

b400f98

cylewitruk added this to the sBTC: Hardening and resiliency milestone Apr 3, 2025

cylewitruk requested review from Copilot, djordon and matteojug April 3, 2025 06:42

cylewitruk self-assigned this Apr 3, 2025

cylewitruk added this to sBTC Apr 3, 2025

github-project-automation bot moved this to Needs Triage in sBTC Apr 3, 2025

Copilot AI reviewed Apr 3, 2025

View reviewed changes

cylewitruk added 2 commits April 3, 2025 08:48

ci

f9442fe

ci

be7c795

cylewitruk mentioned this pull request Apr 3, 2025

feat: integration test improvements #1223

Closed

cylewitruk moved this from Needs Triage to In Progress in sBTC Apr 3, 2025

cylewitruk added CI Relates to CI testing Relates to testing. labels Apr 3, 2025

djordon moved this from In Progress to Todo in sBTC Apr 4, 2025

moodmosaic reviewed Apr 18, 2025

View reviewed changes

djordon reviewed May 7, 2025

View reviewed changes

fdefelici mentioned this pull request May 15, 2025

Integration test from snapshot stacks-network/stacks-core#5800

Open

djordon moved this from Todo to In Progress in sBTC Oct 15, 2025

matteojug mentioned this pull request Nov 7, 2025

[Feature]: Use testcontainers for parallel and isolated integration testing #1854

Open

djordon modified the milestones: sBTC: Hardening and resiliency, sBTC: Nice to have Nov 10, 2025

djordon moved this from In Progress to Todo in sBTC Nov 10, 2025

matteojug mentioned this pull request Nov 12, 2025

feat: add testcontainers compose scaffolding #1858

Closed

4 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: use `testcontainers` for integration tests#1572

feat: use `testcontainers` for integration tests#1572
cylewitruk wants to merge 16 commits intomainfrom
feat/docker-integration-tests

cylewitruk commented Apr 3, 2025 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Apr 3, 2025

Uh oh!

moodmosaic Apr 18, 2025

Uh oh!

moodmosaic Apr 18, 2025

Uh oh!

cylewitruk Apr 23, 2025

Uh oh!

moodmosaic Apr 18, 2025

Uh oh!

cylewitruk Apr 23, 2025

Uh oh!

djordon Apr 18, 2025

Uh oh!

djordon Apr 18, 2025

Uh oh!

djordon Apr 18, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

	.expect("ZMQ endpooint not accessible within allotted timeout");
	.expect("ZMQ endpoint not accessible within allotted timeout");

Conversation

cylewitruk commented Apr 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Changes

Testing Information

Checklist:

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI Apr 3, 2025

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

cylewitruk commented Apr 3, 2025 •

edited

Loading