david-streamlio commented on issue #55: URL: https://github.com/apache/pulsar-connectors/issues/55#issuecomment-4936382835
Closing: this issue's premise was mistaken, and the real gap behind it has been fixed. **MQTT already had a Testcontainers integration test.** `MqttSinkIntegrationTest` shipped 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 `src/integrationTest` source set rather than `src/test`. Verified locally, and it is not a vacuous test — pointing the sink at a different topic than the subscriber listens on makes it fail: ``` ./gradlew :mqtt:integrationTest MqttSinkIntegrationTest > testWriteE2EWithMosquitto PASSED BUILD SUCCESSFUL in 18s ``` **The actual defect was worse than this issue described:** CI only ran `./gradlew test`, never `integrationTest`, so that test had never executed on a single pull request. The coverage existed but was unenforced — a change breaking MQTT delivery would still have gone green. #74 fixed that by adding `integrationTest` to the CI matrix, and it has merged. One follow-up, tracked where it belongs rather than here: `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. Noted on #34. -- 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]
