#608: enhance error messages #653

jan-vcapgemini · 2024-09-25T21:37:43Z

Fixes: #608

Implements:

reformatted process out and error messages
added test with expected Exception and error messages
performLogOnError will only be used on DEFAULT_CAPTURE
added Npm run test (disabled)
adjusted Dotnet run test (Windows only, does not work on CI)

added test for enhanced error messages

added outs to error message added check for DEFAULT_CAPTURE to start performLogOnError

reformatted process out and error messages added test with expected Exception and error messages added test resources

hohwille · 2024-09-27T09:42:15Z

Sorry for interfering with PR #652. I am not sure if #608 is already fixed. I assume I only fixed it partially.
What if we have DEFAULT_CAPTURE combined with THROW_CLI?
IMHO then still the output would not be printed so we still need a small change for #652.

# Conflicts: # cli/src/main/java/com/devonfw/tools/ide/process/ProcessContextImpl.java

jan-vcapgemini · 2024-09-30T08:43:52Z

I need to move logic of getout and getErr into ProcessResult.
And find getErr and GetOut loop in Eclipse which does the same and refactor too.

refactored eclipse log method into ProcessResult added log method to ProcessResult optimized performLogOnError to use logger with proper log levels instead of custom messages

# Conflicts: # cli/src/main/java/com/devonfw/tools/ide/tool/eclipse/Eclipse.java

made sure that context runs on windows to detect dotnet.cmd properly

moved dotnet.cmd to windows folder

moved windows test to parameterized tests

added missing javadocs fixed dotnet windows test script

renamed dotnet.cmd to dotnet.exe

fixed DotNet test added run test to NpmTest removed unused test resource files renamed readme files to .gitkeep

made dotnet.exe executable

adjusted DotnetTest adjusted NpmTest

disabled dotnet run tests on CI disabled npm run test

coveralls · 2024-10-17T15:55:57Z

Pull Request Test Coverage Report for Build 11595805984

Details

0 of 0 changed or added relevant lines in 0 files are covered.
22 unchanged lines in 4 files lost coverage.
Overall coverage increased (+0.2%) to 66.915%

Files with Coverage Reduction	New Missed Lines	%
com/devonfw/tools/ide/process/ProcessResultImpl.java	1	93.1%
com/devonfw/tools/ide/tool/dotnet/DotNet.java	1	66.67%
com/devonfw/tools/ide/tool/eclipse/Eclipse.java	6	75.0%
com/devonfw/tools/ide/process/ProcessContextImpl.java	14	79.86%

Totals
Change from base Build 11580911301:	0.2%
Covered Lines:	6302
Relevant Lines:	9075

💛 - Coveralls

hohwille

@jan-vcapgemini thanks for your PR. You covered the relevant spots and did some good improvements. 👍
However, some things here IMHO need some rework.
Further, ProcessContextTestImpl needs to be adopted since you now have added reasonable logging and you should remove the hack there to log the output again.

cli/src/main/java/com/devonfw/tools/ide/process/ProcessContextImpl.java

cli/src/main/java/com/devonfw/tools/ide/process/ProcessResultImpl.java

cli/src/main/java/com/devonfw/tools/ide/process/ProcessContextImpl.java

cli/src/test/java/com/devonfw/tools/ide/tool/npm/NpmTest.java

removed SubLogger moved throw of exceptions after logger optimized out and err creation removed level checks for logger

re-added logging of exitcode on proper log level

cli/src/main/java/com/devonfw/tools/ide/process/ProcessContextImpl.java

cli/src/main/java/com/devonfw/tools/ide/tool/eclipse/Eclipse.java

removed duplicated log messages removed ProcessResult.run hack fixed and cleaned up tests renamed ProcessConttextUnderTest added new log method with 2 log levels to ProcessResult

hohwille · 2024-10-25T23:58:11Z

There are tests revealing that something is still broken:

