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

#898: Improved output of get-version/edition and uninstall/-plugin #903

Merged

Conversation

WorkingAmeise
Copy link
Contributor

Closes: #898
Edition/VersionGetCommandlet changed them to log the configured option if the tool is not installed and the installed option if it is installed.
Changed the toolInstallInfo method to log into IdeSublogger instead of using context.info or context.warning.
Changed the uninstall commandlet to log a warning in case the requested commandlet is not installed.
Changed the uninstallPlugin commandlet to give feedback wether the plugin could be deleted or if there was not installation found.
Adjusted Tests for Edition/VersionGetCommandlet as well as Uninstall commandlet to expect the correct loglevel or content of logged messages.
Added logAtProccessable to IdeTestContextAssertions to check if a log-message was logged on PROCESSABLE-level.
Added .ide.software.version file to the npm software in the basic ide-test-project-ressource, since it was needed to test my implementation.

WorkingAmeise added 2 commits December 19, 2024 12:57
Edition/VersionGetCommandlet changed them to log the configured option if the tool is not installed and the installed option if it is installed.
Changed the toolInstallInfo method to log into IdeSublogger instead of using context.info or context.warning.
Changed the uninstall commandlet to log a warning in case the requested commandlet is not installed.
Changed the uninstallPlugin commandlet to give feedback wether the plugin could be deleted or if there was not installation found.
Adjusted Tests for Edition/VersionGetCommandlet as well as Uninstall commandlet to expect the correct loglevel or content of logged messages.
Added logAtProccessable to IdeTestContextAssertions to check if a log-message was logged on PROCESSABLE-level.
Added .ide.software.version file to the npm software in the basic ide-test-project-ressource, since it was needed to test my implementation.
added CHANGELOG entry
@WorkingAmeise WorkingAmeise self-assigned this Dec 19, 2024
@coveralls
Copy link
Collaborator

coveralls commented Dec 19, 2024

Pull Request Test Coverage Report for Build 12816835423

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • 18 unchanged lines in 3 files lost coverage.
  • Overall coverage increased (+0.05%) to 68.199%

Files with Coverage Reduction New Missed Lines %
com/devonfw/tools/ide/io/FileAccessImpl.java 2 65.53%
com/devonfw/tools/ide/tool/LocalToolCommandlet.java 7 85.84%
com/devonfw/tools/ide/tool/plugin/PluginBasedCommandlet.java 9 70.11%
Totals Coverage Status
Change from base Build 12815494212: 0.05%
Covered Lines: 7031
Relevant Lines: 9942

💛 - Coveralls

@WorkingAmeise WorkingAmeise added the enhancement New feature or request label Dec 19, 2024
@hohwille hohwille added this to the release:2025.01.001 milestone Dec 20, 2024
@hohwille hohwille self-assigned this Dec 20, 2024
@hohwille
Copy link
Member

@jan-vcapgemini please have a look.
I reworked this PR and consider it as ready for review.
FYI: I also did some improvements and fixes of existing code that before was not part of this PR (e.g. LocalToolCommandlet.getInstalledEdition()).

@hohwille
Copy link
Member

hohwille commented Jan 7, 2025

[ERROR]   EditionGetCommandletTest.testEditionGetCommandletInstalledEditionToolNotInstalled:90 
The first entry that was not matching from a block of 4 expected log-entries was:
PROCESSABLE:No installation of tool java was found.

In the logs of this test:
...
WARNING:Tool java is not installed via software repository (maybe from devonfw-ide). Please consider reinstalling it.
...
PROCESSABLE:java

So if I get this correct, the test was expecting the message No installation of tool java was found. but this PR is changing the behaviour to print the configured edition if no installed edition could be found and no --configured or --installed option is specified.
Therefore the test(s) have to be adjusted.

@hohwille hohwille changed the title #898 improved feedback messages #898: Improved output of get-version/edition and uninstall/-plugin Jan 7, 2025
@hohwille hohwille removed their assignment Jan 14, 2025
Copy link
Member

@hohwille hohwille left a comment

Choose a reason for hiding this comment

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

@jan-vcapgemini thanks for taking over this PR. You did a really awesome job here 👍
I only added minor suggestions but one review comment seems to indicate some kind of bug that we should discuss and if confirmed fix before merging.

hohwille and others added 2 commits January 20, 2025 13:20
made sure that all get tests will not copy test resources
enhanced edition basic test description
@hohwille
Copy link
Member

Thanks for fast and final perfection of this PR.
Ready for merge now! 👍

@hohwille hohwille merged commit 59b245f into devonfw:main Jan 20, 2025
4 checks passed
hohwille added a commit to hohwille/IDEasy that referenced this pull request Jan 20, 2025
hohwille added a commit that referenced this pull request Jan 20, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Status: ✅ Done
Development

Successfully merging this pull request may close these issues.

Improved output of get-version/edition and uninstall/-plugin
4 participants