Skip to content

chore(bqjdbc): move extended test suite to monorepo#12696

Open
Neenu1995 wants to merge 7 commits intomainfrom
ns/move-ext-test-suite
Open

chore(bqjdbc): move extended test suite to monorepo#12696
Neenu1995 wants to merge 7 commits intomainfrom
ns/move-ext-test-suite

Conversation

@Neenu1995
Copy link
Copy Markdown
Contributor

No description provided.

@Neenu1995 Neenu1995 requested review from a team as code owners April 7, 2026 14:22
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@Neenu1995 Neenu1995 changed the title chore: move extended test suit to monorepo chore: move extended test suite to monorepo Apr 7, 2026
@Neenu1995 Neenu1995 changed the title chore: move extended test suite to monorepo chore(bqjdbc): move extended test suite to monorepo Apr 7, 2026

public class ITBase extends BigQueryJdbcBaseTest {

public static final String DEFAULT_CATALOG = ServiceOptions.getDefaultProjectId();
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants