ammachado commented on code in PR #25173: URL: https://github.com/apache/camel/pull/25173#discussion_r3677852777
########## 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: Added the explicit `atMost(5, TimeUnit.SECONDS)` to match the convention elsewhere in this test directory. On the second part — I checked, and the wrapper is still needed even though `renderThreadExecutor` (`Runnable::run`) is synchronous in this test: `onTabSelected()` dispatches the actual fetch through `ctx.backgroundExecutor`, which is a real `Executors.newCachedThreadPool` (see `MonitorContext.java:41`), not the synchronous one. So the render-side callback runs inline once triggered, but triggering it still crosses a real thread boundary — the Awaitility wait covers that hop. Kept it, just added the timeout. _Claude Code on behalf of Adriano Machado_ ########## 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: Traced this through `PopupManager.morePopupShortcut()` (lines 796-805) — duplicate mnemonics don't actually break navigation there. When two active tabs share a shortcut letter, the first press selects the first match and a second press of the same letter cycles to the next one (`matches.indexOf(lastShortcutIndex)`, advance, wrap). So JFR was reachable, just via a second `J` press rather than the first. For context, this pattern already exists at scale in this file — Q, C, M, S, R, P, H and B are all reused across 2+ entries (e.g. `S&QL Query`/`S&QL Trace`, or C across Consumers/Circuit Breaker/Classpath/Catalog/Configuration), so JFR/JDBC DataSource wouldn't have been the first duplicate. That said, we agreed a collision-free mnemonic is nicer UX here, so instead of just picking another already-used letter (F collides with Trans&formers, R collides with &Route Controller and &Recovery Tasks — every letter in "JFR" ties something), we expanded the label to "Java Fli&ght Recorder (JFR)" and used G, which isn't used anywhere else in the popup. Pushed in the latest commit. _Claude Code on behalf of Adriano Machado_ -- 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]
