You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
To make it even better, I would expect that we provide a protected method in AbstractIdeContextTest with the logic to replace such variable from WireMockRuntimeInfo to centralize the approach and reuse it from the places where we need it.
Ideally the test-case does not have to do anything manually to get this variable ${testbaseurl} replaced.
Maybe we can pass WireMockRuntimeInfo as optional argument to AbstractIdeContextTest.newProject method variants so that such variable is automatically replaced why the project is copied to target folder.
Also I found these left-overs that should be fixed to not use hardcoded wiremock ports (most probably these URLs are not used at all, but it should be aligned - see #706):
It would be better to use a different tool for this kind of test where we test what happens if the tool was already installed.
The problem is that all the tests of get-version and get-edition are using the same test-project and the same tool az.
If you read the tests and see that in the test-project az is not installed and therefore we get a specific behaviour if we call different variants of get-version or get-edition when reading the code you start getting of an idea of this scenario reused by many of the test methods.
However, in the test method linked the existing scenario is "mutated" and the behaviour and setup of az is therefore changing leading to different results. Surely that is what we want to test that the result also depends on the state.
However, we had a bug fixed by #955 where this mutation was accidentally causing side-effects for other tests to fail if they were executed afterwards but had been green if executed before this mutation.
So to make it short: A better approach is to create a new "space" where this scenario is given. To avoid the need to create an extra new test-project just for this, I would recommend to just use another tool instead of az that is not yet included in this test-project and not used by any other tests using this test-project.
The text was updated successfully, but these errors were encountered:
Both in PR #909 and in #903 we introduced an antipattern to our tests that directly write into the test project configuration what should be avoided.
IDEasy/cli/src/test/java/com/devonfw/tools/ide/tool/intellij/IntellijTest.java
Lines 189 to 201 in 58fd72a
A better solution can be found e.g. here:
https://github.com/devonfw/IDEasy/blob/main/cli/src/test/resources/ide-projects/dotnet/_ide/urls/dotnet/dotnet/6.0.419/urls
And we have code to resolve such variables:
IDEasy/cli/src/test/java/com/devonfw/tools/ide/tool/npm/NpmJsonUrlUpdaterTest.java
Lines 68 to 71 in 58fd72a
To make it even better, I would expect that we provide a protected method in
AbstractIdeContextTest
with the logic to replace such variable fromWireMockRuntimeInfo
to centralize the approach and reuse it from the places where we need it.Ideally the test-case does not have to do anything manually to get this variable
${testbaseurl}
replaced.Maybe we can pass
WireMockRuntimeInfo
as optional argument toAbstractIdeContextTest.newProject
method variants so that such variable is automatically replaced why the project is copied totarget
folder.Also I found these left-overs that should be fixed to not use hardcoded wiremock ports (most probably these URLs are not used at all, but it should be aligned - see #706):
IDEasy/cli/src/test/resources/ide-projects/basic/_ide/urls/java/java/17.0.6/mac_x64.urls
Line 1 in 58fd72a
IDEasy/cli/src/test/resources/ide-projects/basic/_ide/urls/java/java/17.0.6/linux_x64.urls
Line 1 in 58fd72a
IDEasy/cli/src/test/resources/ide-projects/basic/_ide/urls/java/java/17.0.6/windows_x64.urls
Line 1 in 58fd72a
Also, I consider this approach as an anti-pattern:
IDEasy/cli/src/test/java/com/devonfw/tools/ide/commandlet/EditionGetCommandletTest.java
Line 43 in 58fd72a
It would be better to use a different tool for this kind of test where we test what happens if the tool was already installed.
The problem is that all the tests of
get-version
andget-edition
are using the same test-project and the same toolaz
.If you read the tests and see that in the test-project
az
is not installed and therefore we get a specific behaviour if we call different variants ofget-version
orget-edition
when reading the code you start getting of an idea of this scenario reused by many of the test methods.However, in the test method linked the existing scenario is "mutated" and the behaviour and setup of
az
is therefore changing leading to different results. Surely that is what we want to test that the result also depends on the state.However, we had a bug fixed by #955 where this mutation was accidentally causing side-effects for other tests to fail if they were executed afterwards but had been green if executed before this mutation.
So to make it short: A better approach is to create a new "space" where this scenario is given. To avoid the need to create an extra new test-project just for this, I would recommend to just use another tool instead of
az
that is not yet included in this test-project and not used by any other tests using this test-project.The text was updated successfully, but these errors were encountered: