feat: add TiDB as a compatibility-tested database#313
Conversation
📝 WalkthroughWalkthroughThis pull request adds TiDB support by extending the CI compatibility matrix, adding a Testcontainers-based TiDB integration test setup, and treating TiDB product names as MySQL in database-flavour detection. Changes
Sequence Diagram(s)sequenceDiagram
participant GH as GitHub Actions
participant Runner as CI Runner
participant Gradle as Gradle (integrationTest)
participant TC as Testcontainers
participant TiDB as TiDB Container
participant Test as Integration Tests
participant Art as Artifact Upload
GH->>Runner: matrix job (db = MARIADB, TIDB)
Runner->>Gradle: ./gradlew integrationTest -Ddb=${{ matrix.db }}
Gradle->>TC: start GenericContainer (only for TIDB case)
TC->>TiDB: pull/start pingcap/tidb:v8.5.1
Gradle->>Test: run tests using JDBC -> TiDB (mapped host/port)
Test-->>Gradle: test results
Runner->>Art: upload ${matrix.db}-compat-test-results
GH-->>Runner: report check "${matrix.db} Compatibility Tests"
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
|
Code Coverage
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@core/src/integrationTest/kotlin/net/samyn/kapper/AbstractDbTests.kt`:
- Line 62: The GenericContainer instantiation uses the string-based constructor
which can break Kotlin's SELF type inference and is deprecated; replace
GenericContainer("pingcap/tidb:v8.5.1") with the typed form
GenericContainer<GenericContainer<*>>(DockerImageName.parse("pingcap/tidb:v8.5.1"))
(or more specifically GenericContainer<DockerImageName> style per Testcontainers
examples) and add the import org.testcontainers.utility.DockerImageName so the
container creation uses DockerImageName.parse(...) for stable API typing and
correct SELF bound resolution; update the GenericContainer usage site (the
expression GenericContainer(...)) accordingly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 490e8a2f-12f7-4725-94aa-e215046ff65d
📒 Files selected for processing (5)
.github/workflows/build-and-test.ymlREADME.mdcore/src/integrationTest/kotlin/net/samyn/kapper/AbstractDbTests.ktcore/src/main/kotlin/net/samyn/kapper/internal/DbFlavourFunc.ktcore/src/test/kotlin/net/samyn/kapper/internal/DbFlavourTest.kt
| } | ||
|
|
||
| private val tidb by lazy { | ||
| GenericContainer("pingcap/tidb:v8.5.1").apply { |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
For the current Testcontainers Java API, what is the recommended Kotlin syntax to instantiate GenericContainer? Is GenericContainer("image")valid, or should it beGenericContainer("image")/KGenericContainer?
💡 Result:
For current Testcontainers Java, the “blessed” way is to construct a GenericContainer<?> from a DockerImageName, not a raw string. In Kotlin that typically becomes:
import org.testcontainers.containers.GenericContainer
import org.testcontainers.utility.DockerImageName
val redis = GenericContainer<Nothing>(DockerImageName.parse("redis:6-alpine"))
.withExposedPorts(6379)- Testcontainers’ docs show
GenericContainer<?>created vianew GenericContainer<>(DockerImageName.parse(...)). [1] - The Java API provides
GenericContainer(DockerImageName)(and still has aGenericContainer(String)overload), but Testcontainers has been deprecating the(...String)constructor pattern in favor ofDockerImageName. [2][3]
So:
GenericContainer("image"): often not what you want in Kotlin (type inference forSELF extends GenericContainer<SELF>is awkward) and it also pushes you toward a constructor style Testcontainers has been moving away from. [2][3]GenericContainer<Nothing>(...): recommended Kotlin workaround when you just need a plainGenericContainerand don’t care about fluent “self types”.KGenericContainer: optional convenience (usually a tiny Kotlin subclass people create) if you want fluent chaining to return your subtype, but it’s not required by the Java API. (It’s essentially a Kotlin ergonomics helper built on top ofGenericContainer<SELF>.) [2]
Sources:
[1] (java.testcontainers.org)
[2] (javadoc.io)
[3] (newreleases.io)
Citations:
- 1: https://java.testcontainers.org/features/creating_container/
- 2: https://javadoc.io/static/org.testcontainers/testcontainers/1.20.4/org/testcontainers/containers/GenericContainer.html
- 3: https://newreleases.io/project/github/testcontainers/testcontainers-java/release/1.15.0?utm_source=openai
🏁 Script executed:
# Find the file and check context around line 62
cd core/src/integrationTest/kotlin/net/samyn/kapper && \
cat -n AbstractDbTests.kt | sed -n '55,70p'Repository: driessamyn/kapper
Length of output: 632
🏁 Script executed:
# Check for Testcontainers dependency and version
fd -e "build.gradle" -o "pom.xml" -o "build.gradle.kts" | head -5 | xargs grep -l "testcontainers" -A 2 -B 2Repository: driessamyn/kapper
Length of output: 196
🏁 Script executed:
# Search for other GenericContainer usages in the codebase
rg "GenericContainer" --type kotlin -B 2 -A 2Repository: driessamyn/kapper
Length of output: 1188
🏁 Script executed:
# Find build files with correct syntax
fd "build.gradle" -o "pom.xml" -o "build.gradle.kts" | xargs grep -l "testcontainers" -A 3Repository: driessamyn/kapper
Length of output: 196
🏁 Script executed:
# Check if code currently compiles and check for any reported issues
git log --oneline -20 | head -10Repository: driessamyn/kapper
Length of output: 118
🏁 Script executed:
# Look at the full build.gradle or pom.xml to find Testcontainers version
fd "build.gradle" --type f | head -3 | xargs cat | grep -A 5 -B 5 "testcontainers"Repository: driessamyn/kapper
Length of output: 43
🏁 Script executed:
# List all build files in the repo
find . -maxdepth 2 -name "build.gradle*" -o -name "pom.xml" | head -10Repository: driessamyn/kapper
Length of output: 190
🏁 Script executed:
# Search for testcontainers in all gradle/maven files
rg "testcontainers" -t gradle -t xml -A 2Repository: driessamyn/kapper
Length of output: 209
🏁 Script executed:
# Check the core/build.gradle.kts file since that's where the test is
cat ./core/build.gradle.kts | grep -A 5 -B 5 "testcontainers"Repository: driessamyn/kapper
Length of output: 43
🏁 Script executed:
# Also check the main build.gradle.kts
cat ./build.gradle.kts | grep -A 5 -B 5 "testcontainers"Repository: driessamyn/kapper
Length of output: 43
🏁 Script executed:
# Try searching without grep first to see the full content
cat ./core/build.gradle.ktsRepository: driessamyn/kapper
Length of output: 718
🏁 Script executed:
# Also check if testcontainers is in root build.gradle.kts
cat ./build.gradle.ktsRepository: driessamyn/kapper
Length of output: 1938
🏁 Script executed:
# Find the version catalog (usually in gradle/libs.versions.toml or similar)
find . -name "*.versions.toml" -o -name "libs.versions.toml" | head -5Repository: driessamyn/kapper
Length of output: 88
🏁 Script executed:
# Look for gradle folder with version catalog
ls -la gradle/ 2>/dev/null || echo "No gradle folder"Repository: driessamyn/kapper
Length of output: 300
🏁 Script executed:
# Check the testcontainers version in the libs.versions.toml
cat ./gradle/libs.versions.toml | grep -A 10 -B 2 "testcontainers"Repository: driessamyn/kapper
Length of output: 1694
🏁 Script executed:
# Find the version reference for testcontainers
cat ./gradle/libs.versions.toml | grep "test-containers"Repository: driessamyn/kapper
Length of output: 1100
🏁 Script executed:
# Check if there are any other GenericContainer usages or similar patterns
rg "GenericContainer\|MariaDBContainer\|MSSQLServerContainer" -A 2Repository: driessamyn/kapper
Length of output: 43
🏁 Script executed:
# Look for any DockerImageName usage in the codebase
rg "DockerImageName" -A 2Repository: driessamyn/kapper
Length of output: 43
🏁 Script executed:
# Check the git history for the TiDB addition to see if there were any issues
git log --oneline --all -- "core/src/integrationTest/kotlin/net/samyn/kapper/AbstractDbTests.kt" | head -5Repository: driessamyn/kapper
Length of output: 345
🏁 Script executed:
# Check the imports in AbstractDbTests.kt to see what's currently being used
head -40 core/src/integrationTest/kotlin/net/samyn/kapper/AbstractDbTests.ktRepository: driessamyn/kapper
Length of output: 1526
🏁 Script executed:
# Try to understand the GenericContainer API in 1.21.4
# Search for any comments or documentation about the string constructor
rg "withExposedPorts\|start\(\)" core/src/integrationTest/kotlin/net/samyn/kapper/AbstractDbTests.kt -B 3 -A 1Repository: driessamyn/kapper
Length of output: 43
🏁 Script executed:
# Check if the code compiles by looking at recent commits and build status
git show 392196a --stat | head -20Repository: driessamyn/kapper
Length of output: 1126
🏁 Script executed:
# Check the exact syntax being used in similar containers
cat core/src/integrationTest/kotlin/net/samyn/kapper/AbstractDbTests.kt | sed -n '57,75p'Repository: driessamyn/kapper
Length of output: 777
Use explicit type argument to improve Kotlin type inference with GenericContainer.
The string-based constructor form GenericContainer("image") can encounter type inference issues in Kotlin when resolving the SELF bound of GenericContainer<SELF>. Additionally, Testcontainers (v1.21.4 in use here) has moved away from the string-based constructor in favor of DockerImageName.parse() for better API stability.
Prefer explicit type parameters combined with the recommended DockerImageName pattern:
Proposed fix
- private val tidb by lazy {
- GenericContainer("pingcap/tidb:v8.5.1").apply {
+ private val tidb by lazy {
+ GenericContainer<Nothing>(DockerImageName.parse("pingcap/tidb:v8.5.1")).apply {
withExposedPorts(4000)
start()
}
}Add import: import org.testcontainers.utility.DockerImageName
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@core/src/integrationTest/kotlin/net/samyn/kapper/AbstractDbTests.kt` at line
62, The GenericContainer instantiation uses the string-based constructor which
can break Kotlin's SELF type inference and is deprecated; replace
GenericContainer("pingcap/tidb:v8.5.1") with the typed form
GenericContainer<GenericContainer<*>>(DockerImageName.parse("pingcap/tidb:v8.5.1"))
(or more specifically GenericContainer<DockerImageName> style per Testcontainers
examples) and add the import org.testcontainers.utility.DockerImageName so the
container creation uses DockerImageName.parse(...) for stable API typing and
correct SELF bound resolution; update the GenericContainer usage site (the
expression GenericContainer(...)) accordingly.
driessamyn
force-pushed
the
feature/add-tidb-compat
branch
from
392196a to
2caedbc
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
core/src/integrationTest/kotlin/net/samyn/kapper/AbstractDbTests.kt (1)
62-67: Extract TiDB literals to constants to avoid config drift.Line 64 and Line 100 repeat the TiDB port literal (
4000), and the image/database strings are embedded inline. Pulling these into companion-object constants makes future updates safer and clearer.Proposed refactor
companion object { + private const val TIDB_IMAGE = "pingcap/tidb:v8.5.1" + private const val TIDB_PORT = 4000 + private const val TIDB_DATABASE = "test" + init { Class.forName("org.sqlite.JDBC") Class.forName("org.duckdb.DuckDBDriver") } @@ private val tidb by lazy { - GenericContainer<Nothing>(DockerImageName.parse("pingcap/tidb:v8.5.1")).apply { - withExposedPorts(4000) + GenericContainer<Nothing>(DockerImageName.parse(TIDB_IMAGE)).apply { + withExposedPorts(TIDB_PORT) start() } } @@ "TIDB" to { DriverManager.getConnection( - "jdbc:mysql://${tidb.host}:${tidb.getMappedPort(4000)}/test?user=root&allowPublicKeyRetrieval=true&useSSL=false", + "jdbc:mysql://${tidb.host}:${tidb.getMappedPort(TIDB_PORT)}/$TIDB_DATABASE?user=root&allowPublicKeyRetrieval=true&useSSL=false", ) }, )Also applies to: 98-102
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@core/src/integrationTest/kotlin/net/samyn/kapper/AbstractDbTests.kt` around lines 62 - 67, The TiDB container configuration in the lazy val tidb and its usages hardcodes literals (image tag "pingcap/tidb:v8.5.1", port 4000 and any inline DB name strings); extract these into companion-object constants (e.g., TIDB_IMAGE, TIDB_PORT, TIDB_DB_NAME) and replace occurrences in GenericContainer<Nothing>(DockerImageName.parse(...)), withExposedPorts(4000) and any other inline references so updates are centralized; update the lazy val tidb and any other places (around the second occurrence at lines ~98–102) to reference those constants.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@core/src/integrationTest/kotlin/net/samyn/kapper/AbstractDbTests.kt`:
- Around line 62-67: The TiDB container configuration in the lazy val tidb and
its usages hardcodes literals (image tag "pingcap/tidb:v8.5.1", port 4000 and
any inline DB name strings); extract these into companion-object constants
(e.g., TIDB_IMAGE, TIDB_PORT, TIDB_DB_NAME) and replace occurrences in
GenericContainer<Nothing>(DockerImageName.parse(...)), withExposedPorts(4000)
and any other inline references so updates are centralized; update the lazy val
tidb and any other places (around the second occurrence at lines ~98–102) to
reference those constants.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 36c40a76-c011-42bb-99d2-0a27ba888916
📒 Files selected for processing (5)
.github/workflows/build-and-test.ymlREADME.mdcore/src/integrationTest/kotlin/net/samyn/kapper/AbstractDbTests.ktcore/src/main/kotlin/net/samyn/kapper/internal/DbFlavourFunc.ktcore/src/test/kotlin/net/samyn/kapper/internal/DbFlavourTest.kt
✅ Files skipped from review due to trivial changes (3)
- README.md
- core/src/main/kotlin/net/samyn/kapper/internal/DbFlavourFunc.kt
- core/src/test/kotlin/net/samyn/kapper/internal/DbFlavourTest.kt
🚧 Files skipped from review as they are similar to previous changes (1)
- .github/workflows/build-and-test.yml
1 participant
Summary
pingcap/tidb:v8.5.1Test plan
-Ddb=TIDBSummary by CodeRabbit
New Features
Tests
Documentation
Chores