atiaomar1978-hub commented on code in PR #25514:
URL: https://github.com/apache/camel/pull/25514#discussion_r3792922248
##########
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:
**`--web` + `--record` interaction**
`applyRecording()` keys off process-wide `RecordingConfig.isEnabled()` /
`tamboui.record*` properties. When a user runs `camel tui --web
--record=foo.tape`, the local session sets those properties and browser
sessions created in `TuiWebServer.runSession()` still wrap their `AeshBackend`
here — even though those monitors have no tape path configured.
Consider one of:
- skip `applyRecording()` when the backend is a web session
(`CamelMonitor.webBackend != null`), or
- scope recording to the local session only (explicit flag instead of
JVM-wide properties), or
- reject/document `--web` together with `--record`.
A test covering `--web` + `--record` would lock the intended behaviour.
##########
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:
**Suggestion — test cleanup completeness**
`RecordingConfig.load()` reads `tamboui.record.fps` and
`tamboui.record.duration` as well. Consider clearing those properties in
`@AfterEach` alongside width/height so a future test ordering change cannot
leak recording config into unrelated tests.
##########
dsl/camel-jbang/camel-jbang-plugin-tui/src/main/java/org/apache/camel/dsl/jbang/core/commands/tui/CamelMonitor.java:
##########
@@ -184,6 +200,28 @@ public CamelMonitor(CamelJBangMain main, ClassLoader
classLoader) {
this.classLoader = classLoader;
}
+ /**
+ * Parses a {@code --record-size} value such as {@code 160x44} into {@code
[cols, rows]}.
+ */
+ int[] parseRecordSize(String size) {
Review Comment:
**Nice — validation upfront**
Rejecting malformed `--record-size` via `ParameterException` before any
recording starts is the right UX; the tests cover the important cases including
the historic `200x50` default.
##########
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 {
Review Comment:
**Good regression test**
This is exactly the test that was missing when `TuiBackendHelper` started
supplying an explicit backend: it proves recording wraps the delegate and
honours configured cast dimensions instead of silently exiting with no `.cast`
output.
--
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]
