Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

#608: enhance error messages #653

Merged
Merged
Show file tree
Hide file tree
Changes from 24 commits
Commits
Show all changes
37 commits
Select commit Hold shift + click to select a range
7bbea6e
#608: enhanced error messages
jan-vcapgemini Sep 11, 2024
131933a
Merge branch 'main' into feature/608-enhance-error-messages
jan-vcapgemini Sep 16, 2024
c3a6b1c
Merge branch 'main' into feature/608-enhance-error-messages
jan-vcapgemini Sep 19, 2024
68a695e
Merge branch 'main' into feature/608-enhance-error-messages
jan-vcapgemini Sep 23, 2024
19b17d0
#608: implemented requested changes
jan-vcapgemini Sep 23, 2024
7423297
Merge branch 'main' into feature/608-enhance-error-messages
jan-vcapgemini Sep 25, 2024
91d64ab
#608: Adjusted out and error format
jan-vcapgemini Sep 25, 2024
867e62e
#608: added execute permissions to dotnet
jan-vcapgemini Sep 25, 2024
a952d83
Merge branch 'main' into feature/608-enhance-error-messages
jan-vcapgemini Sep 27, 2024
707d300
Merge branch 'main' into feature/608-enhance-error-messages
jan-vcapgemini Sep 30, 2024
8b60102
#608: applied refactorings and fixes
jan-vcapgemini Oct 4, 2024
8aa538a
Merge branch 'main' into feature/608-enhance-error-messages
jan-vcapgemini Oct 4, 2024
7e89d36
#608: fixed test
jan-vcapgemini Oct 4, 2024
b786464
#608: fixed test
jan-vcapgemini Oct 4, 2024
11dfda2
#608: optimized Dotnet test
jan-vcapgemini Oct 7, 2024
88b1475
#608: fixed tests
jan-vcapgemini Oct 7, 2024
20bd7c6
#608: fixed Dotnet windows test
jan-vcapgemini Oct 7, 2024
7a0433a
#608: fixed tests
jan-vcapgemini Oct 8, 2024
4113983
Merge branch 'main' into feature/608-enhance-error-messages
jan-vcapgemini Oct 14, 2024
bbb4c89
#608: fixed tests
jan-vcapgemini Oct 15, 2024
5d7f6af
#608: fixed tests
jan-vcapgemini Oct 15, 2024
0380559
Merge branch 'main' into feature/608-enhance-error-messages
jan-vcapgemini Oct 15, 2024
6cacf03
Merge branch 'main' into feature/608-enhance-error-messages
jan-vcapgemini Oct 15, 2024
b1ef58e
#608: disabled tests
jan-vcapgemini Oct 17, 2024
e516604
Merge branch 'main' into feature/608-enhance-error-messages
jan-vcapgemini Oct 21, 2024
42f59f5
#608: implemented requested changes
jan-vcapgemini Oct 21, 2024
1e4b116
#608: fixed test
jan-vcapgemini Oct 21, 2024
68770c7
#608: implemented requested changes
jan-vcapgemini Oct 24, 2024
0a3e586
Merge branch 'main' into feature/608-enhance-error-messages
jan-vcapgemini Oct 24, 2024
39400c5
#608: added changes to changelog
jan-vcapgemini Oct 24, 2024
858ffca
#608: fixed tests
jan-vcapgemini Oct 28, 2024
3f1b8ef
Merge branch 'main' into feature/608-enhance-error-messages
jan-vcapgemini Oct 28, 2024
7f65aee
Merge branch 'main' into feature/608-enhance-error-messages
jan-vcapgemini Oct 29, 2024
c39829e
Update cli/src/test/java/com/devonfw/tools/ide/context/ProcessContext…
jan-vcapgemini Oct 30, 2024
c7f2c55
#608: implemented requested changes
jan-vcapgemini Oct 30, 2024
0603a20
Merge branch 'main' into feature/608-enhance-error-messages
jan-vcapgemini Oct 30, 2024
e95034a
#608: moved changelog entry to new version
jan-vcapgemini Oct 30, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
import com.devonfw.tools.ide.common.SystemPath;
import com.devonfw.tools.ide.context.IdeContext;
import com.devonfw.tools.ide.environment.VariableLine;
import com.devonfw.tools.ide.log.IdeLogLevel;
import com.devonfw.tools.ide.log.IdeSubLogger;
import com.devonfw.tools.ide.os.SystemInfoImpl;
import com.devonfw.tools.ide.os.WindowsPathSyntax;
Expand Down Expand Up @@ -180,13 +181,15 @@ public ProcessResult run(ProcessMode processMode) {
}

int exitCode;

if (processMode.isBackground()) {
exitCode = ProcessResult.SUCCESS;
} else {
exitCode = process.waitFor();
}

