gnodet-bot commented on code in PR #27201:
URL: https://github.com/apache/camel/pull/27201#discussion_r4218373577
##########
components/camel-milo/src/test/java/org/apache/camel/component/milo/MonitorItemTest.java:
##########
@@ -45,15 +53,55 @@ public class MonitorItemTest extends AbstractMiloServerTest
{
private static final String MILO_CLIENT_ITEM_C1_1 =
"milo-client:opc.tcp://foo:bar@localhost:@@port@@?node="
+
NodeIds.nodeValue(MiloServerComponent.DEFAULT_NAMESPACE_URI,
"myitem1")
+ +
"&monitorFilterType=dataChangeFilter&dataChangeFilterTrigger=StatusValue&dataChangeFilterDeadbandType=0"
+ +
"&requestedPublishingInterval=2000&samplingInterval=100&queueSize=4"
+ +
"&allowedSecurityPolicies=None&overrideHost=true";
+
+ // second client endpoint with modified dataChangeFilterTrigger to verify
that they are inmdependent on same client
Review Comment:
💬 **Typo:** `inmdependent` → `independent`
```suggestion
// second client endpoint with modified dataChangeFilterTrigger to
verify that they are independent on same client
```
##########
components/camel-milo/src/test/java/org/apache/camel/component/milo/MonitorItemTest.java:
##########
@@ -65,10 +113,18 @@ public void configure() {
from(DIRECT_START_1).to(MILO_SERVER_ITEM_1);
from(resolve(MILO_CLIENT_ITEM_C1_1)).to(MOCK_TEST_1);
+ from(resolve(MILO_CLIENT_ITEM_C1_2)).to(MOCK_TEST_2);
+ from(resolve(MILO_CLIENT_ITEM_C1_3)).to(MOCK_TEST_3);
+ from(resolve(MILO_CLIENT_ITEM_C2_1)).to(MOCK_TEST_4);
}
};
}
+ private static @NonNull Processor getLogProcessor(String prefix) {
Review Comment:
⚠️ **Dead code:** `getLogProcessor()` is defined but never called. The
`configure()` routes only use `.to()` — no `.process()` calls anywhere. Either
wire it in where intended, or remove it.
##########
components/camel-milo/src/test/java/org/apache/camel/component/milo/MonitorItemTest.java:
##########
@@ -78,20 +134,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 3,
plus at least 8 more from rest (if they fall to 2 periods)
Review Comment:
💬 **Inaccurate comment:** The inline comment says "the first 3" but
`test2Endpoint` receives 4 initial messages: `Foo(0)`, `Bar(1)`, `Bar(2)` (dup,
caught by `StatusValueTimestamp`), and `Baz(3)`. The minimum count of 12 = 4+8
is correct, but the comment's breakdown is wrong.
```suggestion
test2Endpoint.setMinimumExpectedMessageCount(12); // the first 4,
plus at least 8 more from rest (if they fall to 2 periods)
```
##########
components/camel-milo/src/test/java/org/apache/camel/component/milo/MonitorItemTest.java:
##########
@@ -102,23 +201,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 catched on MILO_CLIENT_ITEM_C1_2 because
of dataChangeFilterTrigger=StatusValueTimestamp
Review Comment:
💬 **Grammar:** `catched` → `caught`
```suggestion
// This update will only be caught on MILO_CLIENT_ITEM_C1_2 because
of dataChangeFilterTrigger=StatusValueTimestamp
```
--
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]
