-
Notifications
You must be signed in to change notification settings - Fork 2.2k
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
[WFLY-19286] Add ServerSetupTasks to reload server to a desired stabi… #17849
[WFLY-19286] Add ServerSetupTasks to reload server to a desired stabi… #17849
Conversation
c218b18
to
013ae74
Compare
} | ||
|
||
@Override | ||
public void tearDown(ManagementClient managementClient, String containerId) throws Exception { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this be a final method with a doTearDown()
to ensure the snapshot is always restored?
013ae74
to
0a88d1d
Compare
@jamezp fixed (sorry for the delay) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@kabir In general this LGTM to me but...
- I have some minor comments
- Please rebase as this branch now has two unrelated commits that have equivalents that are already in main.
Please feel free to merge this when ready; i.e. don't wait for my formal approval of the above as I'll not be around for a while.
import org.junit.Assert; | ||
import org.junit.Test; | ||
|
||
import static org.jboss.as.controller.descriptions.ModelDescriptionConstants.CORE_SERVICE; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Static imports first.
import static org.jboss.as.controller.descriptions.ModelDescriptionConstants.RELOAD_ENHANCED; | ||
import static org.jboss.as.controller.descriptions.ModelDescriptionConstants.STABILITY; | ||
import static org.jboss.as.server.controller.descriptions.ServerDescriptionConstants.SERVER_ENVIRONMENT; | ||
import static org.junit.Assert.fail; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Static imports first.
|
||
import java.io.File; | ||
import java.util.HashSet; | ||
import java.util.Set; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
java and javax after statics but before others
* Once the test is done, the original stability level is restored. | ||
* | ||
* In order to not pollute the configuration with XML from a different stability level following the run of the test, | ||
* it takes a snapshot of the server configuration in the setup() method, and |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
and ..... ?
// Make sure the desired stability level is one of the ones supported by the server | ||
Set<Stability> supportedStabilityLevels = getSupportedStabilityLevels(managementClient); | ||
Assume.assumeTrue( | ||
String.format("%s is not a supported stability level", desiredStability, supportedStabilityLevels), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is not a valid statement -- two vars but only one expression in the format string
node.get(ModelDescriptionConstants.OP).set("take-snapshot"); | ||
ModelNode result = client.getControllerClient().execute(node); | ||
if (!"success".equals(result.get(ClientConstants.OUTCOME).asString())) { | ||
fail("Reload operation didn't finish successfully: " + result.asString()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
s/Reload/take-snapshot
…config might be incompatible with the higher one
0a88d1d
to
60ce0bd
Compare
…lity level
https://issues.redhat.com/browse/WFLY-19286
This needs the Arquillian upgrade in #17832 merged before the tests can pass
More information about the wildfly-bot[bot]