ProcessResult result = new ProcessResultImpl(exitCode, out, err);

performLogOnError(result, exitCode, interpreter);

return result;
Expand Down Expand Up @@ -313,22 +316,34 @@ private String addExecutable(String exec, List<String> args) {
private void performLogOnError(ProcessResult result, int exitCode, String interpreter) {

if (!result.isSuccessful() && (this.errorHandling != ProcessErrorHandling.NONE)) {
String message = createCommandMessage(interpreter, " failed with exit code " + exitCode + "!");
IdeSubLogger subLogger = this.context.error();
IdeLogLevel ideLogLevel;
String message = createCommandMessage(interpreter, "\nfailed with exit code " + exitCode + "!");

if (this.errorHandling == ProcessErrorHandling.THROW_CLI) {
subLogger.log(message);
hohwille marked this conversation as resolved.
Show resolved Hide resolved
result.log(IdeLogLevel.ERROR, context);
throw new CliProcessException(message, result);
} else if (this.errorHandling == ProcessErrorHandling.THROW_ERR) {
subLogger.log(message);
result.log(IdeLogLevel.ERROR, context);
throw new IllegalStateException(message);
}
hohwille marked this conversation as resolved.
Show resolved Hide resolved
IdeSubLogger level;

if (this.errorHandling == ProcessErrorHandling.LOG_ERROR) {
level = this.context.error();
ideLogLevel = IdeLogLevel.ERROR;
subLogger = this.context.error();
} else if (this.errorHandling == ProcessErrorHandling.LOG_WARNING) {
level = this.context.warning();
ideLogLevel = IdeLogLevel.WARNING;
subLogger = this.context.warning();
} else {
level = this.context.error();
ideLogLevel = IdeLogLevel.ERROR;
subLogger = this.context.error();
this.context.error("Internal error: Undefined error handling {}", this.errorHandling);
}
level.log(message);

subLogger.log(message);
hohwille marked this conversation as resolved.
Show resolved Hide resolved
result.log(ideLogLevel, context);
hohwille marked this conversation as resolved.
Show resolved Hide resolved
}
}

Expand Down
10 changes: 10 additions & 0 deletions cli/src/main/java/com/devonfw/tools/ide/process/ProcessResult.java
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,9 @@

import java.util.List;

import com.devonfw.tools.ide.context.IdeContext;
import com.devonfw.tools.ide.log.IdeLogLevel;

/**
* Result of a {@link Process} execution.
*
Expand Down Expand Up @@ -58,4 +61,11 @@ default boolean isSuccessful() {
*/
List<String> getErr();

/**
* Logs output and error messages on the provided log level.
*
* @param level the {@link IdeLogLevel} to use e.g. IdeLogLevel.ERROR.
* @param context the {@link IdeContext} to use.
*/
void log(IdeLogLevel level, IdeContext context);
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,9 @@
import java.util.Collections;
import java.util.List;

import com.devonfw.tools.ide.context.IdeContext;
import com.devonfw.tools.ide.log.IdeLogLevel;

/**
* Implementation of {@link ProcessResult}.
*/
Expand Down Expand Up @@ -55,4 +58,25 @@ public List<String> getErr() {
return this.err;
}

@Override
public void log(IdeLogLevel level, IdeContext context) {

if (this.out != null && !this.out.isEmpty() && level == IdeLogLevel.INFO) {
doLog(level, this.out, context);
}
if (this.err != null && !this.err.isEmpty() && level == IdeLogLevel.ERROR) {
doLog(level, this.err, context);
}
hohwille marked this conversation as resolved.
Show resolved Hide resolved
}

private void doLog(IdeLogLevel level, List<String> lines, IdeContext context) {
for (String line : lines) {
// remove !MESSAGE from log message
if (line.startsWith("!MESSAGE ")) {
line = line.substring(9);
}
context.level(level).log(line);
}
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,10 @@
import com.devonfw.tools.ide.context.IdeContext;
import com.devonfw.tools.ide.tool.LocalToolCommandlet;

/**
* {@link LocalToolCommandlet} for <a href="https://docs.microsoft.com/en-us/dotnet/core/tools/">dotnet</a>. The .NET CLI (Command Line Interface)
* cross-platform tool for building, running, and managing .NET applications.
*/
public class DotNet extends LocalToolCommandlet {

/**
Expand Down
15 changes: 2 additions & 13 deletions cli/src/main/java/com/devonfw/tools/ide/tool/eclipse/Eclipse.java
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
import java.nio.channels.FileLock;
import java.nio.file.Files;
import java.nio.file.Path;
import java.util.List;
import java.util.Set;

import com.devonfw.tools.ide.cli.CliException;
Expand Down Expand Up @@ -84,21 +83,11 @@ public void installPlugin(ToolPluginDescriptor plugin, Step step) {
}
}
}
log(IdeLogLevel.DEBUG, result.getOut());
log(IdeLogLevel.ERROR, result.getErr());
result.log(IdeLogLevel.DEBUG, context);
result.log(IdeLogLevel.ERROR, context);
hohwille marked this conversation as resolved.
Show resolved Hide resolved
step.error("Failed to install plugin {} ({}): exit code was {}", plugin.name(), plugin.id(), result.getExitCode());
}

private void log(IdeLogLevel level, List<String> lines) {

for (String line : lines) {
if (line.startsWith("!MESSAGE ")) {
line = line.substring(9);
}
this.context.level(level).log(line);
}
}

@Override
protected void configureWorkspace() {

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,13 @@
import org.junit.jupiter.params.provider.EnumSource;
import org.junit.platform.commons.util.ReflectionUtils;

import com.devonfw.tools.ide.cli.CliProcessException;
import com.devonfw.tools.ide.context.AbstractIdeContextTest;
import com.devonfw.tools.ide.context.IdeTestContext;
import com.devonfw.tools.ide.log.IdeLogLevel;
import com.devonfw.tools.ide.os.SystemInfo;
import com.devonfw.tools.ide.os.SystemInfoMock;
import com.devonfw.tools.ide.tool.dotnet.DotNet;

/**
* Unit tests of {@link ProcessContextImpl}.
Expand Down Expand Up @@ -183,7 +187,7 @@ public void unsuccessfulProcessShouldThrowIllegalState() throws Exception {

// act & assert
assertThrows(IllegalStateException.class, () -> {
this.processConttextUnderTest.run(ProcessMode.DEFAULT);
this.processConttextUnderTest.run(ProcessMode.DEFAULT_CAPTURE);
});

}
Expand All @@ -204,6 +208,24 @@ public void processWarningAndErrorShouldBeLogged(ProcessErrorHandling processErr
assertThat(this.context).log(level).hasMessageContaining(expectedMessage);
}

@Test
public void enablingCaptureShouldRedirectAndCaptureStreamsWithErrorsCorrectly() {
// arrange
IdeTestContext context = newContext("processcontext");
SystemInfo systemInfo = SystemInfoMock.of("linux");
context.setSystemInfo(systemInfo);

// act
CliProcessException thrown = assertThrows(CliProcessException.class, () -> context.getCommandletManager().getCommandlet(DotNet.class).run());

// assert
assertThat(thrown.getMessage()).contains("failed with exit code 2");
assertThat(context).log(IdeLogLevel.INFO).hasMessageContaining("content to stdout");
assertThat(context).log(IdeLogLevel.INFO).hasMessageContaining("more content to stdout");
assertThat(context).log(IdeLogLevel.ERROR).hasMessageContaining("error message to stderr");
assertThat(context).log(IdeLogLevel.ERROR).hasMessageContaining("another error message to stderr");
}

private IdeLogLevel convertToIdeLogLevel(ProcessErrorHandling processErrorHandling) {

return switch (processErrorHandling) {
Expand Down
46 changes: 21 additions & 25 deletions cli/src/test/java/com/devonfw/tools/ide/tool/dotnet/DotNetTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@

import java.nio.file.Path;

import org.junit.jupiter.api.Test;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.ValueSource;

Expand All @@ -12,6 +11,9 @@
import com.devonfw.tools.ide.os.SystemInfoImpl;
import com.devonfw.tools.ide.os.SystemInfoMock;

/**
* Integration test of {@link DotNet}.
*/
public class DotNetTest extends AbstractIdeContextTest {

private static final Path PROJECTS_TARGET_PATH = Path.of("target/test-projects");
Expand All @@ -38,10 +40,8 @@ public void dotnetShouldInstallSuccessful(String os) {
assertThat(this.context.getSoftwarePath().resolve("dotnet")).exists();

if (this.context.getSystemInfo().isWindows()) {
assertThat(this.context.getSoftwarePath().resolve("dotnet/dotnet.cmd")).exists();
}

if (this.context.getSystemInfo().isLinux() || this.context.getSystemInfo().isMac()) {
assertThat(this.context.getSoftwarePath().resolve("dotnet/dotnet.exe")).exists();
} else {
assertThat(this.context.getSoftwarePath().resolve("dotnet/dotnet")).exists();
}

Expand All @@ -51,28 +51,24 @@ public void dotnetShouldInstallSuccessful(String os) {
assertThat(this.context).logAtSuccess().hasMessage("Successfully installed dotnet in version 6.0.419");
}

@Test
public void dotnetShouldRunExecutableForWindowsSuccessful() {

String expectedOutputWindows = "Dummy dotnet 6.0.419 on windows ";
if (SystemInfoImpl.INSTANCE.isWindows()) {
runExecutable("windows");
checkExpectedOutput(expectedOutputWindows);
}
}

@ParameterizedTest
@ValueSource(strings = { "mac", "linux" })
@ValueSource(strings = { "windows", "mac", "linux" })
public void dotnetShouldRunExecutableSuccessful(String os) {

String expectedOutputLinux = "Dummy dotnet 6.0.419 on linux ";
String expectedOutputMacOs = "Dummy dotnet 6.0.419 on mac ";
runExecutable(os);

if (this.context.getSystemInfo().isLinux()) {
checkExpectedOutput(expectedOutputLinux);
} else if (this.context.getSystemInfo().isMac()) {
checkExpectedOutput(expectedOutputMacOs);
// TODO: Check: https://github.com/devonfw/IDEasy/issues/701 for reference.
if (SystemInfoImpl.INSTANCE.isWindows()) {
String expectedOutputLinux = "Dummy dotnet 6.0.419 on linux ";
String expectedOutputMacOs = "Dummy dotnet 6.0.419 on mac ";
String expectedOutputWindows = "Dummy dotnet 6.0.419 on windows ";
runExecutable(os);

if (this.context.getSystemInfo().isLinux()) {
checkExpectedOutput(expectedOutputLinux);
} else if (this.context.getSystemInfo().isMac()) {
checkExpectedOutput(expectedOutputMacOs);
} else if (this.context.getSystemInfo().isWindows()) {
checkExpectedOutput(expectedOutputWindows);
}
}
}

Expand All @@ -85,7 +81,7 @@ private void runExecutable(String operatingSystem) {

SystemInfo systemInfo = SystemInfoMock.of(operatingSystem);
this.context.setSystemInfo(systemInfo);

this.context.info("Running dotnet binary from: {}", this.commandlet.getToolBinPath());
this.commandlet.run();
}

Expand Down
37 changes: 33 additions & 4 deletions cli/src/test/java/com/devonfw/tools/ide/tool/npm/NpmTest.java
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package com.devonfw.tools.ide.tool.npm;

import org.junit.jupiter.api.Disabled;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.ValueSource;

Expand Down Expand Up @@ -37,13 +38,41 @@ public void testNpmInstall(String os) {
checkInstallation(context);
}

/**
* Tests if npm can be run properly.
* TODO: Check: <a href="https://github.com/devonfw/IDEasy/issues/700">#700</a> for reference.
*
* @param os Operating System.
*/
@Disabled
@ParameterizedTest
@ValueSource(strings = { "windows", "mac", "linux" })
public void testNpmRun(String os) {

// arrange
IdeTestContext context = newContext(PROJECT_NPM);
SystemInfo systemInfo = SystemInfoMock.of(os);
context.setSystemInfo(systemInfo);
Npm commandlet = new Npm(context);

// act
commandlet.run();

// assert
if (context.getSystemInfo().isWindows()) {
assertThat(context).logAtInfo().hasMessage("npmcmdbin ");
} else {
assertThat(context).logAtInfo().hasMessage("npmcmd ");
}
hohwille marked this conversation as resolved.
Show resolved Hide resolved
}

private void checkInstallation(IdeTestContext context) {

if (context.getSystemInfo().isWindows()) {
assertThat(context.getSoftwarePath().resolve("node/npm")).exists().hasContent("#!/bin/bash\n" + "echo \"npmbin $*\"");
assertThat(context.getSoftwarePath().resolve("node/npm.cmd")).exists().hasContent("@echo off\n" + "echo npmcmdbin %*");
assertThat(context.getSoftwarePath().resolve("node/npx")).exists().hasContent("#!/bin/bash\n" + "echo \"npxbin $*\"");
assertThat(context.getSoftwarePath().resolve("node/npx.cmd")).exists().hasContent("@echo off\n" + "echo npxcmdbin %*");
assertThat(context.getSoftwarePath().resolve("node/npm")).exists();
assertThat(context.getSoftwarePath().resolve("node/npm.cmd")).exists();
assertThat(context.getSoftwarePath().resolve("node/npx")).exists();
assertThat(context.getSoftwarePath().resolve("node/npx.cmd")).exists();
}

assertThat(context.getSoftwarePath().resolve("npm/.ide.software.version")).exists().hasContent("9.9.2");
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
#!/bin/bash

echo "Dummy dotnet 6.0.419 on windows $*"
Empty file.

This file was deleted.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

This file was deleted.

Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
#!/bin/bash
node "npm"
echo "npm windows $*"
echo "npm linux $*"

This file was deleted.

This file was deleted.

Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
#!/bin/bash
node "npm"
echo "npm windows $*"
echo "npm mac $*"

This file was deleted.

This file was deleted.

Loading
Loading