gnodet-bot commented on code in PR #29949:
URL: https://github.com/apache/camel/pull/29949#discussion_r4575019977
##########
dsl/camel-jbang/camel-jbang-plugin-tui/src/main/java/org/apache/camel/dsl/jbang/core/commands/tui/HealthHistory.java:
##########
@@ -31,6 +31,12 @@ final class HealthHistory {
static final byte UNKNOWN = -1;
static final byte READY = 1;
static final byte LIVE = 2;
+ /**
+ * The most seconds between two reads that say the same that are taken to
have said it too. The TUI reads about once
+ * a second while a diagram is on screen, a little over a second apart, so
now and then a second has no read of its
+ * own: a gap in the strip that was never a change.
+ */
Review Comment:
💬 **Javadoc — imprecise wording (low):** "The most seconds between two
reads" is ambiguous: `BRIDGED_SECONDS = 2` means at most **2 skipped seconds**
(i.e. reads up to 3 seconds apart are bridged), not reads that are at most 2
seconds apart. The phrase "between two reads" reads as the elapsed gap, not the
skipped-second count. The sentence is also passive and hard to parse ("that are
taken to have said it too").
```suggestion
/**
* Maximum skipped seconds between two reads that agree on health state:
a gap of this many
* unread seconds is filled with the agreed state rather than left as
UNKNOWN. The TUI reads
* about once a second while a diagram is on screen, a little over a
second apart, so now and
* then a second has no read of its own: a gap in the strip that was
never a change.
*/
```
##########
dsl/camel-jbang/camel-jbang-plugin-tui/src/test/java/org/apache/camel/dsl/jbang/core/commands/tui/HealthHistoryTest.java:
##########
@@ -0,0 +1,72 @@
+/*
+ * 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.dsl.jbang.core.commands.tui;
+
+import org.junit.jupiter.api.Test;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+/**
+ * Ready and live second by second: a second with no read of its own between
two reads that say the same is no gap in
+ * the strip, a longer time without reads is.
+ */
+class HealthHistoryTest {
+
+ private static final byte UP = HealthHistory.READY | HealthHistory.LIVE;
+
+ private final HealthHistory history = new HealthHistory();
+ private final IntegrationInfo info = new IntegrationInfo();
+
+ @Test
+ void aSecondSkippedBetweenTwoReadsThatSayTheSameIsNoGap() {
+ // read a little over a second apart, as while a diagram is on screen:
second 102 has no read of its own
+ read(100, true);
+ read(101, true);
+ read(103, true);
+
+ assertThat(history.strip("1", 103, 4)).containsExactly(UP, UP, UP, UP);
+ }
+
Review Comment:
💬 **Missing boundary test (low):** The PR description says "a second or two"
will be bridged (`BRIDGED_SECONDS = 2` → up to 2 skipped seconds). This test
only exercises 1 skipped second (reads at 100, 101, 103). There is no test for
exactly 2 skipped seconds (e.g. reads at 100, 103 → seconds 101 and 102
bridged), which is the boundary of the constant. Without it, a typo of
`BRIDGED_SECONDS = 1` would not be caught.
Consider adding:
```java
@Test
void twoSecondsBridgedBetweenTwoReadsThatSayTheSameIsNoGap() {
read(100, true);
read(103, true); // seconds 101 and 102 skipped — 2 skipped, within
BRIDGED_SECONDS
assertThat(history.strip("1", 103, 4)).containsExactly(UP, UP, UP, UP);
}
```
--
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]
