ppalaga commented on code in PR #4133:
URL: https://github.com/apache/camel-quarkus/pull/4133#discussion_r979789736
##########
integration-test-groups/foundation/scheduler/src/main/java/org/apache/camel/quarkus/component/scheduler/it/SchedulerRoute.java:
##########
@@ -30,11 +31,24 @@ public class SchedulerRoute extends RouteBuilder {
@Inject
@Named("schedulerCounter")
AtomicInteger schedulerCounter;
+ @Inject
+ CamelContext context;
Review Comment:
The context field does not seem to be used. Can we perhaps remove it?
##########
integration-test-groups/foundation/scheduler/pom.xml:
##########
@@ -34,6 +34,10 @@
<dependency>
<groupId>org.apache.camel.quarkus</groupId>
<artifactId>camel-quarkus-scheduler</artifactId>
+ </dependency>
+ <dependency>
+ <groupId>org.apache.camel.quarkus</groupId>
+ <artifactId>camel-quarkus-bean</artifactId>
</dependency>
Review Comment:
I'd vote for doing without this additional dependency. We could e.g. have a
an ad hoc processor to increment the counter, something like:
```
.process(e -> myCounter.incrementAndGet());
```
##########
integration-test-groups/foundation/scheduler/src/main/java/org/apache/camel/quarkus/component/scheduler/it/ShoppingList.java:
##########
@@ -0,0 +1,43 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.camel.quarkus.component.scheduler.it;
+
+import java.util.*;
+
+import javax.enterprise.context.ApplicationScoped;
+import javax.inject.Named;
+
+@ApplicationScoped
+@Named("shoppingList")
+public class ShoppingList {
+ List<Integer> lists = new ArrayList<>();
+
+ public void addItems() {
+ int item = new Random().nextInt(100);
+ lists.add(item);
+ }
+
+ public void removeItems() {
+ if (lists != null && lists.size() > 0) {
+ lists.remove(lists.size() - 1);
+ }
+ }
Review Comment:
This does not seem to be used. So we may remove it, no?
##########
integration-test-groups/foundation/scheduler/src/main/java/org/apache/camel/quarkus/component/scheduler/it/ShoppingList.java:
##########
@@ -0,0 +1,43 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.camel.quarkus.component.scheduler.it;
+
+import java.util.*;
+
+import javax.enterprise.context.ApplicationScoped;
+import javax.inject.Named;
+
+@ApplicationScoped
+@Named("shoppingList")
+public class ShoppingList {
+ List<Integer> lists = new ArrayList<>();
+
+ public void addItems() {
Review Comment:
You are adding just a single item.
```suggestion
public void addItem() {
```
##########
integration-test-groups/foundation/scheduler/src/test/java/org/apache/camel/quarkus/component/scheduler/it/SchedulerTest.java:
##########
@@ -28,12 +28,43 @@
class SchedulerTest {
@Test
- public void test() throws Exception {
- // wait until the scheduler has run and return a counter that is > 0
+ public void testInitialDelay() throws Exception {
await().atMost(5, TimeUnit.SECONDS).until(() -> {
String body =
RestAssured.get("/scheduler/get").then().statusCode(200).extract().body().asString();
return !body.equals("0");
});
}
+ @Test
+ public void testDelay() throws Exception {
+ await().atMost(2, TimeUnit.SECONDS).until(() -> {
+ String body =
RestAssured.get("/scheduler/get-count").then().statusCode(200).extract().body().asString();
+ return Integer.parseInt(body) == 1;
+ });
+ }
+
+ @Test
+ public void testDelayWithFixed() throws Exception {
+ await().atMost(4, TimeUnit.SECONDS).until(() -> {
+ String body =
RestAssured.get("/scheduler/get-count").then().statusCode(200).extract().body().asString();
+ return Integer.parseInt(body) > 1;
+ });
+ }
Review Comment:
It looks like you assume `testDelay()` to be executed before
`testDelayWithFixed()` ? Maybe one test method would serve better?
```suggestion
@Test
public void testDelay() throws Exception {
await().atMost(2, TimeUnit.SECONDS).until(() -> {
String body =
RestAssured.get("/scheduler/get-count").then().statusCode(200).extract().body().asString();
return Integer.parseInt(body) > 1;
});
await().atMost(2, TimeUnit.SECONDS).until(() -> {
String body =
RestAssured.get("/scheduler/get-count").then().statusCode(200).extract().body().asString();
return Integer.parseInt(body) > 2;
});
}
```
##########
integration-test-groups/foundation/scheduler/src/main/java/org/apache/camel/quarkus/component/scheduler/it/SchedulerRoute.java:
##########
@@ -30,11 +31,24 @@ public class SchedulerRoute extends RouteBuilder {
@Inject
@Named("schedulerCounter")
AtomicInteger schedulerCounter;
+ @Inject
+ CamelContext context;
@Override
public void configure() throws Exception {
- from("scheduler:start?initialDelay=1")
+ from("scheduler:withInitialDelay?initialDelay=1")
.process(e -> schedulerCounter.incrementAndGet());
+ from("scheduler:withDelay?delay=1000")
+ .to("bean:myCounter?method=increment");
+
+ from("scheduler:useFixedDelay?initialDelay=2000&useFixedDelay=true")
+ .to("bean:myCounter?method=increment");
Review Comment:
I'd vote for using shorter delays, to save some testing time. 200ms should
be fine. At the same time, we should not check using `==` in the tests, only
`<`, for the case that the test is too slow to observe the exact value.
##########
integration-test-groups/foundation/scheduler/src/main/java/org/apache/camel/quarkus/component/scheduler/it/GreedyCounter.java:
##########
@@ -0,0 +1,34 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.camel.quarkus.component.scheduler.it;
+
+import javax.enterprise.context.ApplicationScoped;
+import javax.inject.Named;
+
+@ApplicationScoped
+@Named("greedyCounter")
+public class GreedyCounter {
+ private int count = 0;
+
+ public int getCount() {
+ return count;
+ }
+
+ public void increment() {
+ count++;
+ }
Review Comment:
Note that getCount and increment() can be called from differen threads
concurrently. While I do not think it matters much in how we call them from
tests currently, it is still potentially dangerous (end users may copy this and
use differently, we may extend the test in the future). So producing a named
AtomicInteger is perhaps a better choice.
##########
integration-test-groups/foundation/scheduler/src/main/java/org/apache/camel/quarkus/component/scheduler/it/ShoppingList.java:
##########
@@ -0,0 +1,43 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.camel.quarkus.component.scheduler.it;
+
+import java.util.*;
+
+import javax.enterprise.context.ApplicationScoped;
+import javax.inject.Named;
+
+@ApplicationScoped
+@Named("shoppingList")
+public class ShoppingList {
+ List<Integer> lists = new ArrayList<>();
+
+ public void addItems() {
+ int item = new Random().nextInt(100);
+ lists.add(item);
+ }
+
+ public void removeItems() {
+ if (lists != null && lists.size() > 0) {
+ lists.remove(lists.size() - 1);
+ }
+ }
+
+ public List<Integer> getItems() {
+ return lists;
+ }
Review Comment:
There is a potential race between `getItems()` and `addItems()` - you may
get concurrent modification exception when accessing the list via `getItems()`.
You may want to switch from ArrayList to
java.util.concurrent.CopyOnWriteArrayList.
--
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
