Skip to content
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.

Sign up for GitHub

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

Jump to bottom

[WFCORE-7003][WFCORE-7007] In testsuite, do not allow Bootable JAR to start in adm… #6189

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from
Draft

[WFCORE-7003][WFCORE-7007] In testsuite, do not allow Bootable JAR to start in adm… #6189

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
ef7afbb
[WFCORE-7003] In testsuite, do not allow Bootable JAR to start in adm…
yersan Sep 19, 2024
2ef4883
[WFCORE-7003] Do not run test that cannot use Bootable JAR packaging
yersan Sep 20, 2024
0e99454
[WFCORE-7007] Fix typo in pom.xml goals 'provisioning' to 'provision'
yersan Sep 20, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions testsuite/elytron/pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -286,7 +286,7 @@
<execution>
<id>server-provisioning</id>
<goals>
<goal>provisioning</goal>
<goal>provision</goal>
</goals>
<phase>none</phase>
</execution>
Expand Down Expand Up @@ -391,7 +391,7 @@
<execution>
<id>server-provisioning</id>
<goals>
<goal>provisioning</goal>
<goal>provision</goal>
</goals>
<phase>none</phase>
</execution>
Expand Down
5 changes: 3 additions & 2 deletions testsuite/manualmode/pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -406,7 +406,7 @@
<execution>
<id>server-provisioning</id>
<goals>
<goal>provisioning</goal>
<goal>provision</goal>
</goals>
<phase>none</phase>
</execution>
Expand Down Expand Up @@ -504,7 +504,7 @@
<execution>
<id>server-provisioning</id>
<goals>
<goal>provisioning</goal>
<goal>provision</goal>
</goals>
<phase>none</phase>
</execution>
Expand Down Expand Up @@ -622,6 +622,7 @@
<!-- admin mode -->
<exclude>org.wildfly.core.test.standalone.mgmt.HTTPSManagementInterfaceTestCase</exclude>
<exclude>org.wildfly.core.test.standalone.mgmt.ManagementInterfaceResourcesTestCase</exclude>
<exclude>org.wildfly.core.test.standalone.mgmt.HTTPSConnectionWithCLITestCase</exclude>
<!-- start /stop, and install extension before start -->
<exclude>org.wildfly.core.test.standalone.mgmt.PreparedResponseTestCase</exclude>

Expand Down
5 changes: 4 additions & 1 deletion .../java/org/jboss/as/test/manualmode/management/persistence/yaml/YamlExtensionTestCase.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@
import org.jboss.as.controller.client.impl.AdditionalBootCliScriptInvoker;
import org.jboss.as.controller.operations.common.Util;
import org.jboss.as.test.integration.management.util.CLIWrapper;
import org.jboss.as.test.shared.AssumeTestGroupUtil;
import org.jboss.as.test.shared.TimeoutUtil;
import org.jboss.dmr.ModelNode;
import org.jboss.logging.Logger;
Expand Down Expand Up @@ -312,7 +313,7 @@ public void testSimpleYamlWithReload() throws Exception {

@Test
public void testSimpleYamlWithCliBootOps() throws Exception {
Assume.assumeTrue("Boot CLI script can be used on only in admin-only mode which is no valid for bootable jar.", System.getProperty("ts.bootable") == null);
AssumeTestGroupUtil.assumeNotBootableJar();
StringBuilder sb = new StringBuilder();
sb.append(" -D" + AdditionalBootCliScriptInvoker.MARKER_DIRECTORY_PROPERTY + "=").append(markerDirectory.toAbsolutePath());
sb.append(" -D" + AdditionalBootCliScriptInvoker.CLI_SCRIPT_PROPERTY + "=").append(cliScript.toAbsolutePath());
Expand All @@ -330,6 +331,7 @@ public void testSimpleYamlWithCliBootOps() throws Exception {

@Test
public void testYamlChangesAppliedInAdminOnlyModeWithoutBootCliScript() throws Exception {
AssumeTestGroupUtil.assumeNotBootableJar();
container.start(null, null, Server.StartMode.ADMIN_ONLY, System.out, false, null, null, null, null, new Path[]{testYaml});
Assert.assertEquals("Yaml changes to configuration were persisted to xml. This should never happen as it's in read-only mode.", expectedXml, readConfigAsXml());
}
Expand Down Expand Up @@ -535,6 +537,7 @@ public void testIdempotence() throws Exception {

@Test
public void testYamlChangesSurviveReload() throws Exception {
AssumeTestGroupUtil.assumeNotBootableJar();
container.startYamlExtension(new Path[]{testYaml});
Assert.assertEquals("Yaml changes to configuration were persisted to xml. This should never happen as it's in read-only mode.", expectedXml, readConfigAsXml());
// read model and verify that test.yml changes are there
Expand Down
4 changes: 2 additions & 2 deletions testsuite/rbac/pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -296,7 +296,7 @@
<execution>
<id>server-provisioning</id>
<goals>
<goal>provisioning</goal>
<goal>provision</goal>
</goals>
<phase>none</phase>
</execution>
Expand Down Expand Up @@ -365,7 +365,7 @@
<execution>
<id>server-provisioning</id>
<goals>
<goal>provisioning</goal>
<goal>provision</goal>
</goals>
<phase>none</phase>
</execution>
Expand Down
9 changes: 9 additions & 0 deletions testsuite/shared/src/main/java/org/jboss/as/test/shared/AssumeTestGroupUtil.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,4 +103,13 @@ public Void run() {
private AssumeTestGroupUtil() {
// prevent instantiation
}

public static void assumeNotBootableJar() {
assumeCondition("Some tests cannot run in Bootable JAR packaging",
() -> !isBootableJar());
}

public static boolean isBootableJar() {
return System.getProperty("ts.bootable") != null;
}
}
4 changes: 2 additions & 2 deletions testsuite/standalone/pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -272,7 +272,7 @@
<execution>
<id>server-provisioning</id>
<goals>
<goal>provisioning</goal>
<goal>provision</goal>
</goals>
<phase>none</phase>
</execution>
Expand Down Expand Up @@ -366,7 +366,7 @@
<execution>
<id>server-provisioning</id>
<goals>
<goal>provisioning</goal>
<goal>provision</goal>
</goals>
<phase>none</phase>
</execution>
Expand Down
3 changes: 3 additions & 0 deletions testsuite/test-runner/src/main/java/org/wildfly/core/testrunner/Server.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -187,6 +187,9 @@ protected void start(PrintStream out) {
CommandBuilder cbuilder = null;
boolean needNormalModeCheck = false;
if (isBootableJar) {
if (this.startMode != StartMode.NORMAL) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@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?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@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.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@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

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@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.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jfdenise You didn't answer my question about StartMode.SUSPEND though. :)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@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.

throw new IllegalStateException("Bootable JAR only supports StartMode.NORMAL, however " + this.startMode + " was requested.");
}
final Path bootableJarPath = Paths.get(bootableJar);
if (Files.notExists(bootableJarPath) || Files.isDirectory(bootableJarPath)) {
throw new IllegalStateException("Cannot find: " + bootableJar);
Expand Down
Loading