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
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 against isolated Iggy, ZooKeeper, and Pinot containers. Gradle derives the Pinot image from the version catalog, always pulls the VSR-capable Iggy image, and builds a shaded plugin with relocated Netty; 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-tasksAI Usage
Codex was used to understand the existing repo and codebase.