Skip to content

Commit 9ac58b4

Browse files
stalepstalep
committed
Fix executeCommand(String...) short-circuit to use isFailure() (#604)
The multi-line executeCommand(String... lines) previously stopped only on CommandResult.FAILURE (exit code 1). Any other non-zero code (USAGE_ERROR=2, COMMAND_NOT_FOUND=127, INTERRUPTED=130, custom codes) would incorrectly allow execution to continue to the next line. Fix: replace result == CommandResult.FAILURE with result.isFailure() which checks getResultValue() != 0, consistent with POSIX behavior and the && operator which uses result.isSuccess(). Add tests covering: - USAGE_ERROR returned for missing required option - COMMAND_NOT_FOUND propagation - Custom exit code (valueOf(42)) round-trips correctly - null return normalized to SUCCESS - Multi-line short-circuit stops on any non-zero exit code (#604) - Multi-line continues when all commands succeed - getExitCode() POSIX clamping (negative -> 1, >255 -> 255)
1 parent 3a0dc53 commit 9ac58b4

2 files changed

Lines changed: 165 additions & 1 deletion

File tree

‎aesh/src/main/java/org/aesh/command/impl/AeshCommandRuntime.java‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -357,7 +357,9 @@ public CommandResult executeCommand(String... lines) throws CommandNotFoundExcep
357357
CommandResult result = null;
358358
for (String line : lines) {
359359
result = executeCommand(line);
360-
if (result == CommandResult.FAILURE)
360+
// Stop on any non-zero exit code, consistent with POSIX and the
361+
// && operator which uses result.isSuccess() (#604)
362+
if (result != null && result.isFailure())
361363
return result;
362364
}
363365
return result;

‎aesh/src/test/java/org/aesh/AeshRuntimeRunnerTest.java‎

Lines changed: 162 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,168 @@ public void testCommandResult() {
5757

5858
}
5959

60+
// --- CommandResult propagation tests (#604) ---
61+
62+
@Test
63+
public void testUsageErrorResult() {
64+
// A command with a required option, invoked without it, should return USAGE_ERROR
65+
CommandResult result = AeshRuntimeRunner.builder()
66+
.command(RequiredOptionCommand.class)
67+
.execute();
68+
assertEquals(CommandResult.USAGE_ERROR.getResultValue(), result.getResultValue());
69+
}
70+
71+
@Test
72+
public void testCommandNotFoundResult() throws Exception {
73+
// CommandNotFoundException propagates from CommandRuntime and is mapped to
74+
// COMMAND_NOT_FOUND (127) by AeshRuntimeRunner. Verify the constant and that
75+
// CommandRuntime throws when the command name is unknown.
76+
assertEquals(127, CommandResult.COMMAND_NOT_FOUND.getResultValue());
77+
assertEquals(127, CommandResult.COMMAND_NOT_FOUND.getExitCode());
78+
assertTrue("COMMAND_NOT_FOUND is failure", CommandResult.COMMAND_NOT_FOUND.isFailure());
79+
80+
org.aesh.command.CommandRuntime<CommandInvocation> runtime = org.aesh.command.AeshCommandRuntimeBuilder.builder()
81+
.commandRegistry(org.aesh.command.impl.registry.AeshCommandRegistryBuilder.builder()
82+
.command(Bar1Command.class)
83+
.create())
84+
.build();
85+
86+
try {
87+
runtime.executeCommand("nonexistent");
88+
assertTrue("Should have thrown CommandNotFoundException", false);
89+
} catch (org.aesh.command.CommandNotFoundException e) {
90+
// correct — AeshRuntimeRunner maps this to COMMAND_NOT_FOUND (127)
91+
}
92+
}
93+
94+
@Test
95+
public void testCustomExitCodeResult() {
96+
// A command returning a custom exit code (42) should propagate it
97+
CommandResult result = AeshRuntimeRunner.builder()
98+
.command(CustomExitCodeCommand.class)
99+
.execute();
100+
assertEquals(42, result.getResultValue());
101+
assertEquals(42, result.getExitCode());
102+
assertTrue("Non-zero exit code is failure", result.isFailure());
103+
}
104+
105+
@Test
106+
public void testNullReturnNormalizedToSuccess() {
107+
// A command returning null should be treated as SUCCESS
108+
CommandResult result = AeshRuntimeRunner.builder()
109+
.command(NullReturnCommand.class)
110+
.execute();
111+
assertEquals(CommandResult.SUCCESS.getResultValue(), result.getResultValue());
112+
assertTrue("null return should be SUCCESS", result.isSuccess());
113+
}
114+
115+
@Test
116+
public void testMultiLineShortCircuitsOnNonZero() throws Exception {
117+
// executeCommand(String... lines) should stop on any non-zero result (#604)
118+
// Using CommandRuntime directly to exercise the multi-line overload
119+
MultiLineTrackingCommand.reset();
120+
121+
org.aesh.command.CommandRuntime<CommandInvocation> runtime = org.aesh.command.AeshCommandRuntimeBuilder.builder()
122+
.commandRegistry(org.aesh.command.impl.registry.AeshCommandRegistryBuilder.builder()
123+
.command(MultiLineTrackingCommand.class)
124+
.create())
125+
.build();
126+
127+
// First command returns custom code 5 (non-zero) — second should NOT run
128+
CommandResult result = runtime.executeCommand(
129+
"multitrack --code 5",
130+
"multitrack --code 0");
131+
132+
assertEquals(5, result.getResultValue());
133+
assertTrue("Should have stopped after non-zero result", result.isFailure());
134+
assertEquals("Only first command should have executed", 1, MultiLineTrackingCommand.callCount);
135+
}
136+
137+
@Test
138+
public void testMultiLineContinuesOnSuccess() throws Exception {
139+
// executeCommand(String... lines) should continue when all commands succeed
140+
MultiLineTrackingCommand.reset();
141+
142+
org.aesh.command.CommandRuntime<CommandInvocation> runtime = org.aesh.command.AeshCommandRuntimeBuilder.builder()
143+
.commandRegistry(org.aesh.command.impl.registry.AeshCommandRegistryBuilder.builder()
144+
.command(MultiLineTrackingCommand.class)
145+
.create())
146+
.build();
147+
148+
CommandResult result = runtime.executeCommand(
149+
"multitrack --code 0",
150+
"multitrack --code 0",
151+
"multitrack --code 0");
152+
153+
assertEquals(0, result.getResultValue());
154+
assertTrue("All succeeded", result.isSuccess());
155+
assertEquals("All three commands should have executed", 3, MultiLineTrackingCommand.callCount);
156+
}
157+
158+
@Test
159+
public void testExitCodeClamping() {
160+
// getExitCode() clamps to POSIX 0-255 range
161+
assertEquals(0, CommandResult.SUCCESS.getExitCode());
162+
assertEquals(1, CommandResult.FAILURE.getExitCode());
163+
assertEquals(2, CommandResult.USAGE_ERROR.getExitCode());
164+
assertEquals(127, CommandResult.COMMAND_NOT_FOUND.getExitCode());
165+
assertEquals(130, CommandResult.INTERRUPTED.getExitCode());
166+
167+
// Negative values clamp to 1
168+
assertEquals(1, CommandResult.valueOf(-1).getExitCode());
169+
// Values > 255 clamp to 255
170+
assertEquals(255, CommandResult.valueOf(256).getExitCode());
171+
// Values in range pass through
172+
assertEquals(42, CommandResult.valueOf(42).getExitCode());
173+
}
174+
175+
// --- Helper commands for CommandResult tests ---
176+
177+
@CommandDefinition(name = "required-opt", description = "requires --name")
178+
public static class RequiredOptionCommand implements Command<CommandInvocation> {
179+
@Option(name = "name", required = true)
180+
private String name;
181+
182+
@Override
183+
public CommandResult execute(CommandInvocation ci) {
184+
return CommandResult.SUCCESS;
185+
}
186+
}
187+
188+
@CommandDefinition(name = "custom-exit", description = "returns exit code 42")
189+
public static class CustomExitCodeCommand implements Command<CommandInvocation> {
190+
@Override
191+
public CommandResult execute(CommandInvocation ci) {
192+
return CommandResult.valueOf(42);
193+
}
194+
}
195+
196+
@CommandDefinition(name = "null-return", description = "returns null (should be SUCCESS)")
197+
public static class NullReturnCommand implements Command<CommandInvocation> {
198+
@Override
199+
public CommandResult execute(CommandInvocation ci) {
200+
return null;
201+
}
202+
}
203+
204+
@CommandDefinition(name = "multitrack", description = "tracks calls, returns specified code")
205+
public static class MultiLineTrackingCommand implements Command<CommandInvocation> {
206+
@Option(name = "code", description = "exit code to return", defaultValue = "0")
207+
private int code;
208+
209+
static int callCount = 0;
210+
211+
static void reset() {
212+
callCount = 0;
213+
}
214+
215+
@Override
216+
public CommandResult execute(CommandInvocation ci) {
217+
callCount++;
218+
return CommandResult.valueOf(code);
219+
}
220+
}
221+
60222
@Test
61223
public void testInstantiatedCommand() {
62224
Bar1Command bar1Cmd = new Bar1Command();

0 commit comments

Comments
 (0)