test(java): add Pinot connector E2E coverage - #3922
Conversation
|
Thanks for the PR. It is labeled Slash commands (own line, regular comment) move it around the queue:
See CONTRIBUTING.md for details. |
| class IggyPinotIntegrationTest { | ||
|
|
||
| // The Java SDK speaks VSR, so use the same VSR-capable image as its integration tests. | ||
| private static final DockerImageName IGGY_IMAGE = DockerImageName.parse("apache/iggy:edge"); |
There was a problem hiding this comment.
Could we add a USE_EXTERNAL_SERVER switch here, mirroring BaseIntegrationTest? In CI we want this suite to run against the iggy-server built from the current branch (the same way the SDK tests do), so the E2E actually validates the code under review. The apache/iggy:edge container should stay only as the developer-convenience path for local runs.
One thing to solve for the external mode: the Pinot containers resolve the server via the iggy network alias and deployment/table.json hardcodes stream.iggy.host: "iggy" / port 8090. With an external server on the host, the containers need a route to it (e.g. host.docker.internal / host-gateway extra host mapped to the iggy alias, or templating the host/port into the table config before POSTing it).
| import static org.assertj.core.api.Assertions.assertThat; | ||
| import static org.assertj.core.api.Assertions.fail; | ||
|
|
||
| @Testcontainers |
There was a problem hiding this comment.
@Testcontainers is decorative here: there are no @Container fields, so the extension manages nothing - lifecycle is entirely manual in @BeforeAll/@AfterAll. Either remove the annotation or add @Container annotations to let the extension manage the containers. Note that the @Container route will be harder to keep once the external-server env switch is introduced, since some containers would then start conditionally, which doesn't fit the annotation-driven lifecycle well.
| lastResponse = "HTTP " + response.statusCode() + ": " + response.body(); | ||
| if (response.statusCode() >= 200 && response.statusCode() < 300) { | ||
| JsonNode json = OBJECT_MAPPER.readTree(response.body()); | ||
| if (hasExceptionCode(json, 150)) { |
There was a problem hiding this comment.
Nit: 150 is Pinot's SQL parsing error code. pinot-spi is already on the test classpath, so QueryErrorCode.SQL_PARSING.getId() (or a named constant) would document the intent.
| finalizedBy("assemblePlugin") | ||
| } | ||
|
|
||
| tasks.named<Test>("test") { |
There was a problem hiding this comment.
The test task doesn't declare deployment/ as an input, so editing schema.json / table.json and rerunning test can report UP-TO-DATE with stale results. Suggest adding:
inputs.dir(layout.projectDirectory.dir("deployment"))| from(configurations.runtimeClasspath) | ||
| tasks.shadowJar { | ||
| duplicatesStrategy = DuplicatesStrategy.EXCLUDE | ||
| filesMatching("META-INF/services/**") { |
There was a problem hiding this comment.
Nit: the filesMatching("META-INF/services/**") { duplicatesStrategy = INCLUDE } block is dead config - mergeServiceFiles() installs a transformer that takes service descriptors out of the normal copy path entirely, so the top-level EXCLUDE never sees them.
|
@goutamadwant Thanks for the contribution! This is a solid replacement for the shell workflow.
|
github-actions
Bot
added
S-waiting-on-author
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #3922 +/- ##
============================================
- Coverage 83.67% 83.67% -0.01%
Complexity 1358 1358
============================================
Files 1212 1212
Lines 165138 166191 +1053
Branches 132612 133790 +1178
============================================
+ Hits 138181 139059 +878
- Misses 23308 23354 +46
- Partials 3649 3778 +129
🚀 New features to boost your workflow:
|
2 participants
Which issue does this PR address?
Closes #2598
Rationale
The Pinot connector needs repeatable, Gradle-integrated end-to-end coverage for its documented ingestion workflow.
What changed?
Before, Pinot connector E2E coverage lived in a manual shell script with fixed ports, fixed sleeps, and a mutable Pinot image. The workflow was not part of Gradle and could report table creation success even when Pinot rejected the connector.
The shell workflow is now a JUnit and Testcontainers suite that verifies JSON field mapping and 10-message batch ingestion. Local runs use an always-pulled VSR-capable Iggy image, while CI uses the branch-built external server through Testcontainers host-port forwarding and an in-memory Pinot table configuration.
Gradle tracks the deployment configuration and external-server settings as test inputs, and Java CI publishes the Pinot XML results in its test summary. The shaded plugin keeps its relocated Netty dependency, while bounded polling and service-log diagnostics make failures actionable.
Local Execution
./gradlew :iggy-connector-pinot:check --no-daemon --no-build-cache --rerun-tasks./gradlew check -x test --no-daemon --no-build-cache --rerun-tasksUSE_EXTERNAL_SERVER=true ./gradlew test --no-daemon --no-build-cache --rerun-tasksAI Usage
Codex was used to understand the existing repo and codebase.