Conversation
✅ Deploy Preview for testcontainers-go ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
Summary by CodeRabbit
WalkthroughAdds a new TiDB module to testcontainers-go: implementation, examples, integration tests, documentation, Makefile fragment, and Dependabot config to include the module in gomod updates. Changes
Sequence DiagramsequenceDiagram
participant User as User
participant Run as Run()
participant Container as TiDB Container
participant Wait as HTTP Wait Strategy
participant MySQL as MySQL Driver
participant DB as TiDB
User->>Run: Run(ctx, img, opts...)
Run->>Container: Create container (expose ports, env)
Run->>Wait: Configure HTTP readiness (port 10080)
Wait->>Container: Poll /health (HTTP)
Wait-->>Run: Ready
Run-->>User: *Container
User->>Container: ConnectionString(ctx)
Container-->>User: DSN
User->>MySQL: Open(DSN) / Ping
MySQL->>DB: Connect & Query
DB-->>MySQL: Result
MySQL-->>User: Query result
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
modules/tidb/tidb_test.go (1)
62-70: Avoid duplicating theprofileschema in two places.
modules/tidb/testdata/schema.sqlalready defines and seedsprofile, so keeping the same DDL/DML inline here creates an easy drift point. Either load the fixture in this test or drop the unused file.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@modules/tidb/tidb_test.go` around lines 62 - 70, The test in tidb_test.go duplicates the profile DDL/DML via direct db.Exec calls for "CREATE TABLE ... profile" and "INSERT INTO profile", which conflicts with modules/tidb/testdata/schema.sql; replace the inline db.Exec calls by loading and executing the SQL fixture from modules/tidb/testdata/schema.sql (read the file contents and run db.Exec or a helper like loadTestFixture) or remove the unused schema.sql file — update the code paths referencing db.Exec in tidb_test.go and the "profile" setup so the schema is defined only once.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/modules/tidb.md`:
- Around line 13-15: The fenced code block containing the command "go get
github.com/testcontainers/testcontainers-go/modules/tidb" in
docs/modules/tidb.md lacks a language identifier; update the block opener to
include a language (e.g., "```bash") so the snippet is rendered with proper
syntax highlighting while keeping the command text unchanged.
In `@modules/tidb/tidb.go`:
- Around line 57-61: The current readiness only waits for the HTTP endpoint
(wait.ForHTTP("/status").WithPort(restAPIPort)) so Run() can return before the
SQL port (defaultPort) is accepting connections; update the container options in
moduleOpts to combine checks using wait.ForAll(...) so both
wait.ForListeningPort(defaultPort) and
wait.ForHTTP("/status").WithPort(restAPIPort) are awaited (replace the single
wait.ForHTTP call with wait.ForAll(wait.ForListeningPort(defaultPort),
wait.ForHTTP("/status").WithPort(restAPIPort))). Ensure you keep
testcontainers.WithExposedPorts(defaultPort, restAPIPort) and pass the composed
wait strategy to testcontainers.WithWaitStrategy.
---
Nitpick comments:
In `@modules/tidb/tidb_test.go`:
- Around line 62-70: The test in tidb_test.go duplicates the profile DDL/DML via
direct db.Exec calls for "CREATE TABLE ... profile" and "INSERT INTO profile",
which conflicts with modules/tidb/testdata/schema.sql; replace the inline
db.Exec calls by loading and executing the SQL fixture from
modules/tidb/testdata/schema.sql (read the file contents and run db.Exec or a
helper like loadTestFixture) or remove the unused schema.sql file — update the
code paths referencing db.Exec in tidb_test.go and the "profile" setup so the
schema is defined only once.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 4ac04d0c-4e24-47ca-8c63-ed56e47ece18
⛔ Files ignored due to path filters (1)
modules/tidb/go.sumis excluded by!**/*.sum
📒 Files selected for processing (9)
.github/dependabot.ymldocs/modules/tidb.mdmkdocs.ymlmodules/tidb/Makefilemodules/tidb/examples_test.gomodules/tidb/go.modmodules/tidb/testdata/schema.sqlmodules/tidb/tidb.gomodules/tidb/tidb_test.go
mdelapenya
left a comment
There was a problem hiding this comment.
Thanks for the contribution! A few changes are needed to align with the project conventions. Please check the inline comments.
mdelapenya
left a comment
There was a problem hiding this comment.
Thanks for the contribution! A few changes are needed to align with the project conventions. Please check the inline comments.
Add a new Testcontainers module for TiDB, a MySQL-compatible distributed SQL database. - Container struct with Run(), ConnectionString(), MustConnectionString() - Ports 4000/tcp (MySQL protocol) and 10080/tcp (REST API) - wait.ForAll with ForListeningPort and ForHTTP(/status) readiness strategy - Tests and testable examples - Documentation and config updates (mkdocs, dependabot) - Go 1.25 / toolchain go1.25.7 Closes testcontainers#3543
72dc05e to
48ee3c5
Compare
|
hi @mdelapenya this has been updated according to the changes requested |
There was a problem hiding this comment.
♻️ Duplicate comments (1)
docs/modules/tidb.md (1)
13-15:⚠️ Potential issue | 🟡 MinorAdd a language to the install snippet.
This fence is still untyped, so it keeps failing MD040 and loses syntax highlighting.
📝 Proposed fix
-``` +```bash go get github.com/testcontainers/testcontainers-go/modules/tidb</details> <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against the current code and only fix it if needed.
In
@docs/modules/tidb.mdaround lines 13 - 15, The code fence in the docs
example is untyped causing MD040; update the fenced block in tidb.md by adding
the language identifier (bash) to the opening fence so it reads ```bash and
preserves syntax highlighting for the go get snippet.</details> </blockquote></details> </blockquote></details> <details> <summary>🧹 Nitpick comments (1)</summary><blockquote> <details> <summary>modules/tidb/tidb.go (1)</summary><blockquote> `37-40`: **Match the MySQL module's query-arg assembly.** Using `len(args) > 0` here means `ConnectionString(ctx, "")` emits a dangling `?`. Joining first and only prefixing when the result is non-empty keeps TiDB aligned with `modules/mysql/mysql.go`. <details> <summary>🔧 Proposed refactor</summary> ```diff extraArgs := "" if len(args) > 0 { - extraArgs = "?" + strings.Join(args, "&") + extraArgs = strings.Join(args, "&") + } + if extraArgs != "" { + extraArgs = "?" + extraArgs } ``` </details> <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against the current code and only fix it if needed. In `@modules/tidb/tidb.go` around lines 37 - 40, The code assembling query args in modules/tidb (extraArgs := "" / if len(args) > 0 ...) can emit a dangling "?"; change the logic to first join args (e.g., joined := strings.Join(args, "&")) and only prefix with "?" when joined is non-empty so ConnectionString (or the function building the DSN in tidb.go) matches the behavior in modules/mysql/mysql.go; update the code that constructs extraArgs in the same function to use this check. ``` </details> </blockquote></details> </blockquote></details> <details> <summary>🤖 Prompt for all review comments with AI agents</summary>Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In@docs/modules/tidb.md:
- Around line 13-15: The code fence in the docs example is untyped causing
MD040; update the fenced block in tidb.md by adding the language identifier
(bash) to the opening fence so it reads ```bash and preserves syntax
highlighting for the go get snippet.
Nitpick comments:
In@modules/tidb/tidb.go:
- Around line 37-40: The code assembling query args in modules/tidb (extraArgs
:= "" / if len(args) > 0 ...) can emit a dangling "?"; change the logic to first
join args (e.g., joined := strings.Join(args, "&")) and only prefix with "?"
when joined is non-empty so ConnectionString (or the function building the DSN
in tidb.go) matches the behavior in modules/mysql/mysql.go; update the code that
constructs extraArgs in the same function to use this check.</details> --- <details> <summary>ℹ️ Review info</summary> <details> <summary>⚙️ Run configuration</summary> **Configuration used**: Organization UI **Review profile**: CHILL **Plan**: Pro **Run ID**: `8eff5284-5141-4012-abdc-eda4cf0bfbb8` </details> <details> <summary>📥 Commits</summary> Reviewing files that changed from the base of the PR and between 72dc05e81cb0c7ffe7a0ebcb821699b6895dcf51 and 48ee3c5620befd61f63a06626199d9bec15c4fe7. </details> <details> <summary>⛔ Files ignored due to path filters (1)</summary> * `modules/tidb/go.sum` is excluded by `!**/*.sum` </details> <details> <summary>📒 Files selected for processing (8)</summary> * `.github/dependabot.yml` * `docs/modules/tidb.md` * `mkdocs.yml` * `modules/tidb/Makefile` * `modules/tidb/examples_test.go` * `modules/tidb/go.mod` * `modules/tidb/tidb.go` * `modules/tidb/tidb_test.go` </details> <details> <summary>🚧 Files skipped from review as they are similar to previous changes (2)</summary> * mkdocs.yml * .github/dependabot.yml </details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
mdelapenya
left a comment
There was a problem hiding this comment.
LGTM, thanks for adding this module
…m-v2 * upstream/main: (269 commits) chore(deps): bump actions/checkout from 6.0.1 to 6.0.2 (#3560) chore(deps): bump go.opentelemetry.io/otel/sdk to v1.41.0 (#3589) feat: add TiDB module (#3575) feat: add Forgejo module (#3556) feat: improve container conflict detection (#3574) chore(deps): bump go to 1.25 everywhere (#3572) chore(pulsar): bump base image to 4.x, replacing the wait for log strategy with wait for listening port (deterministic) (#3573) chore(deps): bump github.com/sigstore/sigstore in /modules/compose (#3571) chore(compose): update to compose-v5 (#3568) chore(deps): bump github.com/modelcontextprotocol/go-sdk (#3557) chore(deps): bump mkdocs-codeinclude-plugin from 0.2.1 to 0.3.1 (#3561) chore: update usage metrics (2026-03-02) (#3565) chore(deps): bump mkdocs-include-markdown-plugin from 7.2.0 to 7.2.1 (#3562) chore(deps): bump go.opentelemetry.io/otel/sdk in /modules/grafana-lgtm (#3563) chore(deps): bump go.opentelemetry.io/otel/sdk in /modules/toxiproxy (#3564) feat(azure): add lowkey vault container (#3542) feat(chroma): update to chroma 1.x (#3552) chore(deps): bump mkdocs-include-markdown-plugin from 7.2.0 to 7.2.1 (#3547) chore(deps): bump tj-actions/changed-files from 47.0.0 to 47.0.1 (#3546) chore(deps): bump actions/upload-artifact from 4.6.2 to 6.0.0 (#3545) ...
…archive-temp * upstream/main: chore(deps): bump actions/checkout from 6.0.1 to 6.0.2 (#3560) chore(deps): bump go.opentelemetry.io/otel/sdk to v1.41.0 (#3589) feat: add TiDB module (#3575) feat: add Forgejo module (#3556) feat: improve container conflict detection (#3574) chore(deps): bump go to 1.25 everywhere (#3572) chore(pulsar): bump base image to 4.x, replacing the wait for log strategy with wait for listening port (deterministic) (#3573) chore(deps): bump github.com/sigstore/sigstore in /modules/compose (#3571) chore(compose): update to compose-v5 (#3568) chore(deps): bump github.com/modelcontextprotocol/go-sdk (#3557) chore(deps): bump mkdocs-codeinclude-plugin from 0.2.1 to 0.3.1 (#3561) chore: update usage metrics (2026-03-02) (#3565) chore(deps): bump mkdocs-include-markdown-plugin from 7.2.0 to 7.2.1 (#3562) chore(deps): bump go.opentelemetry.io/otel/sdk in /modules/grafana-lgtm (#3563) chore(deps): bump go.opentelemetry.io/otel/sdk in /modules/toxiproxy (#3564)
…lugin * upstream/main: chore(deps): bump golang.org/x/mod in /modules/localstack (#3587) chore(deps): bump golang.org/x/mod in /modules/elasticsearch (#3585) chore(deps): bump golang.org/x/mod in /modules/redpanda (#3588) chore(deps): bump golang.org/x/mod in /modules/kafka (#3586) chore(deps): bump github.com/shirou/gopsutil/v4 from 4.25.12 to 4.26.2 (#3576) chore(deps): bump github.com/moby/go-archive from 0.1.0 to 0.2.0 (#3548) chore(deps): bump github.com/moby/term from 0.5.0 to 0.5.2 (#3081) chore(deps): bump actions/checkout from 6.0.1 to 6.0.2 (#3560) chore(deps): bump go.opentelemetry.io/otel/sdk to v1.41.0 (#3589) feat: add TiDB module (#3575)
…-action * upstream/main: (22 commits) chore(deps): bump golang.org/x/mod in /modules/localstack (#3587) chore(deps): bump golang.org/x/mod in /modules/elasticsearch (#3585) chore(deps): bump golang.org/x/mod in /modules/redpanda (#3588) chore(deps): bump golang.org/x/mod in /modules/kafka (#3586) chore(deps): bump github.com/shirou/gopsutil/v4 from 4.25.12 to 4.26.2 (#3576) chore(deps): bump github.com/moby/go-archive from 0.1.0 to 0.2.0 (#3548) chore(deps): bump github.com/moby/term from 0.5.0 to 0.5.2 (#3081) chore(deps): bump actions/checkout from 6.0.1 to 6.0.2 (#3560) chore(deps): bump go.opentelemetry.io/otel/sdk to v1.41.0 (#3589) feat: add TiDB module (#3575) feat: add Forgejo module (#3556) feat: improve container conflict detection (#3574) chore(deps): bump go to 1.25 everywhere (#3572) chore(pulsar): bump base image to 4.x, replacing the wait for log strategy with wait for listening port (deterministic) (#3573) chore(deps): bump github.com/sigstore/sigstore in /modules/compose (#3571) chore(compose): update to compose-v5 (#3568) chore(deps): bump github.com/modelcontextprotocol/go-sdk (#3557) chore(deps): bump mkdocs-codeinclude-plugin from 0.2.1 to 0.3.1 (#3561) chore: update usage metrics (2026-03-02) (#3565) chore(deps): bump mkdocs-include-markdown-plugin from 7.2.0 to 7.2.1 (#3562) ...
Add a new Testcontainers module for TiDB, a MySQL-compatible distributed SQL database.
What does this PR do?
Adds a new
tidbmodule undermodules/tidb/, modeled after the Java TiDB module. The module provides:testcontainers.Containerpingcap/tidbimage with exposed ports4000/tcp(MySQL protocol) and10080/tcp(REST API)/statusendpoint (port 10080), matching the Java implementationgo-sql-driver/mysql) is only a test dependencyWhy is it important?
TiDB is a popular MySQL-compatible distributed SQL database with no existing Go module in testcontainers-go, while it already exists for Java. This module allows Go developers to use TiDB out of the box without needing to configure generic containers manually.
Related issues
How to test this PR