orpiske commented on code in PR #11711:
URL: https://github.com/apache/camel/pull/11711#discussion_r1358058977
##########
components/camel-quartz/src/test/java/org/apache/camel/component/quartz/SpringQuartzConsumerTwoAppsClusteredRecoveryTest.java:
##########
@@ -55,14 +58,22 @@ public void testQuartzPersistentStoreClusteredApp() throws
Exception {
log.warn("Crashed...");
log.warn("Crashed...");
- Thread.sleep(2000);
+ Awaitility.await().timeout(2000, TimeUnit.MILLISECONDS);
// as well as the second one which will run in slave mode as it will
not be able to acquire the same lock
AbstractXmlApplicationContext app2 =
newAppContext("SpringQuartzConsumerRecoveryClusteredAppTwo.xml");
app2.start();
// wait long enough until the second app takes it over...
- Thread.sleep(2000);
+ Awaitility.await().untilAsserted(() -> {
+ CamelContext camel2 = app2.getBean("camelContext2-" +
getClass().getSimpleName(), CamelContext.class);
+
+ MockEndpoint mock2 = camel2.getEndpoint("mock:result",
MockEndpoint.class);
+ mock2.expectedMinimumMessageCount(2);
+ mock2.expectedMessagesMatches(new ClusteringPredicate(false));
+
+ mock2.assertIsSatisfied();
+ });
Review Comment:
Same as above.
##########
components/camel-quartz/src/test/java/org/apache/camel/component/quartz/SpringQuartzConsumerTwoAppsClusteredFailoverTest.java:
##########
@@ -65,7 +66,15 @@ public void testQuartzPersistentStoreClusteredApp() throws
Exception {
log.warn("Crashed...");
// wait long enough until the second app takes it over...
- Thread.sleep(2000);
+ Awaitility.await().untilAsserted(() -> {
+ CamelContext camel2 = app2.getBean("camelContext2-" +
getClass().getSimpleName(), CamelContext.class);
+
+ MockEndpoint mock2 = camel2.getEndpoint("mock:result",
MockEndpoint.class);
+ mock2.expectedMinimumMessageCount(3);
+ mock2.expectedMessagesMatches(new ClusteringPredicate(false));
+
+ mock2.assertIsSatisfied();
+ });
Review Comment:
We can probably shorten the asserted block.
##########
components/camel-quartz/src/test/java/org/apache/camel/routepolicy/quartz/CronScheduledRoutePolicyTest.java:
##########
@@ -330,10 +331,16 @@ public void configure() {
ServiceHelper.suspendService(context.getRoute("test").getConsumer());
- Thread.sleep(5000);
- template.sendBody("direct:start", "Ready or not, Here, I come");
+ try {
+ Awaitility.await().timeout(5,
TimeUnit.SECONDS).catchUncaughtExceptions().untilAsserted(() -> {
+ template.sendBody("direct:start", "Ready or not, Here, I
come");
+ success.assertIsSatisfied();
+ });
+ } catch (CamelExecutionException e) {
+ throw new CamelExecutionException(e.getMessage(),
e.getExchange(), e);
+ }
Review Comment:
Hm, this seems a bit strange to me. I think it would probably be better to
separate error handling from the test assertion.
##########
components/camel-quartz/src/test/java/org/apache/camel/routepolicy/quartz/SpringMultiplePoliciesOnRouteTest.java:
##########
@@ -34,7 +34,6 @@ public void testMultiplePoliciesOnRoute() throws Exception {
for (int i = 0; i < size; i++) {
template.sendBody(url, "Message " + i);
- Thread.sleep(3);
}
Review Comment:
I think we need this `Thread.sleep` if we want to throttle sending the
messages. I have seen this pattern elsewhere. I don't think you need to fix
this one right now (see: https://issues.apache.org/jira/browse/CAMEL-19986)
--
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: commits-unsubscr...@camel.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
