[WFCORE-7003][WFCORE-7007] In testsuite, do not allow Bootable JAR to start in adm… #6189
Conversation
There was a problem hiding this comment.
@yersan LGTM. We'll see if we need to fix any tests that somehow work. There may be some where admin-only isn't really needed. The existing (before Darran's update) ManagementInterfaceResourcesTestCase is an example of such a test.
|
@bstansberry yes, there are some need to be fixed indeed |
|
In full WF the only use of these methods is one call in org.wildfly.test.manual.management.ManagementOnlyModeTestCase, and that module doesn't run in bootable jar test profiles. So this should be fine there. I note this because CI for this PR wouldn't catch any problems in full WF as it doesn't exercise bootable jar there. |
This comment was marked as outdated.
This comment was marked as outdated.
| @@ -187,6 +187,9 @@ protected void start(PrintStream out) { | |||
| CommandBuilder cbuilder = null; | |||
| boolean needNormalModeCheck = false; | |||
| if (isBootableJar) { | |||
| if (this.startMode != StartMode.NORMAL) { | |||
There was a problem hiding this comment.
@jfdenise if this is true, it means 'startMode' is either 'ADMIN_ONLY' or 'SUSPEND'. Is failing the right fix for 'SUSPEND'? Or is it perhaps just that we're not configuring the bootable jar launch for that when we could?
There was a problem hiding this comment.
@bstansberry , looking at how tests evolved, it appears that some tests start the bootable JAR in adminMode and succeed (no restart of the server). I ran some tests that we are excluding (because of adminMode) and they are passing. I think that the reasoning was: we are not exposing the option to start a bootable JAR in admin mode, so we excluded tests that use admin mode although they could succeed.
There was a problem hiding this comment.
@jfdenise I am a bit lost because Brian's question was more about starting them in SUSPEND.
So, the problem with the Bootable JAR is not about whether it can be started in admin-only (or suspend), it is more that we do not "expose" those options? What does "expose" mean here? are we talking about it is not documented as a valid option? .... my very naive understanding is that changes are not persisted, so if you start in admin-only to later move to normal, as soon as you restart instead of reload, your changes will be lost. In such a case, I guess starting in SUSPEND to go later to NORMAL could be valid as long as you do not modify the server configuration
There was a problem hiding this comment.
@jfdenise Unless you edited the class we're discussing here to actually pass through the --admin-only to the bootable jar process, enabling more tests (I assume by removing pom.xml exclude elements) with the existing code basically demonstrates that the test didn't need to use startInAdminMode at all. That was definitely the case with ManagementInterfaceResourcesTestCase.
So perhaps before merging this we should enable a lot of tests and see which pass. If they pass, the test doesn't need StartMode.ADMIN_ONLY and we can can fix them to not use it and leave them enabled.
There was a problem hiding this comment.
@jfdenise You didn't answer my question about StartMode.SUSPEND though. :)
There was a problem hiding this comment.
@bstansberry , for SUSPEND I would say that we should allow it and set the mode. I don't see why a Bootable JAR couldn't be started in SUSPEND. So perhaps this fix is too radical.
…in-only or suspend modes Jira issue: https://issues.redhat.com/browse/WFCORE-7003
yersan
changed the title
|
Core -> Full Integration Build 13935 outcome was FAILURE using a merge of 0e99454 Failed tests |
…in-only or suspend modes
Jira issue: https://issues.redhat.com/browse/WFCORE-7003
Jira issue: https://issues.redhat.com/browse/WFCORE-7007