Skip to content
Open
Original file line number Diff line number Diff line change
Expand Up @@ -1274,6 +1274,16 @@ public int run(CliArguments arguments) {
IdeLogLevel.INTERACTION.log(LOG, "For additional details run ide help {}", cmd == null ? "" : cmd.getName());
return 1;
} catch (Throwable t) {
if (cmd != null && cmd.isProcessableOutput()) {
// Processable output commandlets (auto-completion, env) write machine-consumed output to stdout. A failure
// there must not pollute that output with an error block and "file a bug" screen — so we record the failure
// (step.error still logs "Step ... ended with failure" for step tracking) and fail quietly instead of
// rethrowing, which would make Ideasy.run() log the error at ERROR level into the captured output.
step.error(t, true);
return 1;
}
// Do not activate logging for processable output commandlets (e.g. CompleteCommandlet) — errors would appear
// in the terminal as completion suggestions to the user.
activateLogging(cmd);
step.error(t, true);
if (this.logfile != null) {
Expand Down
6 changes: 5 additions & 1 deletion cli/src/main/java/com/devonfw/tools/ide/tool/uv/Uv.java
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,11 @@ public List<PythonUvListEntry> parsePythonListJson(List<String> jsonLines) {
public void setEnvironment(EnvironmentContext environmentContext, ToolInstallation toolInstallation, boolean additionalInstallation) {

super.setEnvironment(environmentContext, toolInstallation, additionalInstallation);
Path pythonPath = this.context.getSoftwarePath().resolve("python");
Path softwarePath = this.context.getSoftwarePath();
if (softwarePath == null) {
return;
}
Path pythonPath = softwarePath.resolve("python");
environmentContext.withEnvVar("UV_TOOL_DIR", pythonPath.resolve("tools").toString());
environmentContext.withEnvVar("UV_TOOL_BIN_DIR", pythonPath.resolve("bin").toString());
environmentContext.withPathEntry(pythonPath.resolve("bin"));
Expand Down
81 changes: 76 additions & 5 deletions cli/src/test/java/com/devonfw/tools/ide/cli/IdeasyTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,9 @@

import org.junit.jupiter.api.Test;

import com.devonfw.tools.ide.commandlet.Commandlet;
import com.devonfw.tools.ide.context.AbstractIdeContextTest;
import com.devonfw.tools.ide.context.IdeContext;
import com.devonfw.tools.ide.context.IdeTestContext;
import com.devonfw.tools.ide.version.IdeVersion;

Expand All @@ -34,6 +36,75 @@ void testEnvOutsideProjectDoesNotLogCliExitException() {
assertThat(context).log().hasNoEntryWithException();
}

/**
* Test that a {@link Commandlet#isProcessableOutput() processable-output} commandlet that throws inside {@link Commandlet#run() run} does not leak
* an ERROR-level error block ("An unexpected error occurred! … please file a bug") nor a "Logfile can be found at …" line into the captured log, while
* still marking the step as failed.
* <p>
* Regression test: rethrowing the exception made {@link Ideasy#run(String...)} log the error at ERROR level into the machine-consumed (auto-completion)
* output. The fix swallows the failure for processable-output commandlets instead of rethrowing it.
*/
@Test
void testProcessableOutputCommandletFailureDoesNotLogError() {

// arrange
IdeTestContext context = newContext(Path.of("/"));
context.addCommandlet(new ThrowingProcessableCommandlet(context));
Ideasy ideasy = new Ideasy(context);

// act
int exitCode = ideasy.run("throw");

// assert - the step is marked as failed
assertThat(context).logAtDebug().hasMessage("Step 'ide' ended with failure.");
// assert - no ERROR-level error block and no "Logfile can be found at" line leaked into the captured output
assertThat(exitCode).isNotEqualTo(0);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
assertThat(exitCode).isNotEqualTo(0);
assertThat(exitCode).isEqualTo(1);

assertThat(context).logAtError().hasNoMessageContaining("An unexpected error occurred");
assertThat(context).log().hasNoMessageContaining("An unexpected error occurred");
assertThat(context).log().hasNoMessageContaining("Logfile can be found at");
assertThat(context).log().hasNoEntryWithException();
}

/**
* A minimal {@link Commandlet} that produces processable output (like {@code complete}) but always fails, used to verify how a failure in such a
* commandlet is reported.
*/
private static final class ThrowingProcessableCommandlet extends Commandlet {

/**
* @param context the {@link IdeContext}.
*/
ThrowingProcessableCommandlet(IdeContext context) {

super(context);
addKeyword("throw");
}

@Override
public String getName() {

return "throw";
}

@Override
public boolean isIdeRootRequired() {

return false;
}

@Override
public boolean isProcessableOutput() {

return true;
}

@Override
protected void doRun() {

throw new IllegalStateException("boom");
}
}

/**
* Test of {@code ide --version}.
*/
Expand Down Expand Up @@ -83,7 +154,7 @@ public void testRunWithoutArgumentsDoesNotTriggerInstallation() {
String path = "project/workspaces/foo-test";
IdeTestContext context = newContext("environment", path, false);
Ideasy ideasy = new Ideasy(context);

// Take snapshot of software directory before running ide command
Path softwarePath = context.getSoftwarePath();
Set<String> existingToolsBefore = new HashSet<>();
Expand All @@ -94,7 +165,7 @@ public void testRunWithoutArgumentsDoesNotTriggerInstallation() {
fail("Failed to list software directory: " + e.getMessage());
}
}

// Take snapshot of _ide/software repository before running ide command
Path ideaSoftwarePath = context.getIdeRoot().resolve("_ide").resolve("software");
Set<String> existingIdeToolsBefore = new HashSet<>();
Expand All @@ -118,7 +189,7 @@ public void testRunWithoutArgumentsDoesNotTriggerInstallation() {
fail("Failed to list software directory after ide: " + e.getMessage());
}
}

Set<String> existingIdeToolsAfter = new HashSet<>();
if (Files.exists(ideaSoftwarePath)) {
try (var stream = Files.list(ideaSoftwarePath)) {
Expand All @@ -127,10 +198,10 @@ public void testRunWithoutArgumentsDoesNotTriggerInstallation() {
fail("Failed to list _ide/software directory after ide: " + e.getMessage());
}
}

// Verify no new tools were added to software directory
assertThat(existingToolsAfter).as("No new tools should be installed in software directory").isEqualTo(existingToolsBefore);

// Verify no new tools were added to _ide/software repository
assertThat(existingIdeToolsAfter).as("No new tools should be installed in _ide/software repository").isEqualTo(existingIdeToolsBefore);
}
Expand Down
25 changes: 25 additions & 0 deletions cli/src/test/java/com/devonfw/tools/ide/tool/uv/UvTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,31 @@ public void testSetEnvironment() {
assertThat(variables.get("UV_TOOL_BIN_DIR").getValue().replace('\\', '/')).endsWith("software/python/bin");
}

@Test
public void testSetEnvironmentWithNullSoftwarePath() {

// arrange — force getSoftwarePath() to return null to reproduce the condition of #2312
IdeTestContext context = new IdeTestContext() {
@Override
public Path getSoftwarePath() {
return null;
}
};
Uv uv = new Uv(context);
Path toolDir = Path.of("/software/uv");
ToolInstallation toolInstallation = new ToolInstallation(toolDir, toolDir, toolDir, VersionIdentifier.of("0.1.0"), true);
Map<String, VariableLine> variables = new HashMap<>();
EnvironmentVariableCollectorContext environmentContext = new EnvironmentVariableCollectorContext(variables,
new VariableSource(EnvironmentVariablesType.WORKSPACE, null), WindowsPathSyntax.MSYS);

// act — must not throw a NullPointerException when the software path is null
assertThatCode(() -> uv.setEnvironment(environmentContext, toolInstallation, false)).doesNotThrowAnyException();

// assert — the uv tool directories are not registered when the software path is null
assertThat(variables).doesNotContainKey("UV_TOOL_DIR");
assertThat(variables).doesNotContainKey("UV_TOOL_BIN_DIR");
}

@Test
public void testParsePythonListJson() {

Expand Down