davsclaus commented on code in PR #24526:
URL: https://github.com/apache/camel/pull/24526#discussion_r3543371057
##########
components/camel-zookeeper-master/src/test/java/org/apache/camel/component/zookeepermaster/integration/MasterEndpointFailoverIT.java:
##########
@@ -36,12 +39,16 @@
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
+import static org.awaitility.Awaitility.await;
+
public class MasterEndpointFailoverIT {
@RegisterExtension
static ZooKeeperService service = ZooKeeperServiceFactory.createService();
private static final transient Logger LOG =
LoggerFactory.getLogger(MasterEndpointFailoverIT.class);
+ private static final Duration FAILOVER_TIMEOUT = Duration.ofSeconds(5);
+
protected ProducerTemplate template;
protected CamelContext producerContext;
protected CamelContext consumerContext1;
Review Comment:
The `FAILOVER_TIMEOUT` of 5 seconds may be tight on heavily loaded CI nodes.
Consider bumping to 10–15 seconds to avoid flakiness — the project's own
guidelines use `atMost(20, TimeUnit.SECONDS)` as an example.
```suggestion
private static final Duration FAILOVER_TIMEOUT = Duration.ofSeconds(15);
```
##########
components/camel-zookeeper-master/src/test/java/org/apache/camel/component/zookeepermaster/integration/MasterEndpointFailoverIT.java:
##########
@@ -113,22 +120,67 @@ public void afterRun() {
@Test
public void testEndpoint() throws Exception {
LOG.info("Starting consumerContext1");
- consumerContext1.start();
+ consumerContext1.start(); // Idempotent re-start for test flow clarity
+ awaitMaster(consumerContext1);
assertMessageReceived(result1Endpoint, result2Endpoint);
LOG.info("Starting consumerContext2");
consumerContext2.start();
+ awaitMaster(consumerContext1);
+ awaitStandby(consumerContext2);
assertMessageReceivedLoop(result1Endpoint, result2Endpoint, 3);
Review Comment:
This uses `/* */` (single-star) instead of `/** */`, so the `@param` tag
won't be processed by Javadoc tooling. That said, for a test-internal helper,
the doc block could simply be removed.
##########
components/camel-zookeeper-master/src/test/java/org/apache/camel/component/zookeepermaster/integration/MasterEndpointFailoverIT.java:
##########
@@ -113,22 +120,67 @@ public void afterRun() {
@Test
public void testEndpoint() throws Exception {
LOG.info("Starting consumerContext1");
- consumerContext1.start();
+ consumerContext1.start(); // Idempotent re-start for test flow clarity
+ awaitMaster(consumerContext1);
assertMessageReceived(result1Endpoint, result2Endpoint);
LOG.info("Starting consumerContext2");
consumerContext2.start();
+ awaitMaster(consumerContext1);
+ awaitStandby(consumerContext2);
assertMessageReceivedLoop(result1Endpoint, result2Endpoint, 3);
LOG.info("Stopping consumerContext1");
consumerContext1.stop();
+ awaitMaster(consumerContext2);
assertMessageReceivedLoop(result2Endpoint, result1Endpoint, 3);
}
+ /*
+ * Wait fixed time for the master consumer to be the master and connected.
+ * @param context the CamelContext
+ */
+ protected void awaitMaster(CamelContext context) {
+ await().atMost(FAILOVER_TIMEOUT)
+ .until(() -> {
+ MasterConsumer masterConsumer = getMasterConsumer(context);
+ return masterConsumer.isMaster() &&
masterConsumer.isConnected();
+ });
+ }
+
+ /*
+ * Wait fixed time for the master consumer to be the standby and connected.
+ * @param context the CamelContext
+ */
+ protected void awaitStandby(CamelContext context) {
Review Comment:
Minor: could use Java 17+ pattern matching to simplify:
```suggestion
protected MasterConsumer getMasterConsumer(CamelContext context) {
if (context.getRoutes().isEmpty()) {
throw new IllegalStateException("No routes registered in
context");
}
Consumer consumer = context.getRoutes().get(0).getConsumer();
if (consumer instanceof MasterConsumer mc) {
return mc;
}
throw new IllegalStateException("Expected MasterConsumer but got: "
+ consumer.getClass());
}
```
--
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]
