chore(jdbc): standalone jar to run driver-agnostic tests#4132
chore(jdbc): standalone jar to run driver-agnostic tests#4132
Conversation
product-auto-label
bot
added
size: xl
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request establishes the foundational infrastructure for running driver-independent tests in a standalone manner. By separating these tests into their own Maven module and providing build and run commands, it enables continuous testing against released driver versions, reducing code duplication and streamlining the testing pipeline. This initial setup serves as a proof-of-concept for future test organization. Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a new module and Makefile targets to create a standalone JAR for running driver-agnostic tests. This is a good step towards improving the testing pipeline. My review focuses on improving maintainability by removing hardcoded versions and cleaning up the new Maven POM file (pom-it.xml). I've identified several areas where versions are hardcoded, which could lead to issues down the line. I've also found a redundant dependency and inconsistencies in the POM's parent configuration.
| <parent> | ||
| <groupId>com.google.cloud</groupId> | ||
| <artifactId>google-cloud-bigquery-parent</artifactId> | ||
| <version>2.60.0</version> | ||
| <relativePath>../pom.xml</relativePath> | ||
| </parent> |
There was a problem hiding this comment.
The parent POM configuration has a couple of issues:
- The version
2.60.0is different from the parent version in the maingoogle-cloud-bigquery-jdbc/pom.xml(2.60.1-SNAPSHOT). These should be consistent to ensure dependencies are managed correctly. - The
relativePathis set to../pom.xml. The main module's POM resolves its parent from a remote repository. This inconsistency in parent resolution can cause unexpected build behavior.
It's recommended to align the parent configuration with the main pom.xml by using the same version and removing the relativePath.
<parent>
<groupId>com.google.cloud</groupId>
<artifactId>google-cloud-bigquery-parent</artifactId>
<version>2.60.1-SNAPSHOT</version>
</parent>
|
|
||
| <groupId>com.google.cloud</groupId> | ||
| <artifactId>google-cloud-bigquery-jdbc-it</artifactId> | ||
| <version>0.4.0</version> |
There was a problem hiding this comment.
The version 0.4.0 is hardcoded and inconsistent with the main module's version (0.4.1-SNAPSHOT). To ensure consistency and ease maintenance, this module should have the same version as the google-cloud-bigquery-jdbc module.
<version>0.4.1-SNAPSHOT</version>
| <dependency> | ||
| <groupId>com.google.cloud</groupId> | ||
| <artifactId>google-cloud-bigquery-jdbc</artifactId> | ||
| <version>0.4.0</version> | ||
| <type>test-jar</type> | ||
| <scope>test</scope> | ||
| </dependency> | ||
| <dependency> | ||
| <groupId>com.google.cloud</groupId> | ||
| <artifactId>google-cloud-bigquery-jdbc</artifactId> | ||
| <version>0.4.0</version> | ||
| <scope>test</scope> | ||
| </dependency> |
There was a problem hiding this comment.
There are a couple of issues with these dependencies:
- The dependency on
google-cloud-bigquery-jdbc(lines 31-36) is redundant. Thetest-jardependency is sufficient for packaging the test classes, and the main JAR is correctly excluded intest-assembly.xml. - The version
0.4.0is hardcoded. This should use${project.version}to ensure the test classes from the currently built artifact are used.
<dependency>
<groupId>com.google.cloud</groupId>
<artifactId>google-cloud-bigquery-jdbc</artifactId>
<version>${project.version}</version>
<type>test-jar</type>
<scope>test</scope>
</dependency>
| DRIVER_JAR ?= target/google-cloud-bigquery-jdbc-0.4.0.jar | ||
| STANDALONE_JAR = target-it/google-cloud-bigquery-jdbc-it-0.4.0-standalone.jar |
There was a problem hiding this comment.
The versions for DRIVER_JAR and STANDALONE_JAR are hardcoded. This will require manual updates whenever the project version changes, making it brittle. It would be more robust to determine these versions dynamically from the pom.xml files. You can use mvn help:evaluate for this, similar to how BUILD_DIR is determined elsewhere in this file.
1 participant
Separate driver-independent tests in its own module so we can run tests continuous with released versions with too much of a code-duplication.
This PR is more of a POC to setup the pipeline, we can organize tests later (proxy/nightly etc tests)