diff --git a/cli/src/main/java/com/devonfw/tools/ide/context/AbstractIdeContext.java b/cli/src/main/java/com/devonfw/tools/ide/context/AbstractIdeContext.java index f6417d28c..7b3d99b88 100644 --- a/cli/src/main/java/com/devonfw/tools/ide/context/AbstractIdeContext.java +++ b/cli/src/main/java/com/devonfw/tools/ide/context/AbstractIdeContext.java @@ -132,7 +132,7 @@ public abstract class AbstractIdeContext implements IdeContext { * @param factory the {@link Function} to create {@link IdeSubLogger} per {@link IdeLogLevel}. * @param userDir the optional {@link Path} to current working directory. * @param toolRepository @param toolRepository the {@link ToolRepository} of the context. If it is set to {@code null} - * {@link DefaultToolRepository} will be used. + * {@link DefaultToolRepository} will be used. */ public AbstractIdeContext(IdeLogLevel minLogLevel, Function factory, Path userDir, ToolRepository toolRepository) { @@ -273,7 +273,7 @@ private String getMessageIdeHomeNotFound() { /** * @return the status message about the {@link #getIdeHome() IDE_HOME} detection and environment variable - * initialization. + * initialization. */ public String getMessageIdeHome() { @@ -612,7 +612,7 @@ public DirectoryMerger getWorkspaceMerger() { /** * @return the {@link #getDefaultExecutionDirectory() default execution directory} in which a command process is - * executed. + * executed. */ public Path getDefaultExecutionDirectory() { @@ -768,13 +768,20 @@ public StepImpl newStep(boolean silent, String name, Object... parameters) { */ public void endStep(StepImpl step) { - assert (step == this.currentStep); - this.currentStep = this.currentStep.getParent(); + if (step == this.currentStep) { + this.currentStep = this.currentStep.getParent(); + } else { + String currentStepName = "null"; + if (this.currentStep != null) { + currentStepName = this.currentStep.getName(); + } + warning("endStep called with wrong step '{}' but expected '{}'", step.getName(), currentStepName); + } } /** - * Finds the matching {@link Commandlet} to run, applies {@link CliArguments} to its {@link Commandlet#getProperties() - * properties} and will execute it. + * Finds the matching {@link Commandlet} to run, applies {@link CliArguments} to its + * {@link Commandlet#getProperties() properties} and will execute it. * * @param arguments the {@link CliArgument}. * @return the return code of the execution. @@ -827,7 +834,7 @@ public int run(CliArguments arguments) { * {@link #apply(CliArguments, Commandlet, CompletionCandidateCollector) apply} and {@link Commandlet#run() * run}. * @return {@code true} if the given {@link Commandlet} matched and did {@link Commandlet#run() run} successfully, - * {@code false} otherwise (the {@link Commandlet} did not match and we have to try a different candidate). + * {@code false} otherwise (the {@link Commandlet} did not match and we have to try a different candidate). */ private boolean applyAndRun(CliArguments arguments, Commandlet cmd) { @@ -888,7 +895,7 @@ public List complete(CliArguments arguments, boolean includ /** * @param arguments the {@link CliArguments} to apply. Will be {@link CliArguments#next() consumed} as they are - * matched. Consider passing a {@link CliArguments#copy() copy} as needed. + * matched. Consider passing a {@link CliArguments#copy() copy} as needed. * @param cmd the potential {@link Commandlet} to match. * @param collector the {@link CompletionCandidateCollector}. * @return {@code true} if the given {@link Commandlet} matches to the given {@link CliArgument}(s) and those have diff --git a/cli/src/main/java/com/devonfw/tools/ide/step/StepImpl.java b/cli/src/main/java/com/devonfw/tools/ide/step/StepImpl.java index ff7fee7d9..a8b8ad523 100644 --- a/cli/src/main/java/com/devonfw/tools/ide/step/StepImpl.java +++ b/cli/src/main/java/com/devonfw/tools/ide/step/StepImpl.java @@ -131,7 +131,8 @@ public void end() { private void end(Boolean newSuccess, Throwable error, boolean suppress, String message, Object[] args) { - if (this.success != null) { + boolean firstCallOfEnd = (this.success == null); + if (!firstCallOfEnd) { assert (this.duration > 0); if (newSuccess != null) { this.context.warning("Step '{}' already ended with {} and now ended again with {}.", this.name, this.success, @@ -177,7 +178,9 @@ private void end(Boolean newSuccess, Throwable error, boolean suppress, String m } logger.log("Step '{}' ended with failure.", this.name); } - this.context.endStep(this); + if (firstCallOfEnd) { + this.context.endStep(this); + } } /** diff --git a/cli/src/test/java/com/devonfw/tools/ide/context/AbstractIdeContextTest.java b/cli/src/test/java/com/devonfw/tools/ide/context/AbstractIdeContextTest.java index 74ff877fe..fb93b4bd2 100644 --- a/cli/src/test/java/com/devonfw/tools/ide/context/AbstractIdeContextTest.java +++ b/cli/src/test/java/com/devonfw/tools/ide/context/AbstractIdeContextTest.java @@ -38,7 +38,7 @@ public abstract class AbstractIdeContextTest extends Assertions { /** * @param testProject the (folder)name of the project test case, in this folder a 'project' folder represents the test - * project in {@link #TEST_PROJECTS}. E.g. "basic". + * project in {@link #TEST_PROJECTS}. E.g. "basic". * @return the {@link IdeTestContext} pointing to that project. */ protected IdeTestContext newContext(String testProject) { @@ -48,7 +48,7 @@ protected IdeTestContext newContext(String testProject) { /** * @param testProject the (folder)name of the project test case, in this folder a 'project' folder represents the test - * project in {@link #TEST_PROJECTS}. E.g. "basic". + * project in {@link #TEST_PROJECTS}. E.g. "basic". * @param projectPath the relative path inside the test project where to create the context. * @return the {@link IdeTestContext} pointing to that project. */ @@ -59,7 +59,7 @@ protected static IdeTestContext newContext(String testProject, String projectPat /** * @param testProject the (folder)name of the project test case, in this folder a 'project' folder represents the test - * project in {@link #TEST_PROJECTS}. E.g. "basic". + * project in {@link #TEST_PROJECTS}. E.g. "basic". * @param projectPath the relative path inside the test project where to create the context. * @param copyForMutation - {@code true} to create a copy of the project that can be modified by the test, * {@code false} otherwise (only to save resources if you are 100% sure that your test never modifies anything @@ -136,7 +136,7 @@ protected static void assertLogMessage(IdeTestContext context, IdeLogLevel level * @param level the expected {@link IdeLogLevel}. * @param message the expected {@link com.devonfw.tools.ide.log.IdeSubLogger#log(String) log message}. * @param contains - {@code true} if the given {@code message} may only be a sub-string of the log-message to assert, - * {@code false} otherwise (the entire log message including potential parameters being filled in is asserted). + * {@code false} otherwise (the entire log message including potential parameters being filled in is asserted). */ protected static void assertLogMessage(IdeTestContext context, IdeLogLevel level, String message, boolean contains) { @@ -155,6 +155,43 @@ public boolean matches(String e) { } } + /** + * @param context the {@link IdeContext} that was created via the {@link #newContext(String) newContext} method. + * @param level the expected {@link IdeLogLevel}. + * @param message the {@link com.devonfw.tools.ide.log.IdeSubLogger#log(String) log message} that should not have been + * logged. + */ + protected static void assertNoLogMessage(IdeTestContext context, IdeLogLevel level, String message) { + + assertNoLogMessage(context, level, message, false); + } + + /** + * @param context the {@link IdeContext} that was created via the {@link #newContext(String) newContext} method. + * @param level the expected {@link IdeLogLevel}. + * @param message the {@link com.devonfw.tools.ide.log.IdeSubLogger#log(String) log message} that should not have been + * logged. + * @param contains - {@code true} if the given {@code message} may only be a sub-string of the log-message to assert, + * {@code false} otherwise (the entire log message including potential parameters being filled in is asserted). + */ + protected static void assertNoLogMessage(IdeTestContext context, IdeLogLevel level, String message, + boolean contains) { + + IdeTestLogger logger = context.level(level); + ListAssert assertion = assertThat(logger.getMessages()).as(level.name() + "-Log messages"); + if (contains) { + Condition condition = new Condition<>() { + public boolean matches(String e) { + + return e.contains(message); + } + }; + assertion.filteredOn(condition).isEmpty(); + } else { + assertion.doesNotContain(message); + } + } + /** * Checks if a {@link com.devonfw.tools.ide.io.IdeProgressBar} was implemented correctly and reflects a default * behavior diff --git a/cli/src/test/java/com/devonfw/tools/ide/log/IdeSlf4jLogger.java b/cli/src/test/java/com/devonfw/tools/ide/log/IdeSlf4jLogger.java index 5f3176844..102d40a8e 100644 --- a/cli/src/test/java/com/devonfw/tools/ide/log/IdeSlf4jLogger.java +++ b/cli/src/test/java/com/devonfw/tools/ide/log/IdeSlf4jLogger.java @@ -35,9 +35,15 @@ public IdeSlf4jLogger(IdeLogLevel level) { @Override public String log(Throwable error, String message, Object... args) { + if ((message == null) && (error != null)) { + message = error.getMessage(); + if (message == null) { + message = error.toString(); + } + } String msg = message; if ((this.level == IdeLogLevel.STEP) || (this.level == IdeLogLevel.INTERACTION) - || (this.level == IdeLogLevel.SUCCESS)) { + || (this.level == IdeLogLevel.SUCCESS)) { msg = this.level.name() + ":" + message; } LoggingEventBuilder builder = LOG.atLevel(this.logLevel); diff --git a/cli/src/test/java/com/devonfw/tools/ide/step/StepTest.java b/cli/src/test/java/com/devonfw/tools/ide/step/StepTest.java new file mode 100644 index 000000000..132cc869f --- /dev/null +++ b/cli/src/test/java/com/devonfw/tools/ide/step/StepTest.java @@ -0,0 +1,133 @@ +package com.devonfw.tools.ide.step; + +import com.devonfw.tools.ide.context.AbstractIdeContextTest; +import com.devonfw.tools.ide.context.IdeTestContext; +import com.devonfw.tools.ide.log.IdeLogLevel; +import org.junit.jupiter.api.Test; + +/** + * Test of {@link Step}. + */ +public class StepTest extends AbstractIdeContextTest { + + @Test + public void testValidUsageSuccess() { + + // arrage + IdeTestContext context = newContext(PROJECT_BASIC, "project", false); + // act + Step step = context.newStep("Test-Step"); + try { + step.success("The Test-Step succeeded as expected"); + } finally { + step.end(); + } + // assert + assertThat(step.getSuccess()).isTrue(); + assertThat(step.getDuration()).isPositive(); + assertLogMessage(context, IdeLogLevel.TRACE, "Starting step Test-Step..."); + assertLogMessage(context, IdeLogLevel.STEP, "Start: Test-Step"); + assertLogMessage(context, IdeLogLevel.SUCCESS, "The Test-Step succeeded as expected"); + assertLogMessage(context, IdeLogLevel.DEBUG, "Step 'Test-Step' ended successfully."); + } + + @Test + public void testValidUsageSuccessSilent() { + + // arrage + IdeTestContext context = newContext(PROJECT_BASIC, "project", false); + // act + Step step = context.newStep(true, "Test-Step", "arg1", "arg2"); + try { + step.success(); + } finally { + step.end(); + } + // assert + assertThat(step.getSuccess()).isTrue(); + assertThat(step.getDuration()).isPositive(); + assertThat(step.getParameterCount()).isEqualTo(2); + assertThat(step.getParameter(0)).isEqualTo("arg1"); + assertThat(step.getParameter(1)).isEqualTo("arg2"); + assertThat(step.getParameter(2)).isNull(); + assertLogMessage(context, IdeLogLevel.TRACE, "Starting step Test-Step with params [arg1, arg2]..."); + assertNoLogMessage(context, IdeLogLevel.STEP, "Start: Test-Step"); + assertNoLogMessage(context, IdeLogLevel.SUCCESS, "Test-Step", true); + assertLogMessage(context, IdeLogLevel.DEBUG, "Step 'Test-Step' ended successfully."); + } + + @Test + public void testValidUsageError() { + + // arrage + IdeTestContext context = newContext(PROJECT_BASIC, "project", false); + // act + Step step = context.newStep("Test-Step"); + try { + step.error("The Test-Step failed as expected"); + } finally { + step.end(); + } + assertThat(step.getSuccess()).isFalse(); + assertThat(step.getDuration()).isPositive(); + // assert + assertLogMessage(context, IdeLogLevel.TRACE, "Starting step Test-Step..."); + assertLogMessage(context, IdeLogLevel.STEP, "Start: Test-Step"); + assertLogMessage(context, IdeLogLevel.ERROR, "The Test-Step failed as expected"); + assertLogMessage(context, IdeLogLevel.DEBUG, "Step 'Test-Step' ended with failure."); + } + + @Test + public void testInvalidUsageSuccessError() { + + // arrage + IdeTestContext context = newContext(PROJECT_BASIC, "project", false); + // act + Step step = context.newStep("Test-Step"); + try { + step.success("The Test-Step succeeded as expected"); + throw new IllegalStateException("unexpected situation!"); + } catch (IllegalStateException e) { + step.error(e); + } finally { + step.end(); + } + assertThat(step.getSuccess()).isFalse(); + assertThat(step.getDuration()).isPositive(); + // assert + assertLogMessage(context, IdeLogLevel.TRACE, "Starting step Test-Step..."); + assertLogMessage(context, IdeLogLevel.STEP, "Start: Test-Step"); + assertLogMessage(context, IdeLogLevel.WARNING, + "Step 'Test-Step' already ended with true and now ended again with false."); + assertLogMessage(context, IdeLogLevel.ERROR, "unexpected situation!"); + assertLogMessage(context, IdeLogLevel.DEBUG, "Step 'Test-Step' ended with failure."); + } + + @Test + public void testInvalidUsageErrorSuccess() { + + // arrage + IdeTestContext context = newContext(PROJECT_BASIC, "project", false); + // act + Step step = context.newStep("Test-Step"); + try { + step.error("The Test-Step failed as expected"); + // WOW this is really inconsistent and hopefully never happens elsewhere + step.success("The Test-Step succeeded as expected"); + } finally { + step.end(); + } + assertThat(step.getSuccess()).isFalse(); + assertThat(step.getDuration()).isPositive(); + // assert + assertLogMessage(context, IdeLogLevel.TRACE, "Starting step Test-Step..."); + assertLogMessage(context, IdeLogLevel.STEP, "Start: Test-Step"); + assertLogMessage(context, IdeLogLevel.ERROR, "The Test-Step failed as expected"); + assertLogMessage(context, IdeLogLevel.DEBUG, "Step 'Test-Step' ended with failure."); + assertLogMessage(context, IdeLogLevel.WARNING, + "Step 'Test-Step' already ended with false and now ended again with true."); + assertLogMessage(context, IdeLogLevel.SUCCESS, "The Test-Step succeeded as expected"); + assertLogMessage(context, IdeLogLevel.DEBUG, "Step 'Test-Step' ended successfully."); + } + +}