chore: fix NATS JetStream IT tests sharing a stream/durable name - #25630
Conversation
… name NatsJetstreamConsumerAckPolicyNoneIT, NatsJetstreamConsumerMaxDeliverIT, and NatsJetstreamConsumerRedeliveryIT all used the identical JetStream stream name (mystream2), subject (mytopic2), and durable consumer name (camel2). NatsITSupport does not tear down streams/consumers between IT classes, and NatsConsumer.setupJetStreamConsumer() never reconciles an already-existing durable consumer's config with a new subscribe request, so whichever of these three tests ran first bound its own ackPolicy/ maxDeliver config to the shared durable consumer server-side. The next test(s) in the suite then silently bound to that stale, mismatched consumer and received zero messages. This is why NatsJetstreamConsumerMaxDeliverIT and NatsJetstreamConsumerRedeliveryIT were previously guarded with @DisabledIfSystemProperty(named = "ci.env.name", ..., "Flaky on GitHub Actions"). That guard was removed without addressing the underlying collision, most likely because it was verified by running each test class individually rather than as part of the full suite. Give each test its own unique stream/subject/durable name, matching the convention already used by the other JetStream IT tests in this package (-manualack, -manualack-nak, -pull, etc). Test-only change, no production code touched. Co-authored-by: Claude <noreply@anthropic.com> Signed-off-by: Claus Ibsen <claus.ibsen@gmail.com>
|
🌟 Thank you for your contribution to the Apache Camel project! 🌟 🐫 Apache Camel Committers, please review the following items:
|
gnodet
left a comment
gnodet
left a comment
There was a problem hiding this comment.
Clean, minimal test-only fix that correctly resolves the NATS JetStream IT stream/durable name collision.
The root-cause analysis in the PR description is thorough: CAMEL-22539 (685e1a4) removed @DisabledIfSystemProperty from these three tests without addressing the shared mystream2/camel2 names, so whichever test ran first owned the durable consumer config and the others silently received zero messages.
The new -ackpolicynone/-maxdeliver/-redelivery suffixes follow the same pattern already used by the other JetStream ITs (-manualack, -pull, etc.), and no remaining collisions exist across the package.
📋 PR Metadata
| Aspect | Current | Suggested |
|---|---|---|
| Labels | components |
test (test-only change, not a component change) |
| Milestone | (none) | 4.22.0 |
Backport: not needed — on camel-4.18.x these tests still carry @DisabledIfSystemProperty, so the collision is masked there.
This review was generated by an AI agent and may contain inaccuracies. Please verify all suggestions before applying.
Claude Code on behalf of @gnodet
|
🧪 CI tested the following changed modules:
🔬 Scalpel shadow comparison — Scalpel: 1 tested, 0 compile-only — current: 9 all testedMaveniverse Scalpel detected 1 affected modules (current approach: 9). Modules only in current approach (8)
Skip-tests mode would test 1 modules (1 direct + 0 downstream), skip tests for 0 (generated code, meta-modules) Modules Scalpel would test (1)
All tested modules (9 modules)
|
Backport of #25630 to camel-4.22.x. NatsJetstreamConsumerAckPolicyNoneIT, NatsJetstreamConsumerMaxDeliverIT, and NatsJetstreamConsumerRedeliveryIT used identical JetStream stream, subject, and durable consumer names. Since NatsITSupport doesn't tear down streams/consumers between IT classes, and NatsConsumer.setupJetStreamConsumer() never reconciles an existing durable consumer's config with a new subscribe request, whichever test ran first bound its config to the shared durable consumer server-side, causing subsequent tests to silently receive zero messages from a stale, mismatched consumer. Gives each test its own unique stream/subject/durable name, matching the convention already used by the other JetStream IT tests in this package. Test-only change, no production code touched. Closes #25632
2 participants
Summary
NatsJetstreamConsumerAckPolicyNoneIT,NatsJetstreamConsumerMaxDeliverIT, andNatsJetstreamConsumerRedeliveryITall used the identical JetStream stream name (mystream2), subject (mytopic2), and durable consumer name (camel2).NatsITSupportdoesn't tear down streams/consumers between IT classes, andNatsConsumer.setupJetStreamConsumer()never reconciles an already-existing durable consumer's config with a new subscribe request. Whichever of these three tests ran first bound its ownackPolicy/maxDeliverconfig to the shared durable consumer server-side; the next test(s) in the suite then silently bound to that stale, mismatched consumer and received zero messages.NatsJetstreamConsumerMaxDeliverITandNatsJetstreamConsumerRedeliveryITwere previously guarded with@DisabledIfSystemProperty(named = "ci.env.name", ..., "Flaky on GitHub Actions"). That guard was removed in a prior "fix flaky tests" pass without addressing the underlying collision — most likely because it was verified by running each test class individually rather than as part of the full suite, which is the exact condition that triggers it.-ackpolicynone,-maxdeliver,-redeliverysuffixes), matching the convention already used correctly by the other JetStream IT tests in this package (-manualack,-manualack-nak,-pull, etc). Test-only change, no production code touched.Test plan
mvn verify -Ddevelocity.cache.local.enabled=falseincomponents/camel-nats— all 28 IT tests pass, including the previously-failing two running immediately afterNatsJetstreamConsumerAckPolicyNoneITin the same suite (the exact ordering that used to trigger the failure).-Dit.test=NatsJetstreamConsumerAckPolicyNoneIT,NatsJetstreamConsumerMaxDeliverIT) that the failure reproduced before the fix and is gone after.Claude Code on behalf of davsclaus