-
Notifications
You must be signed in to change notification settings - Fork 82
Migrated MariaDB, MySQL, PostgreSQL tests to use testcontainers to simplify local setup #2011
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
koperagen
wants to merge
11
commits into
master
Choose a base branch
from
testcontainers
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from 7 commits
Commits
Show all changes
11 commits
Select commit
Hold shift + click to select a range
df693b4
Migrate postresql tests to testcontainers
koperagen 13b0747
Migrate mysql tests to testcontainers
koperagen 514283e
Migrate mariadb tests to testcontainers
koperagen 50e731f
Move testcontainers tests under common package
koperagen db02bc8
Extract image versions in common file & update to latest LTS
koperagen 6ced75a
Configure task to run all testcontainer tests separately
koperagen 15b656f
Restore qualifier for JDBC types to avoid confusion with other simila…
koperagen 3ce079d
Refactor DB tests to allow both Docker and locally installed DBMS con…
koperagen 7af1db1
Move image versions to libs.versions.toml + BuildConfig
koperagen b76bcde
Fix locale problem with PgMoney type
koperagen 15f3ad4
Migrate MSSQL tests to testcontainers
koperagen File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Some comments aren't visible on the classic Files Changed page.
There are no files selected for viewing
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
10 changes: 10 additions & 0 deletions
10
dataframe-jdbc/src/test/kotlin/org/jetbrains/kotlinx/dataframe/io/testcontainers/images.kt
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,10 @@ | ||
| package org.jetbrains.kotlinx.dataframe.io.testcontainers | ||
|
|
||
| // Available image tags: https://hub.docker.com/_/mariadb/tags | ||
| const val MARIADB_IMAGE = "mariadb:12.3.2" | ||
|
|
||
| // Available image tags: https://hub.docker.com/_/mysql/tags | ||
| const val MYSQL_IMAGE = "mysql:9.7" | ||
|
|
||
| // Available image tags: https://hub.docker.com/_/postgres/tags | ||
| const val POSTGRES_IMAGE = "postgres:18-alpine" | ||
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,4 +1,4 @@ | ||
| package org.jetbrains.kotlinx.dataframe.io.local | ||
| package org.jetbrains.kotlinx.dataframe.io.testcontainers | ||
|
|
||
| import io.kotest.matchers.shouldBe | ||
| import org.intellij.lang.annotations.Language | ||
|
|
@@ -15,8 +15,8 @@ import org.jetbrains.kotlinx.dataframe.io.readSqlTable | |
| import org.jetbrains.kotlinx.dataframe.schema.DataFrameSchema | ||
| import org.junit.AfterClass | ||
| import org.junit.BeforeClass | ||
| import org.junit.Ignore | ||
| import org.junit.Test | ||
| import org.testcontainers.mariadb.MariaDBContainer | ||
| import java.math.BigDecimal | ||
| import java.math.BigInteger | ||
| import java.sql.Blob | ||
|
|
@@ -26,8 +26,9 @@ import java.sql.SQLException | |
| import java.util.Date | ||
| import kotlin.reflect.typeOf | ||
| import kotlin.time.Instant | ||
| import java.sql.Time as SqlTime | ||
| import java.sql.Timestamp as SqlTimestamp | ||
|
|
||
| private const val URL = "jdbc:mariadb://localhost:3306" | ||
| private const val USER_NAME = "root" | ||
| private const val PASSWORD = "pass" | ||
| private const val TEST_DATABASE_NAME = "testKDFdatabase" | ||
|
|
@@ -118,15 +119,23 @@ private const val JSON_STRING = | |
| " \t\"favorites\": [{\"description\": \"Pepperoni deep dish\", \"price\": 18.75}, \n" + | ||
| "{\"description\": \"The Lou\", \"price\": 24.75}]}" | ||
|
|
||
| @Ignore | ||
| class MariadbTest { | ||
| companion object { | ||
| private val mariadb: MariaDBContainer = MariaDBContainer(MARIADB_IMAGE).apply { | ||
| withUsername(USER_NAME) | ||
| withPassword(PASSWORD) | ||
| } | ||
|
|
||
| private lateinit var connection: Connection | ||
|
|
||
| private val rootUrl: String | ||
| get() = "jdbc:mariadb://${mariadb.host}:${mariadb.firstMappedPort}" | ||
|
|
||
| @BeforeClass | ||
| @JvmStatic | ||
| fun setUpClass() { | ||
| connection = DriverManager.getConnection(URL, USER_NAME, PASSWORD) | ||
| mariadb.start() | ||
| connection = DriverManager.getConnection(rootUrl, USER_NAME, PASSWORD) | ||
|
|
||
| connection.createStatement().use { st -> | ||
| // Drop the test database if it exists | ||
|
|
@@ -263,9 +272,9 @@ class MariadbTest { | |
| st.setDouble(11, i * 10.0) | ||
| st.setBigDecimal(12, BigDecimal(i * 10)) | ||
| st.setDate(13, java.sql.Date(System.currentTimeMillis())) | ||
| st.setTimestamp(14, java.sql.Timestamp(System.currentTimeMillis())) | ||
| st.setTimestamp(15, java.sql.Timestamp(System.currentTimeMillis())) | ||
| st.setTime(16, java.sql.Time(System.currentTimeMillis())) | ||
| st.setTimestamp(14, SqlTimestamp(System.currentTimeMillis())) | ||
| st.setTimestamp(15, SqlTimestamp(System.currentTimeMillis())) | ||
| st.setTime(16, SqlTime(System.currentTimeMillis())) | ||
|
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Automatic refactorings break such fully qualified links :( I recovered it as import alias, should survive file moves etc |
||
| st.setInt(17, 2023) | ||
| st.setString(18, "varcharValue$i") | ||
| st.setString(19, "charValue$i") | ||
|
|
@@ -303,9 +312,9 @@ class MariadbTest { | |
| st.setDouble(11, i * 20.0) | ||
| st.setBigDecimal(12, BigDecimal(i * 20)) | ||
| st.setDate(13, java.sql.Date(System.currentTimeMillis())) | ||
| st.setTimestamp(14, java.sql.Timestamp(System.currentTimeMillis())) | ||
| st.setTimestamp(15, java.sql.Timestamp(System.currentTimeMillis())) | ||
| st.setTime(16, java.sql.Time(System.currentTimeMillis())) | ||
| st.setTimestamp(14, SqlTimestamp(System.currentTimeMillis())) | ||
| st.setTimestamp(15, SqlTimestamp(System.currentTimeMillis())) | ||
| st.setTime(16, SqlTime(System.currentTimeMillis())) | ||
| st.setInt(17, 2023) | ||
| st.setString(18, "varcharValue$i") | ||
| st.setString(19, "charValue$i") | ||
|
|
@@ -337,6 +346,7 @@ class MariadbTest { | |
| } catch (e: SQLException) { | ||
| e.printStackTrace() | ||
| } | ||
| mariadb.stop() | ||
| } | ||
| } | ||
|
|
||
|
|
@@ -358,7 +368,7 @@ class MariadbTest { | |
| schema.columns["dateCol"]!!.type shouldBe typeOf<Date>() | ||
| schema.columns["datetimeCol"]!!.type shouldBe typeOf<Instant>() | ||
| schema.columns["timestampCol"]!!.type shouldBe typeOf<Instant>() | ||
| schema.columns["timeCol"]!!.type shouldBe typeOf<java.sql.Time>() | ||
| schema.columns["timeCol"]!!.type shouldBe typeOf<SqlTime>() | ||
| schema.columns["yearCol"]!!.type shouldBe typeOf<Date>() | ||
|
|
||
| val df2 = DataFrame.readSqlTable(connection, "table2").cast<Table2MariaDb>() | ||
|
|
@@ -493,19 +503,19 @@ class MariadbTest { | |
| @Test | ||
| fun `readAllSqlTables without catalogue should only return tables from URL database`() { | ||
| val secondDb = "testKDFdatabase2" | ||
| val testRootConn = DriverManager.getConnection(URL, USER_NAME, PASSWORD) | ||
| val testRootConn = DriverManager.getConnection(rootUrl, USER_NAME, PASSWORD) | ||
| try { | ||
| testRootConn.createStatement().use { stmt -> | ||
| stmt.executeUpdate("DROP DATABASE IF EXISTS $secondDb") | ||
| stmt.executeUpdate("CREATE DATABASE $secondDb") | ||
| } | ||
| DriverManager.getConnection("$URL/$secondDb", USER_NAME, PASSWORD).use { conn2 -> | ||
| DriverManager.getConnection("$rootUrl/$secondDb", USER_NAME, PASSWORD).use { conn2 -> | ||
| conn2.createStatement().use { stmt -> | ||
| stmt.executeUpdate("CREATE TABLE onlyInDb2 (id INT PRIMARY KEY, val VARCHAR(50))") | ||
| } | ||
| } | ||
|
|
||
| DriverManager.getConnection("$URL/$TEST_DATABASE_NAME", USER_NAME, PASSWORD).use { scopedConn -> | ||
| DriverManager.getConnection("$rootUrl/$TEST_DATABASE_NAME", USER_NAME, PASSWORD).use { scopedConn -> | ||
| val tableNames = DataFrame.readAllSqlTables(scopedConn).keys | ||
|
|
||
| tableNames.none { "onlyInDb2" in it } shouldBe true | ||
|
|
||
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
Oops, something went wrong.
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I feel like these should be configurable higher up, like in Gradle with BuildConfig. If we need to bump them, it will be difficult to find them so deeply nested
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the question here is how often we expect to update them. It's not really a dependency exposed to our users, only a test database instance we'll use to make sure our changes to JDBC module do not break anything on isolated environment. I picked all very fresh versions that are expected to be supported for 5 years.
If we're interested, i can add renovatebot with custom rule to watch over these versions right here in images.kt. That's the only common practice i could find regarding where image versions should be stored. I believe other just keep it in sync with their production database, which is not applicable to us
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't they be updated along with the library versions of the databases?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah no, they're different, it seems. Still, it's a test dependency from an external source, which makes it little different than other
testImplementationdependencies. It can be taken offline, updated, etc. so hiding it in a source file is probably not the best way to goThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think so, i'd expect a very thorough backward compatibility
https://jdbc.postgresql.org/download/
This is the current version of the driver. Unless you have unusual requirements (running old applications or JVMs), this is the driver you should be using. It supports PostgreSQL 8.4 or newer and requires Java 6 or newer. It contains support for SSL and the javax.sql package.
https://mariadb.com/docs/connectors/mariadb-connector-j/about-mariadb-connector-j
MariaDB Connector/J is compatible with all MariaDB and MySQL server versions.
MariaDB Connector/J releases older than 1.2.0 may be compatible with server versions older than MySQL 5.5, but those MariaDB Connector/J releases aren't supported anymore.
https://dev.mysql.com/doc/connector-j/en/connector-j-versions.html
MySQL Server versions: Connector/J 26.7 supports MySQL 8.0 and up. ( first released 19 April 2018)
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So we go in other direction and need to update only if we want to test new feature that is supported only in most recent database
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tbh i don't really mind moving it to buildconfid altogether. Mostly wanted to clarify my impression that updating those is not the same as updating usual dependencies, we probably shouldn't do it frequently even if it ends up in the version catalog
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I understand :) It's comparable to us sticking to an older JUnit version since it covers all we need in our tests. Even though we don't bump it often, it will be bumped at some point, so it's nice to know where the single source-of-truth regarding versions exists in the project