gnodet-bot commented on code in PR #27201:
URL: https://github.com/apache/camel/pull/27201#discussion_r4218738415
##########
components/camel-milo/src/test/java/org/apache/camel/component/milo/MonitorItemTest.java:
##########
@@ -78,20 +125,63 @@ public void setup(TestInfo testInfo) {
}
/**
- * Monitor multiple events
+ * Monitor multiple events With explicit parameters for
requestedPublishingInterval, samplingInterval, and queueSize
*/
@Test
public void testMonitorItem1() throws Exception {
/*
- * we will wait 2 * 1_000 milliseconds between server updates since the
- * default server update rate is 1_000 milliseconds
+ * we will wait 2 * 100 milliseconds between server updates since the
+ * explicitly set update rate is 100 milliseconds (samplingInterval)
in most clients.
+ * With 2000ms requestedPublishingInterval and bigger queueSize of 4
we should get all updates which change the value (in client 1).
+ * The test relies on milo's default discardOldest=true so the newest
value ("Done") survives a queue overflow,
+ * there is currently no parameter to change this.
+ * Extra clients have been added to test independence of parameters
set. See description on test endpoints
+ *
*/
- final var time = 2 * 1_000;
+ final var time = 2 * 100;
final var timeout = 10 * 1_000; // 10 seconds timeout for assertions
- // item 1 ... only this one receives
+ /*
+ * test1Endpoint is related to client C1_1 with:
requestedPublishingInterval=2000, samplingInterval=100,
+ * queueSize=4. It should get the 3 of the first 4 messages (with
changes) because of samplingInterval less
+ * than update rate and queueSize over 3 and an update pause of 2
seconds after they are sent.
+ * From the rest 16 messages, sent in about 3 seconds, some should be
dropped because of queueSize. They should
+ * fall into 2 (or 3 depending on exact timing) publishing periods, so
there should be at least 8 and at most 12 extra messages.
+ */
test1Endpoint.reset();
- test1Endpoint.setExpectedCount(3);
+ test1Endpoint.setMinimumExpectedMessageCount(11); // the first 3,
plus at least 8 more from rest (if they fall to 2 periods)
+ test1Endpoint.setAssertPeriod(3000);
+
+ /*
+ * test2Endpoint is related to client C1_2 which differs from C1_1
only in dataChangeFilterTrigger=StatusValueTimestamp,
+ * so it will get the 3rd update from the first set of 4 which does
not change the value.
+ */
+ test2Endpoint.reset();
+ test2Endpoint.setMinimumExpectedMessageCount(12); // the first 4,
plus at least 8 more from rest (if they fall to 2 periods)
+ test2Endpoint.setAssertPeriod(3000);
+
+ /*
+ * test3Endpoint is related to client C1_3 with:
requestedPublishingInterval=2000, samplingInterval=1000 from
+ * default, queueSize=4. It should get 1 (or 2 depending on exact
timing) of the first 3 messages because of
+ * samplingInterval and an update pause of 2 seconds after they are
sent. From the rest 16 messages, sent in
+ * about 3 seconds, only 3 or 4 should be sampled. 8 and at most 12
extra messages. So minimum is 4, maximum is
+ * 6 messages
Review Comment:
💬 **Stray fragment in test3 comment.** The clause `"8 and at most 12 extra
messages."` was copied from the test1 block and contradicts the `"minimum is 4,
maximum is 6"` conclusion that follows.
```suggestion
* about 3 seconds, only 3 or 4 should be sampled. So minimum is 4,
maximum is
* 6 messages
```
##########
components/camel-milo/src/test/java/org/apache/camel/component/milo/MonitorItemTest.java:
##########
@@ -102,23 +192,61 @@ public void testMonitorItem1() throws Exception {
LOG.info("Server Port: {}", this.getServerPort());
LOG.info("Client URI resolved: {}", resolve(MILO_CLIENT_ITEM_C1_1));
- // set server values
+ // set server values sent faster than requestedPublishingInterval so
they must be put to queue
this.producer1.sendBody("Foo");
await().pollDelay(time, TimeUnit.MILLISECONDS).untilAsserted(() -> {
});
this.producer1.sendBody("Bar");
await().pollDelay(time, TimeUnit.MILLISECONDS).untilAsserted(() -> {
});
+ // This update will only be caught on MILO_CLIENT_ITEM_C1_2 because of
dataChangeFilterTrigger=StatusValueTimestamp
+ this.producer1.sendBody("Bar");
+ await().pollDelay(time, TimeUnit.MILLISECONDS).untilAsserted(() -> {
+ });
this.producer1.sendBody("Baz");
await().pollDelay(time, TimeUnit.MILLISECONDS).untilAsserted(() -> {
});
+ // now wait for requestedPublishingInterval so we are sure the 3
values are passed
+ await().pollDelay(10 * time, TimeUnit.MILLISECONDS).untilAsserted(()
-> {
+ });
+ for (int i = 1; i <= 15; i++) {
+ this.producer1.sendBody("Message " + i);
+ await().pollDelay(time, TimeUnit.MILLISECONDS).untilAsserted(() ->
{
+ });
+ }
+ this.producer1.sendBody("Done");
+
// tests
testBody(this.test1Endpoint.message(0), assertGoodValue("Foo"));
testBody(this.test1Endpoint.message(1), assertGoodValue("Bar"));
testBody(this.test1Endpoint.message(2), assertGoodValue("Baz"));
+ testBody(this.test2Endpoint.message(0), assertGoodValue("Foo"));
+ testBody(this.test2Endpoint.message(1), assertGoodValue("Bar"));
+ // this client should get the extra message without value change
+ testBody(this.test2Endpoint.message(2), assertGoodValue("Bar"));
+ testBody(this.test2Endpoint.message(3), assertGoodValue("Baz"));
+
// assert
MockEndpoint.assertIsSatisfied(context, timeout,
TimeUnit.MILLISECONDS);
+
+ // The number of messages must be less than the number sent, because
from the 15 messages sent at last part,
+ // about 10 should be in one requestedPublishingInterval, everything
over 4 should be dropped
+ int count1 = this.test1Endpoint.getReceivedCounter();
+ assertTrue(count1 < 18, "No messages have been dropped, but should
because of queueSize");
+
+ // get the last exchange, this must be the last one, if messages are
dropped it must be some it between
Review Comment:
💬 **Typo:** `some it between` → `some in between`.
```suggestion
// get the last exchange, this must be the last one, if messages are
dropped it must be some in between
```
--
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]
