feat: add CockroachDB as a compatibility-tested database#316
Open
driessamyn wants to merge 1 commit into
Open
Conversation
Unit Tests 61 files ±0 61 suites ±0 2m 44s ⏱️ -18s Results for commit da3b233. ± Comparison against base commit c431fe4. This pull request removes 40 and adds 41 tests. Note that renamed tests count towards both.♻️ This comment has been updated with latest results. |
Code Coverage
|
📝 WalkthroughWalkthroughAdds CockroachDB support: CI matrix now runs compatibility tests for COCKROACHDB, README badge added, Testcontainers setup for CockroachDB included in integration tests, DbFlavour detects CockroachDB as PostgreSQL, converter improved for Long→Int handling, and test coverage updated. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 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 |
![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
🧹 Nitpick comments (1)
core/src/integrationTest/kotlin/net/samyn/kapper/AbstractDbTests.kt (1)
57-59: Consider pinning to a specific patch version for reproducibility.The
latest-v24.3tag is a floating reference that updates with new patch releases. Other database containers in this file use fixed versions (e.g.,postgres:16,mysql:8.4). While this ensures you get security patches automatically, it could cause non-deterministic CI failures if a new patch introduces regressions.Consider using a specific version like
cockroachdb/cockroach:v24.3.0for more predictable builds, or document the rationale for using the floating tag.🤖 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 57 - 59, The CockroachDB test container uses the floating tag "cockroachdb/cockroach:latest-v24.3" which can introduce non-deterministic CI failures; update the lazy val cockroachdb initialization (the CockroachContainer(...) call) to use a fixed patch version (e.g., "cockroachdb/cockroach:v24.3.0" or another concrete patch) for reproducible builds, or add a comment documenting why a floating tag is intentionally used.
🤖 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/main/kotlin/net/samyn/kapper/internal/Converters.kt`:
- Line 180: The pattern branch matching "is java.lang.Long -> value.toInt()"
silently truncates out-of-range Longs; replace it with a guarded conversion that
checks the Long against Int.MIN_VALUE..Int.MAX_VALUE (or the specific INT8 range
if converting for INT8) and throws an explicit exception (e.g.,
IllegalArgumentException or ArithmeticException) when out of range, so the
conversion in Converters.kt fails fast instead of truncating; locate the branch
matching "is java.lang.Long" and update the logic to validate the range before
calling toInt() and add/adjust unit tests to cover overflow cases.
---
Nitpick comments:
In `@core/src/integrationTest/kotlin/net/samyn/kapper/AbstractDbTests.kt`:
- Around line 57-59: The CockroachDB test container uses the floating tag
"cockroachdb/cockroach:latest-v24.3" which can introduce non-deterministic CI
failures; update the lazy val cockroachdb initialization (the
CockroachContainer(...) call) to use a fixed patch version (e.g.,
"cockroachdb/cockroach:v24.3.0" or another concrete patch) for reproducible
builds, or add a comment documenting why a floating tag is intentionally used.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 62711d38-4051-4e63-b3a4-1c7c999c6b80
📒 Files selected for processing (7)
.github/workflows/build-and-test.ymlREADME.mdcore/src/integrationTest/kotlin/net/samyn/kapper/AbstractDbTests.ktcore/src/main/kotlin/net/samyn/kapper/internal/Converters.ktcore/src/main/kotlin/net/samyn/kapper/internal/DbFlavourFunc.ktcore/src/test/kotlin/net/samyn/kapper/internal/DbFlavourTest.ktgradle/libs.versions.toml
driessamyn
force-pushed
the
feature/add-cockroachdb-compat
branch
from
e7cf0f5 to
da3b233
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
core/src/integrationTest/kotlin/net/samyn/kapper/AbstractDbTests.kt (1)
57-59: Consider pinning to a specific image version for reproducibility.The
latest-v24.3tag will pull the latest patch version within the v24.3 series. While this ensures security updates, it may cause non-deterministic builds if a new patch introduces behavioral changes. Other containers in this file use fully pinned versions (e.g.,postgres:16,mysql:8.4).Consider using a specific tag like
cockroachdb/cockroach:v24.3.0for more predictable CI behavior, or document the rationale for using the floating tag if this is intentional.🤖 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 57 - 59, The CockroachContainer image tag is floating ("cockroachdb/cockroach:latest-v24.3") which can make CI non-deterministic; update the image string in the cockroachdb lazy property (the cockroachdb variable in AbstractDbTests.kt) to a fully pinned version such as "cockroachdb/cockroach:v24.3.0" (or another explicit patch tag you choose) so test containers are reproducible, or alternatively add a brief comment next to the cockroachdb definition documenting why the floating tag is intentionally used.
🤖 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 57-59: The CockroachContainer image tag is floating
("cockroachdb/cockroach:latest-v24.3") which can make CI non-deterministic;
update the image string in the cockroachdb lazy property (the cockroachdb
variable in AbstractDbTests.kt) to a fully pinned version such as
"cockroachdb/cockroach:v24.3.0" (or another explicit patch tag you choose) so
test containers are reproducible, or alternatively add a brief comment next to
the cockroachdb definition documenting why the floating tag is intentionally
used.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: fa2df30f-a2eb-4c8c-a2c0-69030d1c114c
📒 Files selected for processing (7)
.github/workflows/build-and-test.ymlREADME.mdcore/src/integrationTest/kotlin/net/samyn/kapper/AbstractDbTests.ktcore/src/main/kotlin/net/samyn/kapper/internal/Converters.ktcore/src/main/kotlin/net/samyn/kapper/internal/DbFlavourFunc.ktcore/src/test/kotlin/net/samyn/kapper/internal/DbFlavourTest.ktgradle/libs.versions.toml
✅ Files skipped from review due to trivial changes (3)
- core/src/main/kotlin/net/samyn/kapper/internal/DbFlavourFunc.kt
- README.md
- core/src/main/kotlin/net/samyn/kapper/internal/Converters.kt
🚧 Files skipped from review as they are similar to previous changes (1)
- .github/workflows/build-and-test.yml
|
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.
Summary
cockroachdb/cockroach:latest-v24.3convertIntto handle Long→Int conversion (CockroachDB returns INT8 for integer columns)Test plan
-Ddb=COCKROACHDBSummary by CodeRabbit
New Features
Bug Fixes
Tests
Chores