[GitHub] [camel-quarkus] aldettinger commented on a diff in pull request #4133: Improve test coverage for scheduler component.
aldettinger commented on code in PR #4133:
URL: https://github.com/apache/camel-quarkus/pull/4133#discussion_r993471663
##
integration-test-groups/foundation/scheduler/src/test/java/org/apache/camel/quarkus/component/scheduler/it/SchedulerTest.java:
##
@@ -28,12 +28,48 @@
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-delay-count").then().statusCode(200).extract().body()
+.asString();
+return Integer.parseInt(body) > 2;
+});
+
+}
+
+@Test
+public void testFixedDelay() throws Exception {
+await().atMost(2, TimeUnit.SECONDS).until(() -> {
+String body =
RestAssured.get("/scheduler/get-fixed-delay-count").then().statusCode(200).extract().body()
+.asString();
+return Integer.parseInt(body) > 2;
+});
+}
+
+@Test
+public void testDelayWithRepeat() throws Exception {
+await().atMost(4, TimeUnit.SECONDS).until(() -> {
+String body =
RestAssured.get("/scheduler/get-repeat-count").then().statusCode(200).extract().body()
+.asString();
+return Integer.parseInt(body) >= 4;
+});
+}
+
+@Test
+public void testGreedyScheduler() throws Exception {
+await().atMost(1, TimeUnit.SECONDS).until(() -> {
+String body =
RestAssured.get("/scheduler/get-greedy-count").then().statusCode(200).extract().body()
+.asString();
+return Integer.parseInt(body) > 10;
Review Comment:
For a next round, maybe `body().as(Integer.class)` would avoid manual
parsing here. Not a blocker though :)
--
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
[GitHub] [camel-quarkus] aldettinger commented on a diff in pull request #4133: Improve test coverage for scheduler component.
aldettinger commented on code in PR #4133:
URL: https://github.com/apache/camel-quarkus/pull/4133#discussion_r993302765
##
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-delay-count").then().statusCode(200).extract().body()
+.asString();
+return Integer.parseInt(body) > 2;
+});
+
+await().atMost(2, TimeUnit.SECONDS).until(() -> {
Review Comment:
And so, at the end of the day. Those 2 tests are independent ? So we could
better separate them back in tests ? Like `testDelay` and `testFixedDelay` ?
--
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
[GitHub] [camel-quarkus] aldettinger commented on a diff in pull request #4133: Improve test coverage for scheduler component.
aldettinger commented on code in PR #4133:
URL: https://github.com/apache/camel-quarkus/pull/4133#discussion_r993300252
##
integration-test-groups/foundation/scheduler/src/test/java/org/apache/camel/quarkus/component/scheduler/it/SchedulerTest.java:
##
@@ -28,12 +28,42 @@
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(() -> {
Review Comment:
Ok, looks more robust now :+1:
--
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
[GitHub] [camel-quarkus] aldettinger commented on a diff in pull request #4133: Improve test coverage for scheduler component.
aldettinger commented on code in PR #4133:
URL: https://github.com/apache/camel-quarkus/pull/4133#discussion_r993299422
##
integration-test-groups/foundation/scheduler/src/main/java/org/apache/camel/quarkus/component/scheduler/it/SchedulerResource.java:
##
@@ -33,11 +31,15 @@
public class SchedulerResource {
@Inject
-@Named("myCounter")
-AtomicInteger myCounter;
+@Named("withDelayCounter")
+AtomicInteger withDelayCounter;
-@Named("shoppingList")
-CopyOnWriteArrayList shoppingList;
+@Inject
+@Named("useFixedDelayCounter")
+AtomicInteger useFixedDelayCounter;
+
+@Named("withDelayRepeat")
+AtomicInteger withDelayRepeat;
Review Comment:
Maybe `withDelayRepeatCounter` would align with other names ? What do you
think ?
--
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
[GitHub] [camel-quarkus] aldettinger commented on a diff in pull request #4133: Improve test coverage for scheduler component.
aldettinger commented on code in PR #4133:
URL: https://github.com/apache/camel-quarkus/pull/4133#discussion_r993099264
##
integration-test-groups/foundation/scheduler/src/test/java/org/apache/camel/quarkus/component/scheduler/it/SchedulerTest.java:
##
@@ -28,12 +28,42 @@
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(() -> {
Review Comment:
The main idea is that, from my point of view, the current implementation of
this test could pass even when the `withDelay` route is never called.
From there, one solution could be to have two distinct AtomicInteger, let's
say `withDelayCounter` and `useFixedDelayCounter`. And then, it's possible to
check that each route has been invoked separately.
--
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
[GitHub] [camel-quarkus] aldettinger commented on a diff in pull request #4133: Improve test coverage for scheduler component.
aldettinger commented on code in PR #4133:
URL: https://github.com/apache/camel-quarkus/pull/4133#discussion_r992280042
##
integration-test-groups/foundation/scheduler/src/test/java/org/apache/camel/quarkus/component/scheduler/it/SchedulerTest.java:
##
@@ -28,12 +28,42 @@
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;
+});
+
+await().atMost(2, TimeUnit.SECONDS).until(() -> {
+String body =
RestAssured.get("/scheduler/get-count").then().statusCode(200).extract().body().asString();
+return Integer.parseInt(body) > 2;
+});
+}
+
+@Test
+public void testDelayWithRepeat() throws Exception {
+await().atMost(4, TimeUnit.SECONDS).until(() -> {
+String body =
RestAssured.get("/scheduler/get-items").then().statusCode(200).extract().body().asString();
+return body.split(",").length >= 4;
Review Comment:
Here, we expect a list of items >= 4. Can't we do that with a simple
AtomicInteger ?
--
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
[GitHub] [camel-quarkus] aldettinger commented on a diff in pull request #4133: Improve test coverage for scheduler component.
aldettinger commented on code in PR #4133:
URL: https://github.com/apache/camel-quarkus/pull/4133#discussion_r992277981
##
integration-test-groups/foundation/scheduler/src/test/java/org/apache/camel/quarkus/component/scheduler/it/SchedulerTest.java:
##
@@ -28,12 +28,42 @@
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(() -> {
Review Comment:
Could the body could be > 1 even if withDelay is never called ?
At the end of the day, we could maybe better to separate `withDelay` and
`useFixedDelay`.
So, 2 distinct tests and 2 distinct atomic integers.
What do you think ?
--
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
[GitHub] [camel-quarkus] aldettinger commented on a diff in pull request #4133: Improve test coverage for scheduler component.
aldettinger commented on code in PR #4133:
URL: https://github.com/apache/camel-quarkus/pull/4133#discussion_r979915835
##
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:
And maybe the proposal is even to produce one `AtomicInteger` named
`greedyCounter` and a second one named `mycounter`
--
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
