#78: the properties file stays stable. Wrote test #107
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
The PR tests should succeed if PR #108 is merged. |
hohwille
reviewed
Oct 20, 2023
cli/src/test/java/com/devonfw/tools/ide/environment/EnvironmentVariablesPropertiesFileTest.java
Outdated
Show resolved
Hide resolved
…hanges-to-ide.properties
…ork on github, so now tmp file to target dir
hohwille
reviewed
Oct 20, 2023
cli/src/test/java/com/devonfw/tools/ide/environment/EnvironmentVariablesPropertiesFileTest.java
Outdated
Show resolved
Hide resolved
hohwille
reviewed
Oct 20, 2023
cli/src/test/java/com/devonfw/tools/ide/environment/EnvironmentVariablesPropertiesFileTest.java
Outdated
Show resolved
Hide resolved
hohwille
reviewed
Oct 20, 2023
cli/src/test/java/com/devonfw/tools/ide/environment/EnvironmentVariablesPropertiesFileTest.java
Outdated
Show resolved
Hide resolved
hohwille
reviewed
Oct 20, 2023
cli/src/test/java/com/devonfw/tools/ide/environment/EnvironmentVariablesPropertiesFileTest.java
Outdated
Show resolved
Hide resolved
hohwille
reviewed
Oct 20, 2023
cli/src/test/java/com/devonfw/tools/ide/environment/EnvironmentVariablesPropertiesFileTest.java
Outdated
Show resolved
Hide resolved
hohwille
approved these changes
Oct 20, 2023
There was a problem hiding this comment.
@MattesMrzik thanks for adding this test. You created a nice test-scenario that should cover most relevant cases. 👍
You could even add a case that removes a variable by settings its value to null to cover that as well.
I left a few general comments on writing great tests. Please have a look. After resolving these comments, we can merge.
…improve-writing-changes-to-ide.properties
…rties' of https://github.com/MattesMrzik/IDEasy into feature/devonfw#78-improve-writing-changes-to-ide.properties
hohwille
approved these changes
Oct 23, 2023
There was a problem hiding this comment.
@MattesMrzik thanks for the rework. Now everything is perfectly nailed - great work 👍
hohwille
pushed a commit
to hohwille/IDEasy
that referenced
this pull request
Oct 24, 2023
hohwille
pushed a commit
to hohwille/IDEasy
that referenced
this pull request
Oct 24, 2023
MattesMrzik
deleted the
feature/#78-improve-writing-changes-to-ide.properties
branch
hohwille
added
the
story-review
hohwille
added
reviewed
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The test confirms that the properties file remains stable even if values are updated. The test further revealed, that the set method of EnvironmentVariablesPropertiesFile does not update the "export" flag if the old and new value are identical (see issue #106 )