chore(bqjdbc): move extended test suite to monorepo#12696
chore(bqjdbc): move extended test suite to monorepo#12696
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces a comprehensive suite of integration tests for the BigQuery JDBC driver, covering core components such as Connection, Statement, PreparedStatement, CallableStatement, DatabaseMetaData, ResultSetMetaData, and Driver functionality, as well as connection pooling. It also establishes a base class, ITBase, containing utility methods for environment setup and metadata verification. The review feedback identifies several instances where JDBC resources like Connection, Statement, and ResultSet are not properly closed, potentially leading to resource leaks. It is recommended to refactor these sections to use try-with-resources blocks to ensure exception-safe resource management.
...query/google-cloud-bigquery-jdbc/src/test/java/com/google/cloud/bigquery/jdbc/it/ITBase.java
Show resolved
Hide resolved
...d-bigquery-jdbc/src/test/java/com/google/cloud/bigquery/jdbc/it/ITConnectionPoolingTest.java
Show resolved
Hide resolved
...google-cloud-bigquery-jdbc/src/test/java/com/google/cloud/bigquery/jdbc/it/ITDriverTest.java
Show resolved
Hide resolved
|
|
||
| public class ITBase extends BigQueryJdbcBaseTest { | ||
|
|
||
| public static final String DEFAULT_CATALOG = ServiceOptions.getDefaultProjectId(); |
There was a problem hiding this comment.
I'd suggest to keep only functions that can be re-used by different classes of tests in ITBase class.
E.g. connectionUrl, DEFAULT_CATALOG 100% makes sense to keep there, but various verify methods that are very specific to a specific test (e.g. validate metadata) I'd suggest to keep with tests that leverage them.
e.g. verifyAllBooleanMethods is very specific (and it is just a single test of its own from what I understand) and I'd prefer to keep it separate
No description provided.