david-streamlio commented on issue #55: URL: https://github.com/apache/pulsar-connectors/issues/55#issuecomment-4929947784
Correction to this issue's premise, found while starting the work: **the MQTT sink already has a Testcontainers integration test.** `MqttSinkIntegrationTest` was added alongside the connector in #22 — it starts an `eclipse-mosquitto:2` container, subscribes before writing, publishes through `MqttSink`, and asserts delivery. My original audit missed it because it lives in a separate `src/integrationTest` source set rather than `src/test`, and I only scanned `src/test`. Verified locally: ``` ./gradlew :mqtt:integrationTest MqttSinkIntegrationTest > testWriteE2EWithMosquitto PASSED BUILD SUCCESSFUL in 18s ``` It is also a meaningful test, not a vacuous one: pointing the sink at a different topic than the subscriber listens on makes it fail. **However, the real gap is worse than the one this issue describes.** CI only runs `./gradlew test`, never `integrationTest`, so this test has never executed on a pull request. The coverage exists but is unenforced — a change breaking MQTT delivery would still go green. #74 fixes that by adding `integrationTest` to the CI matrix. Suggest closing this issue once #74 merges, with one follow-up worth tracking separately: `MqttSink.write()` always publishes to the static `mqttSinkConfig.getTopic()`, so the dynamic-topic behavior proposed in #34 will need its own test case once that PR lands. -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
