Draft
Conversation
Review of PR #4154: Replace ory/dockertest with testcontainers-goCommit message validation✅ Single commit: Code reviewScope: ~8500 lines across 50+ integration test files, migrating container management from No high-signal issues found. This is a clean mechanical migration with consistent patterns throughout:
The migration is consistent and follows testcontainers-go idioms correctly throughout. |
mmatczuk
force-pushed
the
mmt/testcontainers_v2
branch
5 times, most recently
from
46598ba to
f929b0a
Compare
Replace outdated GenericContainer/Terminate examples with testcontainers.Run functional options API and CleanupContainer. Add reference for all option functions, wait strategies, and readiness retry patterns.
- Fix TestRedpandaConnectionTestPrematureConnectIntegration: align topic
names to use "topic-{id}" prefix matching createKafkaTopic
- Fix TestRedpandaRecordOrderIntegration: use dedicated context instead
of t.Context() to avoid premature cancellation, increase StopWithin to 5s
- Serialize topic creation with semaphore and add retry logic for
INVALID_PARTITIONS errors under concurrent Redpanda load
- Skip deprecated flaky Sarama balanced subtest
…imeout - Switch TestIntegrationMigratorSoak to CheckSkipExact so it only runs when explicitly targeted with -run, preventing it from consuming the 600s test budget during normal CI runs - Use longer timeout for the first message in startMigratorAndWaitForMessages to account for consumer group join/balance protocol latency with testcontainers
SQL Server 2025 CU1 fixed AVX instruction issues under Rosetta 2, making it the best choice for ARM64 development. Hardcode the image to 2025-latest and remove the version parameter from setup helpers since all callers used the same version.
With testcontainers the input now binds and the output dials (reversed from the dockertest setup) because the mapped port is only reachable from the host side. Set VAR3 to empty for PULL-mode subtests to avoid an undefined variable error in the template.
mmatczuk
force-pushed
the
mmt/testcontainers_v2
branch
from
f929b0a to
33d08c6
Compare
Switch from container-registry.oracle.com/database/express (x86-only, needed Rosetta) to database/free:23.6.1.0-lite which runs natively on ARM64. Drop the version parameter and ImagePlatform override since the Free image is multi-arch. Update service name from XE to FREE in connection strings and bench tooling.
Replace bind mounts with testcontainers WithFiles to avoid macOS temp directory inaccessibility in Docker (colima). Add an entrypoint wrapper to chown the copied cert files to the postgres user before startup, since WithFiles copies as root and postgres rejects keys it cannot read.
…d port announcements
- redis: add darwin skip for TestIntegrationRedisFailoverCache since sentinel resolves docker gateway IP that is unreachable from macOS/colima host; use host.docker.internal for sentinel-to-master connection on Linux - pulsar: use WithWaitStrategyAndDeadline + WithStartupTimeout both at 3min to properly override testcontainers' 60s default deadline
Replace port-listening wait strategy with ForHTTP on the HEC health endpoint, using WithWaitStrategyAndDeadline to override testcontainers' internal 60s deadline that was too short for Rosetta emulation. This also removes the redundant require.Eventually HEC health poll since the wait strategy now covers it. Pin image to 9.3.3 (10.x breaks under Rosetta due to MongoDB 7 AVX requirements).
Add cmd/tools/integration, a CLI tool that runs integration tests one package at a time with pause/resume support, subtest compaction, timeout detection, per-package timeouts, and structured index file output.
mmatczuk
force-pushed
the
mmt/testcontainers_v2
branch
from
33d08c6 to
e136501
Compare
…S race Concurrent DFS walkers could exhaust a pre-computed refcount on shared dependency subjects, causing one walker to restore IMPORT mode while another was still syncing. Fix by deferring all mode restoration to Close() and removing the refcount mechanism entirely. Close() now restores modes in parallel (bounded by MaxParallelHTTPRequests) with up to 3 retry attempts.
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
No description provided.