-
Notifications
You must be signed in to change notification settings - Fork 76
Added tests for the DataSource parameter #1562
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
Conversation
…tions and HikariDataSource integration
dataframe-jdbc/src/test/kotlin/org/jetbrains/kotlinx/dataframe/io/h2/h2Test.kt
Show resolved
Hide resolved
dataframe-jdbc/src/test/kotlin/org/jetbrains/kotlinx/dataframe/io/h2/h2Test.kt
Outdated
Show resolved
Hide resolved
dataframe-jdbc/src/test/kotlin/org/jetbrains/kotlinx/dataframe/io/h2/h2Test.kt
Outdated
Show resolved
Hide resolved
dataframe-jdbc/src/test/kotlin/org/jetbrains/kotlinx/dataframe/io/h2/h2Test.kt
Show resolved
Hide resolved
dataframe-jdbc/src/test/kotlin/org/jetbrains/kotlinx/dataframe/io/h2/h2Test.kt
Outdated
Show resolved
Hide resolved
| casted[0][0] shouldBe "John" | ||
| } | ||
|
|
||
| private fun assertCustomerSalesDataWithLimit(df: AnyFrame) { |
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.
Same, duplicating whole logic
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.
Pull Request Overview
This PR adds DataSource parameter support to all JDBC methods, refactors test code to reduce duplication, and adds connection pool stress tests using HikariCP. The changes improve code maintainability and ensure the library works correctly with connection pooling scenarios.
Key changes:
- Added HikariCP dependency and DataSource support across all JDBC read methods
- Extracted repetitive test assertions into reusable helper functions
- Added comprehensive connection pool tests to verify proper connection lifecycle management
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| gradle/libs.versions.toml | Added HikariCP version definition and library reference (upgraded from 5.1.0 to 7.0.2) |
| dataframe-jdbc/build.gradle.kts | Added HikariCP test dependency |
| dataframe-jdbc/src/test/kotlin/org/jetbrains/kotlinx/dataframe/io/h2/h2Test.kt | Refactored tests to use helper assertion functions, added DataSource initialization, and created new test cases for DataSource API and connection pooling |
Comments suppressed due to low confidence (1)
gradle/libs.versions.toml:194
- There are now two library definitions for HikariCP:
hikari(line 183) andhikaricp(line 194). Both point to the same artifact (com.zaxxer:HikariCP) with the same version reference. This creates unnecessary duplication. Consider keeping only one definition (preferablyhikaricpsince it's used inbuild.gradle.ktsand matches the artifact name better).
hikari = { group = "com.zaxxer", name = "HikariCP", version.ref = "hikari" }
duckdb-jdbc = { group = "org.duckdb", name = "duckdb_jdbc", version.ref = "duckdb" }
exposed-core = { group = "org.jetbrains.exposed", name = "exposed-core", version.ref = "exposed" }
exposed-jdbc = { group = "org.jetbrains.exposed", name = "exposed-jdbc", version.ref = "exposed" }
exposed-kotlin-datetime = { group = "org.jetbrains.exposed", name = "exposed-kotlin-datetime", version.ref = "exposed" }
exposed-json = { group = "org.jetbrains.exposed", name = "exposed-json", version.ref = "exposed" }
exposed-money = { group = "org.jetbrains.exposed", name = "exposed-money", version.ref = "exposed" }
hibernate-core = { group = "org.hibernate.orm", name = "hibernate-core", version.ref = "hibernate" }
hibernate-hikaricp = { group = "org.hibernate.orm", name = "hibernate-hikaricp", version.ref = "hibernate" }
hikaricp = { group = "com.zaxxer", name = "HikariCP", version.ref = "hikari" }
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Closes #1512