SEPURI-SAI-KRISHNA opened a new pull request, #12436: URL: https://github.com/apache/seatunnel/pull/12436
### Purpose of this pull request Follow-up to #12416. That PR merged at 04:37Z, shortly before I had folded in the review points @SEZ9 and @DanielLeens raised on it, so those improvements are not on `dev`. This carries exactly those and nothing else. `MqttSourceConfigTest` is already identical to `dev`, so this touches a single file. ### What changes **The restored `sourceConfig` is now checked for its value, not just its presence.** #12416 built a reader from the deserialized source and asserted only that it did not throw. That distinguishes a lost `sourceConfig` from a present one, but it observes nothing inside it, and the source was built from `baseConfig()` so every field carried its default. The source now sets `max_queue_size = 1`, which is not the default, and the restored reader is driven past that limit, mirroring `testReaderFailsWhenMessageQueueIsFull`. **Both halves of the boundedness pair are asserted again.** #12416 removed `assertEquals(Boundedness.UNBOUNDED, restored.getBoundedness())` because it cannot detect a lost job context. That is true, but as @SEZ9 pointed out it was the half that detects a context restored with a null `jobMode`: `STREAMING.equals(null)` is false, so such a context throws in the STREAMING case, while the BATCH test cannot tell it apart from a genuine BATCH context. The comment now says the two are complementary. **A factual error in a comment is corrected.** It said `sourceConfig` is "the third and last field of `MqttSource`". The declaration order is `sourceConfig`, `catalogTable`, `jobContext`, so it is the first. The ordinal is dropped rather than renumbered so it cannot go stale if the fields are reordered. Not included: @SEZ9's Issue 5, a shared `fullConfig()` helper for `testExplicitValues` and the round-trip test. The drift it describes is real, but it means rewriting a neighbouring test that is not broken, so I would rather do it separately if wanted. ### Does this PR introduce _any_ user-facing change? No. One test file. `MqttSource`, `MqttSourceConfig` and `MqttSourceReader` are byte-identical to `dev`. ### How was this patch tested? ``` ./mvnw -pl seatunnel-connectors-v2/connector-mqtt test Tests run: 34, Failures: 0, Errors: 0, Skipped: 0 ``` `spotless:check` is green. Two mutations, to show the new assertion has teeth and to be precise about what it adds: | Mutation | Old assertion | New assertion | |---|---|---| | `maxQueueSize` marked `transient` | fails | fails | | `MqttSourceReader` hardcodes `new LinkedBlockingQueue<>(2048)` | **passes** | fails | The first does not separate them, which supports @DanielLeens's point that a pure serialization defect fails `MqttSourceConfigTest` regardless. The second does: it is a config-to-reader wiring defect rather than a serialization one. `testReaderFailsWhenMessageQueueIsFull` also catches it, so the new assertion is not uniquely responsible; what is new is that it is caught on a deserialized source. -- 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]
