You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pulsar.apache.org by "david-streamlio (via GitHub)" <gi...@apache.org> on 2026/07/10 15:47:42 UTC

[PR] [fix][test] Bound MqttSinkIntegrationTest container startup so it cannot hang CI [pulsar-connectors]

david-streamlio opened a new pull request, #91:
URL: https://github.com/apache/pulsar-connectors/pull/91

   Fixes #89
   
   ### Motivation
   
   The `Tests - Connectors` job on #86 ran **45m16s** and was **cancelled** by the runner's `timeout-minutes: 45` — not failed. No diagnostics, 45 minutes of runner time gone, and a red PR with nothing actually wrong with it. [Run 29100595779](https://github.com/apache/pulsar-connectors/actions/runs/29100595779).
   
   `:mqtt:integrationTest` started and never emitted another line; everything else in the job had already passed. The test is careful *inside* the method — every wait is bounded — but container startup in `@BeforeClass` was not:
   
   ```java
   private final GenericContainer<?> mqttContainer = new GenericContainer<>(MOSQUITTO_IMAGE)
           .withExposedPorts(MQTT_PORT);          // no waitingFor(), no withStartupTimeout()
   ```
   
   A stalled image pull there hangs with no bound, and neither TestNG (no `@Test(timeOut)`) nor Gradle can interrupt it, so the 45-minute job limit is the only backstop. This test only started running in CI when #74 added `integrationTest` to the matrix, so the hazard was newly live.
   
   ### Modifications
   
   - `.waitingFor(Wait.forListeningPort())` with `.withStartupTimeout(Duration.ofMinutes(3))` — bounds the pull/start.
   - `@Test(timeOut = 120_000)` as a backstop, consistent with the Debezium tests hardened in #71.
   
   ### Verifying this change
   
   Passes normally: `MqttSinkIntegrationTest > testWriteE2EWithMosquitto PASSED`, `BUILD SUCCESSFUL in 8s`.
   
   The guard is demonstrated, not assumed. Forcing an unsatisfiable wait strategy with a short startup timeout makes it **fail in ~28s** instead of hanging:
   
   ```
   beforeClass FAILED
       org.testcontainers.containers.ContainerLaunchException: Container startup failed for image eclipse-mosquitto:2
           ... Timed out waiting for log output ...
   ```
   
   ### Note
   
   #89 also suggests surfacing Testcontainers logs in CI (none appear in the job output, which is why the cause had to be inferred from timestamps) and, separately, splitting `integrationTest` into its own parallel job — the test suite has roughly doubled in wall-clock time as container tests landed today. Both are follow-ups, not part of this fix.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [fix][test] Bound MqttSinkIntegrationTest container startup so it cannot hang CI [pulsar-connectors]

Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #91:
URL: https://github.com/apache/pulsar-connectors/pull/91#discussion_r3560228320


##########
mqtt/src/integrationTest/java/org/apache/pulsar/io/mqtt/MqttSinkIntegrationTest.java:
##########
@@ -51,7 +53,11 @@ public class MqttSinkIntegrationTest {
     private static final DockerImageName MOSQUITTO_IMAGE = DockerImageName.parse("eclipse-mosquitto:2");
 
     private final GenericContainer<?> mqttContainer = new GenericContainer<>(MOSQUITTO_IMAGE)
-            .withExposedPorts(MQTT_PORT);
+            .withExposedPorts(MQTT_PORT)
+            // Without an explicit wait strategy and startup timeout, a stalled image pull hangs
+            // @BeforeClass with no bound, taking the whole CI job to its 45-minute limit.

Review Comment:
   The inline comment claims a stalled image pull and container start can hang `@BeforeClass` “with no bound”, but Testcontainers already has defaults (e.g., `pull.timeout` and a default wait strategy). Also, `withStartupTimeout(...)` only configures the wait strategy timeout, not the image pull itself, so the wording here is misleading.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [fix][test] Bound MqttSinkIntegrationTest container startup so it cannot hang CI [pulsar-connectors]

Posted by "david-streamlio (via GitHub)" <gi...@apache.org>.
david-streamlio merged PR #91:
URL: https://github.com/apache/pulsar-connectors/pull/91


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org