[ERROR] MvnTest.testMvnRun:69 Could not find log message equal to 'mvn foo bar' on level INFO
[ERROR] TomcatTest.testTomcat:50 Found expected log entries:
SUCCESS:Successfully installed java in version 8u402b06
The first entry that was not matching from a block of 10 expected log-entries was:
INFO:OpenJDK version 8u402b06

The message OpenJDK version 8u402b06 that is the output of the faked java --version is not logged anymore so the process output is somehow not logged here anymore. Probably we have the same problem with mvn where the test now also failed.

enforced log output of test scripts even if processMode was BACKGROUND

cli/src/test/java/com/devonfw/tools/ide/context/ProcessContextTestImpl.java

cli/src/test/java/com/devonfw/tools/ide/tool/tomcat/TomcatTest.java

hohwille

@jan-vcapgemini thanks for completing this rather complicated and underestimated story 👍
Have a look at the remaining comments. Then we can merge. Great work 🥇

…TestImpl.java Co-authored-by: Jörg Hohwiller <hohwille@users.noreply.github.com>

removed hacks

jan-vcapgemini added 7 commits September 11, 2024 14:23

devonfw#608: enhanced error messages

7bbea6e

added test for enhanced error messages

Merge branch 'main' into feature/608-enhance-error-messages

131933a

Merge branch 'main' into feature/608-enhance-error-messages

c3a6b1c

Merge branch 'main' into feature/608-enhance-error-messages

68a695e

devonfw#608: implemented requested changes

19b17d0

added outs to error message added check for DEFAULT_CAPTURE to start performLogOnError

Merge branch 'main' into feature/608-enhance-error-messages

7423297

devonfw#608: Adjusted out and error format

91d64ab

reformatted process out and error messages added test with expected Exception and error messages added test resources

jan-vcapgemini self-assigned this Sep 25, 2024

devonfw#608: added execute permissions to dotnet

867e62e

jan-vcapgemini and others added 2 commits September 27, 2024 12:27

Merge branch 'main' into feature/608-enhance-error-messages

a952d83

# Conflicts: # cli/src/main/java/com/devonfw/tools/ide/process/ProcessContextImpl.java

Merge branch 'main' into feature/608-enhance-error-messages

707d300

jan-vcapgemini requested a review from alfeilex October 4, 2024 08:39

jan-vcapgemini assigned alfeilex Oct 4, 2024

jan-vcapgemini and others added 14 commits October 4, 2024 12:35

devonfw#608: applied refactorings and fixes

8b60102

refactored eclipse log method into ProcessResult added log method to ProcessResult optimized performLogOnError to use logger with proper log levels instead of custom messages

Merge branch 'main' into feature/608-enhance-error-messages

8aa538a

# Conflicts: # cli/src/main/java/com/devonfw/tools/ide/tool/eclipse/Eclipse.java

devonfw#608: fixed test

7e89d36

made sure that context runs on windows to detect dotnet.cmd properly

devonfw#608: fixed test

b786464

moved dotnet.cmd to windows folder

devonfw#608: optimized Dotnet test

11dfda2

moved windows test to parameterized tests

devonfw#608: fixed tests

88b1475

added missing javadocs fixed dotnet windows test script

devonfw#608: fixed Dotnet windows test

20bd7c6

renamed dotnet.cmd to dotnet.exe

devonfw#608: fixed tests

7a0433a

fixed DotNet test added run test to NpmTest removed unused test resource files renamed readme files to .gitkeep

Merge branch 'main' into feature/608-enhance-error-messages

4113983

devonfw#608: fixed tests

bbb4c89

made dotnet.exe executable

devonfw#608: fixed tests

5d7f6af

adjusted DotnetTest adjusted NpmTest

Merge branch 'main' into feature/608-enhance-error-messages

0380559

Merge branch 'main' into feature/608-enhance-error-messages

6cacf03

devonfw#608: disabled tests

b1ef58e

disabled dotnet run tests on CI disabled npm run test

hohwille requested changes Oct 18, 2024

View reviewed changes

jan-vcapgemini and others added 2 commits October 21, 2024 13:38

Merge branch 'main' into feature/608-enhance-error-messages

e516604

