SEPURI-SAI-KRISHNA opened a new pull request, #12416: URL: https://github.com/apache/seatunnel/pull/12416
### Purpose of this pull request Follow-up to @SEZ9's review on #12410, which was already in the merge queue when the review landed, so these did not go into it. Test-only; both main classes are byte-identical to `dev`. **Issue 1, the config round trip asserted five fields of thirteen.** It now sets all eleven configurable options and asserts every getter. `MqttSourceReader` reads `password`, `format`, `fieldDelimiter`, the three timeouts and `maxQueueSize` on the worker, so losing any of them broke a job rather than a test. **Issue 2, the boundedness assertion was a constant in disguise.** `MqttSource.getBoundedness()` only throws when `jobContext != null`, so a dropped job context falls through to `UNBOUNDED` and the assertion held anyway. `testJobContextSurvivesJavaSerialization` now round-trips a BATCH context and expects `MqttConnectorException`, mirroring the existing `testBatchJobModeFails`. **Issue 3, hand-rolled stream boilerplate.** Both round trips now use `SerializationUtils.deserialize(SerializationUtils.serialize(x))`, dropping four `java.io` imports. This matches `SyslogSinkFactoryTest`, `PulsarSourceTest`, `LsnOffsetFactoryTest` and three other connector tests. Two further gaps came out of checking those three rather than from the review itself. **`sourceConfig` was not covered at all.** It is the third field of `MqttSource` and the one `createReader` hands to the reader. With it marked `transient`, all 34 tests passed. That is the field #12406 was about. There is no getter for it, so `testSourceIsJavaSerializable` now builds a reader from the restored source, which fails because `MqttSourceReader`'s constructor dereferences it at line 77. **The round-trip values were chosen against the wrong baseline.** `qos=0` and `clean_session=false` were picked to differ from the option defaults. Deserialization does not run the constructor, so a rebuilt default is not a reachable failure mode, while a field that stops being written leaves the type's zero value. `0` and `false` are exactly those, so neither mutation was detected. They are now `1` and `true`, chosen to differ from zero. Those two options have no value that is both non-default and non-zero, since `validate()` limits `qos` to 0 or 1, so catching the reachable failure wins and the comment says so. ### Does this PR introduce _any_ user-facing change? No. Two test classes, both extended rather than created. No production code. ### How was this patch tested? `./mvnw -pl seatunnel-connectors-v2/connector-mqtt test`: **34 tests, 0 failures, 0 errors**. `./mvnw -q -DskipTests verify` on the module passes on JDK 11, and `spotless:check` was run separately with up-to-date caching disabled. Since the point of this PR is that assertions were not catching things, I mutated every field to `transient`, one at a time, and confirmed each is now detected: | Target | Before | After | |---|---|---| | 13 `MqttSourceConfig` fields | 11 of 13 caught (`qos`, `cleanSession` missed) | 13 of 13 caught | | 3 `MqttSource` fields | 2 of 3 caught (`sourceConfig` missed) | 3 of 3 caught | | `MqttSourceConfig` loses `implements Serializable` | caught | caught | The `jobContext` mutation is the clearest illustration: it fails exactly one test, the new one, while `testSourceIsJavaSerializable` still passes, which is what Issue 2 was about. -- 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]
