gnodet-bot commented on code in PR #29514:
URL: https://github.com/apache/camel/pull/29514#discussion_r4518429827
##########
dsl/camel-jbang/camel-jbang-core/src/main/java/org/apache/camel/dsl/jbang/core/common/ChaosExperimentRun.java:
##########
@@ -170,13 +187,32 @@ static JsonObject run(
answer.put("traffic", traffic);
}
answer.put("waited", waited);
+ if (warnings != null && !warnings.isEmpty()) {
+ answer.put("warnings", warnings);
+ }
JsonObject exploration = exploration(integration.status());
if (exploration != null && exploration.containsKey("timeLeft")) {
answer.put("timeLeft", exploration.getLong("timeLeft"));
}
return answer;
}
+ /** The longest within= of the rules, in millis, or 0 (an expectation
without it has the time of its rule). */
+ static long longestWithin(String rules) {
+ long answer = 0;
+ Matcher m = WITHIN.matcher(rules);
+ while (m.find()) {
+ try {
+ answer = Math.max(answer,
TimeUtils.toMilliSeconds(m.group(1)));
+ } catch (RuntimeException e) {
+ // not a time: camel-chaos refuses the rule with the reason
+ }
+ }
+ return answer;
+ }
+
+ private static final Pattern WITHIN =
Pattern.compile("(?:^|\\s)within=(\\S+)");
Review Comment:
⚠️ **Bug: `\S+` captures trailing `;` in semicolon-separated rules.**
`\S+` matches any non-whitespace including `;`. When rules look like
`within=90s; route=...`, the captured group is `"90s;"`.
`TimeUtils.toMilliSeconds("90s;")` throws, the catch swallows it silently, and
that `within=` value is lost — yielding a shorter (or zero) wait and an
INCONCLUSIVE verdict.
The existing test passes despite this because `1m;` → throws, `90s` → 90 000
ms, and 90 s is still the max. A case where the semicoloned value is the
longest would return the wrong result.
```suggestion
private static final Pattern WITHIN =
Pattern.compile("(?:^|\\s)within=([^;\\s]+)");
```
##########
dsl/camel-jbang/camel-jbang-core/src/main/java/org/apache/camel/dsl/jbang/core/commands/ChaosExplore.java:
##########
@@ -147,20 +160,40 @@ public Integer doCall() throws Exception {
// the AI may have finished it already; else its rules go now
ActionClient.execute(process, "chaos", root -> root.put("command",
"explore-finish"), 10000);
}
- JsonObject exploration =
ChaosExperimentRun.exploration(RuntimeHelper.readStatus(process.pid()));
+ JsonObject status = RuntimeHelper.readStatus(process.pid());
+ JsonObject exploration = ChaosExperimentRun.exploration(status);
List<String> lines = journal(exploration);
printer().println();
lines.forEach(printer()::println);
+ if (save && exploration != null) {
+ Saved saved = save(Path.of("test", "chaos"), LocalDateTime.now(),
+ new Run(integration, budget, focus, ask.usedModel,
seed(status)), exploration, ask.lastAnswer,
+ lines);
+ printer().println();
+ printer().println("Report: " + saved.report());
+ saved.scenarios().forEach(s -> printer().println("Scenario: " + s
+ " (replay: camel run * --chaos=" + s + ")"));
+ }
if (failed(exploration)) {
return CliExitCode.FAILURE;
}
return answer;
}
+ /** The name of the integration as camel get chaos has it: its
CamelContext, else the name of the process. */
+ static String displayName(CamelProcess process) {
+ if (process.status() != null && process.status().get("context")
instanceof JsonObject context) {
Review Comment:
💡 **Nit: `process.status()` is called twice on the same line.** Assign to a
local first to avoid the double call (and to make the null-check and the access
obviously on the same object):
```suggestion
var status = process.status();
if (status != null && status.get("context") instanceof JsonObject
context) {
```
--
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]
