DanielLeens commented on PR #11461: URL: https://github.com/apache/seatunnel/pull/11461#issuecomment-6367058871
I prepared a minimal patch for the serialization-test blocker on current head `8a9c8bf4f83e9af3f30b9f2573cc7cdd86d862c3`. The [current-head JDK 11 Ubuntu job](https://github.com/ealeonraz/seatunnel/actions/runs/38245466425/job/114800838331) reports `Expected java.lang.IllegalArgumentException to be thrown, but nothing was thrown` in `MqttSinkTest.testSinkConfigValuesSurviveJavaSerialization:93`. The test added by #12455 observes whether a non-default configuration value survives Java serialization. This PR moves QoS validation into the factory, so `qos=2` no longer triggers the writer-constructor exception used by that test. The patch uses `format=xml` instead: the writer still rejects that format before connecting to MQTT. It preserves the exception assertion and strengthens the message assertion to an exact match. No production code or test coverage is removed. Validation: module-scoped `./mvnw spotless:apply -pl seatunnel-connectors-v2/connector-mqtt -nsu -Dmaven.gitcommitid.skip=true`, `git diff --check`, and patch application against the current-head index all passed. Compilation and tests have not been run for this patch; a new GitHub CI run is still required. I cannot push this directly: GitHub rejected a dry-run push to the source branch with `Permission to ealeonraz/seatunnel.git denied to DanielLeens` (HTTP 403). The branch owner or a maintainer with write access can apply this diff and push it normally: ```diff diff --git a/seatunnel-connectors-v2/connector-mqtt/src/test/java/org/apache/seatunnel/connectors/seatunnel/mqtt/sink/MqttSinkTest.java b/seatunnel-connectors-v2/connector-mqtt/src/test/java/org/apache/seatunnel/connectors/seatunnel/mqtt/sink/MqttSinkTest.java index f1443e4c79f..3ae7ddd773d 100644 --- a/seatunnel-connectors-v2/connector-mqtt/src/test/java/org/apache/seatunnel/connectors/seatunnel/mqtt/sink/MqttSinkTest.java +++ b/seatunnel-connectors-v2/connector-mqtt/src/test/java/org/apache/seatunnel/connectors/seatunnel/mqtt/sink/MqttSinkTest.java @@ -77,13 +77,12 @@ class MqttSinkTest { @Test void testSinkConfigValuesSurviveJavaSerialization() { - // qos 2 is not a legal MQTT QoS for this connector and MqttSinkWriter rejects it, but - // MqttSink itself does not validate, so the sink builds fine and only a writer surfaces - // it. That makes it an observable, non-default value: MqttSinkOptions.QOS defaults to 1, - // so a pluginConfig that came back empty would yield a legal qos and the writer would - // build without complaint. The rejection below only happens if the 2 travelled. + // Unsupported formats are still rejected when the writer creates its serializer, before + // connecting to MQTT. Unlike qos validation, this check remains in the writer after the + // OptionRule migration. The default format is json, so this rejection proves that the + // non-default xml value survived serialization. Map<String, Object> config = baseConfig(); - config.put("qos", 2); + config.put("format", "xml"); MqttSink sink = newSink(config); MqttSink restored = SerializationUtils.deserialize(SerializationUtils.serialize(sink)); @@ -93,9 +92,7 @@ class MqttSinkTest { Assertions.assertThrows( IllegalArgumentException.class, () -> restored.createWriter(Mockito.mock(SinkWriter.Context.class))); - Assertions.assertTrue( - exception.getMessage().contains("got: 2"), - "expected the restored qos in the message, got: " + exception.getMessage()); + Assertions.assertEquals("Unsupported MQTT sink format: xml", exception.getMessage()); } } ``` -- 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]
