gnodet commented on code in PR #25173:
URL: https://github.com/apache/camel/pull/25173#discussion_r3677695568
##########
dsl/camel-jbang/camel-jbang-core/src/main/java/org/apache/camel/dsl/jbang/core/commands/Run.java:
##########
@@ -1344,6 +1354,49 @@ protected void addDependencies(String... deps) {
dependencies.addAll(depsList);
}
+ boolean jfrEnabled() {
+ return debugOptions.jfr || debugOptions.jfrProfile != null;
+ }
+
+ String jfrFileName() {
+ return (name != null ? name : "camel") + ".jfr";
+ }
+
+ /**
+ * Builds the JFR JVM arguments for runtimes that fork a subprocess
(Quarkus, Spring Boot), or {@code null} when JFR
+ * was not requested.
+ */
+ String buildJfrJvmArgs() {
+ if (!jfrEnabled()) {
+ return null;
+ }
+ StringBuilder arg = new StringBuilder();
+ if (jvmArgs != null && jvmArgs.contains("-XX:StartFlightRecording")) {
+ // the explicit JVM argument wins, as two recordings would
otherwise compete for the same file
Review Comment:
The warning says "Ignoring --jfr" but the method still appends
`-Dcamel.main.startupRecorderRuntimeEnabled=true` unconditionally, meaning
Camel runtime instrumentation IS still enabled. An operator reading "Ignoring
--jfr" would reasonably expect nothing JFR-related to be active.
Consider rewording to clarify that only the `-XX:StartFlightRecording` JVM
flag is being skipped:
```suggestion
printer().printErr("WARN: --jvm-args already starts a flight
recording, skipping -XX:StartFlightRecording from --jfr (Camel runtime
instrumentation is still enabled)");
```
##########
dsl/camel-jbang/camel-jbang-plugin-tui/src/test/java/org/apache/camel/dsl/jbang/core/commands/tui/JfrTabRenderTest.java:
##########
@@ -0,0 +1,129 @@
+/*
+ * 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 java.util.ArrayList;
+import java.util.List;
+import java.util.concurrent.atomic.AtomicReference;
+
+import dev.tamboui.text.Span;
+import org.apache.camel.util.json.JsonObject;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.awaitility.Awaitility.await;
+
+/**
+ * Rendering tests for {@link JfrTab}. These tests render the tab into a
virtual terminal buffer via
+ * {@link Frame#forTesting(Buffer)} and inspect the rendered cell content.
+ */
+class JfrTabRenderTest {
+
+ private MonitorContext ctx;
+ private IntegrationInfo info;
+
+ @BeforeEach
+ void setUp() {
+ Theme.resetForTesting();
+ info = new IntegrationInfo();
+ info.pid = "1234";
+ info.name = "test-app";
+
+ AtomicReference<List<IntegrationInfo>> data = new
AtomicReference<>(List.of(info));
+ AtomicReference<List<InfraInfo>> infraData = new
AtomicReference<>(List.of());
+ ctx = new MonitorContext(data, infraData);
+ ctx.selectedPid = "1234";
+ }
+
+ @Test
+ void renderNoSelectionShowsPrompt() {
+ ctx.selectedPid = null;
+ JfrTab tab = new JfrTab(ctx);
+ String rendered = TuiTestHelper.renderToString(tab, 120, 20);
+ assertThat(rendered).containsAnyOf("No integration selected", "Select
an integration");
+ }
+
+ @Test
+ void renderShowsBlockTitle() {
+ JfrTab tab = new JfrTab(ctx);
+ String rendered = TuiTestHelper.renderToString(tab, 120, 20);
+ assertThat(rendered).contains("JFR");
+ }
+
+ @Test
+ void renderShowsRegisteredAndRecordingState() {
+ JfrTab tab = new JfrTab(ctx);
+ String rendered = TuiTestHelper.renderToString(tab, 120, 20);
+ assertThat(rendered).contains("registered").contains("no active
recording");
+ }
+
+ @Test
+ void renderShowsStatusErrorFromIntegration() {
+ TestMonitorContext errorContext = new
TestMonitorContext(dataWith(info), errorResponse());
+ errorContext.selectedPid = "1234";
+ JfrTab tab = new JfrTab(errorContext, Runnable::run);
+
+ tab.onTabSelected();
+
+ await().untilAsserted(() ->
assertThat(TuiTestHelper.renderToString(tab, 120, 20))
Review Comment:
`await().untilAsserted(...)` is used without an explicit `.atMost()`
timeout. Per project conventions: "Always set an explicit atMost timeout to
avoid hanging builds." Every other Awaitility usage in the TUI test directory
uses `.atMost(5, TimeUnit.SECONDS)` or `.atMost(10, TimeUnit.SECONDS)`.
Also, since the test uses `Runnable::run` as the executor (synchronous), the
Awaitility wrapper may be unnecessary — a plain `assertThat()` after
`onTabSelected()` would likely suffice.
##########
dsl/camel-jbang/camel-jbang-plugin-tui/src/main/java/org/apache/camel/dsl/jbang/core/commands/tui/TabRegistry.java:
##########
@@ -231,6 +233,9 @@ void initTabs(MonitorContext ctx, DataRefreshService
dataService, Runnable reset
// JVM
new MoreTab(TuiIcons.TAB_CLASSPATH, "Classpath", "&Classpath",
classpathTab, "JVM"),
new MoreTab(TuiIcons.TAB_HEAP, "Heap Memory Histogram", "Heap
&Memory Histogram", heapHistogramTab, "JVM"),
+ new MoreTab(
+ TuiIcons.TAB_JFR, "JFR", "&JFR", jfrTab, "JVM",
Review Comment:
The JFR tab uses mnemonic `&JFR` (key `J`), which duplicates the existing
`&JDBC DataSource` at line 217. Unlike what might be expected, all 23 existing
mnemonics in this file are unique — this would be the first duplication. The
`morePopupShortcut()` handler in PopupManager iterates all tabs globally and
returns the first match, so pressing `J` would always select JDBC DataSource,
making the JFR tab unreachable via its mnemonic. Consider using a different
mnemonic (e.g., `J&FR` for `F`, or `JF&R` for `R`).
--
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]