devonfw#608: implemented requested changes

42f59f5

removed SubLogger moved throw of exceptions after logger optimized out and err creation removed level checks for logger

jan-vcapgemini requested a review from hohwille October 21, 2024 11:58

devonfw#608: fixed test

1e4b116

re-added logging of exitcode on proper log level

hohwille reviewed Oct 21, 2024

View reviewed changes

cli/src/main/java/com/devonfw/tools/ide/process/ProcessContextImpl.java Outdated Show resolved Hide resolved

hohwille reviewed Oct 21, 2024

View reviewed changes

cli/src/main/java/com/devonfw/tools/ide/tool/eclipse/Eclipse.java Outdated Show resolved Hide resolved

jan-vcapgemini and others added 3 commits October 24, 2024 17:08

devonfw#608: implemented requested changes

68770c7

removed duplicated log messages removed ProcessResult.run hack fixed and cleaned up tests renamed ProcessConttextUnderTest added new log method with 2 log levels to ProcessResult

Merge branch 'main' into feature/608-enhance-error-messages

0a3e586

devonfw#608: added changes to changelog

39400c5

jan-vcapgemini and others added 3 commits October 28, 2024 14:20

devonfw#608: fixed tests

858ffca

enforced log output of test scripts even if processMode was BACKGROUND

Merge branch 'main' into feature/608-enhance-error-messages

3f1b8ef

Merge branch 'main' into feature/608-enhance-error-messages

7f65aee

hohwille mentioned this pull request Oct 29, 2024

Add Npm run test #700

Open

hohwille added this to the release:2024.11.001 milestone Oct 29, 2024

hohwille reviewed Oct 29, 2024

View reviewed changes

cli/src/test/java/com/devonfw/tools/ide/context/ProcessContextTestImpl.java Outdated Show resolved Hide resolved

hohwille reviewed Oct 29, 2024

View reviewed changes

cli/src/test/java/com/devonfw/tools/ide/tool/tomcat/TomcatTest.java Outdated Show resolved Hide resolved

hohwille approved these changes Oct 29, 2024

View reviewed changes

jan-vcapgemini and others added 4 commits October 30, 2024 11:07

Update cli/src/test/java/com/devonfw/tools/ide/context/ProcessContext…

c39829e

…TestImpl.java Co-authored-by: Jörg Hohwiller <hohwille@users.noreply.github.com>

devonfw#608: implemented requested changes

c7f2c55

removed hacks

Merge branch 'main' into feature/608-enhance-error-messages

0603a20

devonfw#608: moved changelog entry to new version

e95034a

hohwille merged commit 39d1434 into devonfw:main Oct 31, 2024
4 checks passed

hohwille mentioned this pull request Nov 1, 2024

Improve ProcessResult: get out and err in order #734

Open

jan-vcapgemini deleted the feature/608-enhance-error-messages branch November 4, 2024 11:19

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

#608: enhance error messages #653

#608: enhance error messages #653

jan-vcapgemini commented Sep 25, 2024 •

edited

Loading

hohwille commented Sep 27, 2024

jan-vcapgemini commented Sep 30, 2024 •

edited

Loading

coveralls commented Oct 17, 2024 •

edited

Loading

hohwille left a comment

hohwille commented Oct 25, 2024

hohwille left a comment

#608: enhance error messages #653

#608: enhance error messages #653

Conversation

jan-vcapgemini commented Sep 25, 2024 • edited Loading

hohwille commented Sep 27, 2024

jan-vcapgemini commented Sep 30, 2024 • edited Loading

coveralls commented Oct 17, 2024 • edited Loading

Pull Request Test Coverage Report for Build 11595805984

Details

💛 - Coveralls

hohwille left a comment

Choose a reason for hiding this comment

hohwille commented Oct 25, 2024

hohwille left a comment

Choose a reason for hiding this comment

jan-vcapgemini commented Sep 25, 2024 •

edited

Loading

jan-vcapgemini commented Sep 30, 2024 •

edited

Loading

coveralls commented Oct 17, 2024 •

edited

Loading