vbhanuchander-lang commented on PR #7860: URL: https://github.com/apache/hop/pull/7860#issuecomment-5234866143
Pushed `56bb885` closing the two gaps I flagged in the description. Both turned out to matter more than I expected. **The plugin was not packaged.** I said I could not verify the assembly build locally and that the change was "a six-line dependency entry copied from the Kafka one". That was true but insufficient: each plugin module also needs its own `src/assembly/assembly.xml` to produce the `zip` that `assemblies/plugins` consumes. Without it the build fails with: ``` Could not find artifact org.apache.hop:hop-transform-jms:zip:2.19.0-SNAPSHOT ``` So **CI would have failed on this PR**. Added the descriptor and the `version.xml` it filters, both mirroring the Kafka plugin. The zip now builds and packages the client correctly: ``` plugins/transforms/jms/version.xml plugins/transforms/jms/dependencies.xml plugins/transforms/jms/lib/artemis-jakarta-client-2.42.0.jar plugins/transforms/jms/lib/jakarta.jms-api-3.1.0.jar plugins/transforms/jms/lib/... (netty, jgroups, artemis-core-client, …) ``` **There is now a real broker test.** Rather than guess at the `integration-tests` harness conventions for a new broker, `JmsProviderBrokerTest` starts an embedded ActiveMQ Artemis broker in-process — no Docker, no external service, runs in the normal `mvn test` phase. It covers a queue round trip with correlation id and properties, topic publish/subscribe, a transacted send, variable resolution in the destination name, `receive` returning null on an empty queue, the blank-destination and unreachable-broker error paths, and idempotent `close`. Most importantly it **proves the acknowledgement claim instead of asserting it**: a message received but not acknowledged is redelivered after reconnect, and disappears only once acknowledged. That was the main behavioural claim in the description and it was previously untested. One detail worth noting: I first named it `JmsProviderBrokerIT`, and a plain `mvn test` reported 9 tests instead of 18 — Surefire's default includes skip `*IT` classes, so the suffix alone decided whether the broker test ran in CI. Renamed to `...BrokerTest`. ``` Tests run: 4 JmsProviderTest Tests run: 9 JmsProviderBrokerTest Tests run: 5 JmsConsumerMetaTest Tests run: 18, Failures: 0, Errors: 0, Skipped: 0 ``` `spotless:apply` clean, `apache-rat:check` 0 unapproved over 25 files. `artemis-server` is added test-scoped only (declared in `lib/pom.xml` like the others), so it does not ship. A Docker-based `integration-tests/jms` project with an Artemis service is still worth having for a full pipeline-level test, and I am happy to add it — I would just want to follow your conventions for a new broker service rather than invent them. -- 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]
