ammachado commented on code in PR #25514: URL: https://github.com/apache/camel/pull/25514#discussion_r3792952308
########## docs/user-manual/modules/ROOT/pages/camel-jbang-tui.adoc: ########## @@ -892,6 +892,19 @@ https://asciinema.org/[Asciinema] `.cast` recording: camel tui --record=demo.tape ---- +The `.cast` file is written next to the tape, with the `.tape` suffix replaced by `.cast`. +Recording is headless: the TUI is driven entirely by the tape rather than by your terminal, +so no keyboard input is read and nothing is drawn on screen. + +The recorded terminal is 200x50 by default, which is wider than a documentation page can +display. Use `--record-size` to record at a size that fits, and `--record-fps` or +`--record-duration` to control the capture rate and the cut-off: + +[source,bash] +---- +camel tui --record=demo.tape --record-size=160x44 --record-fps=15 Review Comment: Fixed in 29bb3178a7d9. `TuiCommand` now declares `--record-size`, `--record-fps` and `--record-duration` and forwards them in the same non-default-only style as `--refresh` / `--mcp-port`. To keep the wiring from drifting again, `doCall()` was split so the argument list is built by a package-private `buildArgs()`, and `TuiCommandRecordOptionsTest` asserts both the forwarded command line and that `CamelMonitor` actually parses it. _AI-generated on behalf of @ammachado_ ########## dsl/camel-jbang/camel-jbang-plugin-tui/src/main/java/org/apache/camel/dsl/jbang/core/commands/tui/TuiBackendHelper.java: ########## @@ -35,10 +37,28 @@ static TuiRunner createTuiRunner() throws Exception { // classpath (for --web), auto-discovery can pick AeshBackend for the local session too, // which drives a native PosixSysTerminal that doesn't shut down cleanly here. JLineBackend backend = activeTerminal != null ? new JLineBackend(activeTerminal) : new JLineBackend(); - return TuiRunner.create(TuiConfig.builder().backend(backend).mouseCapture(true).build()); + return createTuiRunner(backend); } static TuiRunner createTuiRunner(Backend backend) throws Exception { - return TuiRunner.create(TuiConfig.builder().backend(backend).mouseCapture(true).build()); + return TuiRunner.create(TuiConfig.builder().backend(applyRecording(backend)).mouseCapture(true).build()); + } + + /** + * Wraps the backend for Asciinema recording when {@code --record} configured the {@code tamboui.record*} system + * properties. + * <p> + * TamboUI normally does this inside {@code BackendFactory.create()}, but {@code TuiRunner} only calls that factory + * when no explicit backend is configured. Because we must pass an explicit backend (see + * {@link #createTuiRunner()}), the wrapping has to be done here instead, otherwise {@code --record} exits cleanly + * without replaying the tape or writing a {@code .cast} file. + */ + static Backend applyRecording(Backend backend) { + // Guard on isEnabled() first: load() caches its result process-wide and installs a System.out capture + if (!RecordingConfig.isEnabled()) { + return backend; + } + RecordingConfig config = RecordingConfig.load(); + return config != null ? new RecordingBackend(backend, config) : backend; Review Comment: Fixed in 29bb3178a7d9, taking the "reject and document" option. `camel tui --record=... --web` now fails with a `ParameterException` from `CamelMonitor.configureRecording()`. Skipping `applyRecording()` for web backends would have left the local session recording into a cast file while browser sessions ran unrecorded in the same process, which is confusing rather than useful; the two modes are conceptually exclusive since `--record` drives a headless TUI from a tape rather than from a connected terminal. Rejecting up front also means the `tamboui.record*` properties are never set at all when `--web` is in play, so no other code path has to defend against them. Covered by `CamelMonitorRecordOptionsTest.rejectsRecordingCombinedWithTheWebTerminal`, documented in `camel-jbang-tui.adoc`, and noted in the 4.23 upgrade guide since the combination was previously accepted (silently doing nothing). _AI-generated on behalf of @ammachado_ ########## dsl/camel-jbang/camel-jbang-plugin-tui/src/main/java/org/apache/camel/dsl/jbang/core/commands/tui/CamelMonitor.java: ########## @@ -212,12 +250,13 @@ public Integer doCall() throws Exception { if (record != null) { Path tapeFile = Path.of(record); Path castFile = Path.of(record.replaceAll("\\.tape$", "") + ".cast"); + int[] size = parseRecordSize(recordSize); System.setProperty("tamboui.record", castFile.toAbsolutePath().toString()); System.setProperty("tamboui.record.config", tapeFile.toAbsolutePath().toString()); - System.setProperty("tamboui.record.width", "200"); - System.setProperty("tamboui.record.height", "50"); - System.setProperty("tamboui.record.duration", "120000"); - System.setProperty("tamboui.record.fps", "10"); + System.setProperty("tamboui.record.width", String.valueOf(size[0])); + System.setProperty("tamboui.record.height", String.valueOf(size[1])); + System.setProperty("tamboui.record.duration", String.valueOf(recordDuration)); + System.setProperty("tamboui.record.fps", String.valueOf(recordFps)); Review Comment: Fixed in 29bb3178a7d9. The six keys now live in a single `CamelMonitor.RECORD_PROPERTIES` constant, and the `finally` block clears them when this session is the one that set them (`record != null`). Worth noting for the record: clearing the properties does not lose the recording. `RecordingConfig` caches the loaded config in a static field and writes the cast file from a shutdown hook using that copy, so clearing only affects `RecordingConfig.isEnabled()` and therefore whether a *later* backend gets wrapped. `CamelMonitorRecordOptionsTest.configuringRecordingSetsEveryPropertyThatIsClearedAgainAfterwards` pins the set list and the clear list together, so a property added to `configureRecording()` but forgotten in the constant fails the build. _AI-generated on behalf of @ammachado_ ########## dsl/camel-jbang/camel-jbang-plugin-tui/src/test/java/org/apache/camel/dsl/jbang/core/commands/tui/TuiBackendHelperRecordingTest.java: ########## @@ -0,0 +1,162 @@ +/* + * 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.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; + +import dev.tamboui.buffer.DiffResult; +import dev.tamboui.internal.record.AnsiTerminalCapture; +import dev.tamboui.layout.Position; +import dev.tamboui.layout.Size; +import dev.tamboui.terminal.Backend; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; + +import static org.assertj.core.api.Assertions.assertThat; + +/** + * Verifies that {@code camel tui --record} actually engages TamboUI's recording backend. + * <p> + * TamboUI only wraps a backend for recording inside {@code BackendFactory.create()}, and {@code TuiRunner} calls that + * factory <em>only</em> when no explicit backend is configured. Since camel-tui must supply an explicit + * {@link dev.tamboui.backend.jline3.JLineBackend} (auto-discovery would otherwise pick the Aesh backend that is on the + * classpath for {@code --web}), the recording wrapper is never applied unless camel-tui applies it itself. When that + * wrapping is missing, {@code --record} replays no tape and writes no {@code .cast} file, yet still exits cleanly, so + * only a test like this one catches the regression. + */ +class TuiBackendHelperRecordingTest { + + @TempDir + Path tempDir; + + @AfterEach + void tearDown() { + // load() installs a System.out capture; restore the real stream so other tests are unaffected + if (AnsiTerminalCapture.isInstalled()) { + AnsiTerminalCapture.uninstall(); + } + System.clearProperty("tamboui.record"); + System.clearProperty("tamboui.record.config"); + System.clearProperty("tamboui.record.width"); + System.clearProperty("tamboui.record.height"); Review Comment: Fixed in 29bb3178a7d9, using junit-pioneer as suggested by @ammachado rather than extending the manual `@AfterEach`. The class is annotated with `@ClearSystemProperty` for all six keys, which clears them before each test and restores the original values afterwards. That covers values the test body sets itself (the paths depend on `@TempDir`, so `@SetSystemProperty` was not an option) and it cannot drift out of sync the way a hand-written teardown does. junit-pioneer 2.3.0 is already managed in `parent/pom.xml` and used by `camel-jbang-core`, so this is not a new dependency for the project. The same annotations were applied to `CamelMonitorRecordOptionsTest`, which now also touches those properties. The `@AfterEach` remains, reduced to uninstalling the `AnsiTerminalCapture` that `RecordingConfig.load()` installs. _AI-generated on behalf of @ammachado_ -- 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]
