#1761: exclude BETA_OR_BUILD from development phases#1776
#1761: exclude BETA_OR_BUILD from development phases#1776laert-ll wants to merge 2 commits intodevonfw:mainfrom
Conversation
Pull Request Test Coverage Report for Build 23643547060Details
💛 - Coveralls |
There was a problem hiding this comment.
@laert-ll thanks for your PR and fix. Great finding. 👍
Could you also extend VersionIdentifierTest so we actually cover the bug and specify in the test what we actually expect to happen (and prevent that it may break again in the future).
Please also add the issue to our CHANGELOG (see DoD).
…t Java 8* matches
a725879 to
1a11b26
Compare
hohwille
left a comment
There was a problem hiding this comment.
@laert-ll thanks for adding the test.
I expected exactly this
assertThat(VersionIdentifier.of("8u412b08").isStable()).isTrue();
that you added 👍
However, you also changed that "8u412b08" is expected to be a valid version without changing the implementation of VersionSegment.isValid().
IMHO this expectation also makes sense but then we need to change isValid() implementation.
| @ParameterizedTest | ||
| // arrange | ||
| @ValueSource(strings = { "1.0", "0.1", "2023.08.001", "2023-06-M1", "11.0.4_11.4", "5.2.23.RELEASE" }) | ||
| @ValueSource(strings = { "1.0", "0.1", "2023.08.001", "2023-06-M1", "11.0.4_11.4", "5.2.23.RELEASE", "8u412b08" }) |
There was a problem hiding this comment.
I agree that if Java is using such version, we should consider that as valid.
However, our implementation does not agree.
Test is now failing:
[ERROR] com.devonfw.tools.ide.version.VersionIdentifierTest.testValid(String)[7] -- Time elapsed: 0.012 s <<< FAILURE!
org.opentest4j.AssertionFailedError:
[8u412b08]
Expecting value to be true but was false
at com.devonfw.tools.ide.version.VersionIdentifierTest.testValid(VersionIdentifierTest.java:71)
If we want to include this change here, we need to have a look at isValid() and somehow understand and fix this problem...
This PR fixes #1761
Implemented changes:
Checklist for this PR
Make sure everything is checked before merging this PR. For further info please also see
our DoD.
mvn clean testlocally all tests pass and build is successful#«issue-id»: «brief summary»(e.g.#921: fixed setup.bat). If no issue ID exists, title only.In Progressand assigned to you or there is no issue (might happen for very small PRs)