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.

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

<springBootApplication/> issue fix #1844

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

arunvenmany-ibm
Copy link
Contributor

Making change to create thin application with same name as provided in server.xml node

…n server.xml <springBootApplication/> node

Signed-off-by: Arun Venmany <[email protected]>
Copy link
Member

@cherylking cherylking left a comment

Choose a reason for hiding this comment

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

I think the changes are good, but we need some negative tests added.

  1. More than one springBootApplication element specified to trigger the exception. Need to use invoker.properties for that.
  2. Specify the apps folder but do not provide the configuration for the springBootApplication and test that the config gets generated in the configDropins folder.

<featureManager>
<feature>appSecurityClient-1.0</feature>
<feature>jaxws</feature>
<platform>badname</platform>
Copy link
Member

Choose a reason for hiding this comment

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

Why do you have a bad platform name here? And I don't see multiple springBootApplication elements here which is what the test is supposed to be for.

@@ -27,6 +27,9 @@ public void testThinApplicationExistsInAppsDirectory() throws Exception {

File f = new File("target/liberty/wlp/usr/servers/test/apps/thin-springboot-3-appsdirectory-apps-it-1.0.0.Final-exec.jar");
assertTrue(f.getCanonicalFile() + " doesn't exist. Plugin failed to place the file at right destination.", f.exists());
File f2 = new File("target/liberty/wlp/usr/servers/test/configDropins/defaults");
assertTrue(f2.getCanonicalFile() + " folder doesn't exist. Plugin failed to create the config dropins folder ", f2.exists());
assertTrue(f2.getCanonicalFile() + " folder doesn't contain any files. Plugin failed to place config xml at right destination.", f2.list().length == 1);
Copy link
Member

Choose a reason for hiding this comment

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

Can we verify the contents of the generated file?

@@ -9,6 +9,9 @@ switch (testname) {
case "kernel-install-versionless-feature-fail-no-platform-it":
expectedMessage = "CWWKF1516E: The platform could not be determined. The following versionless features cannot be installed: [servlet]";
break;
case "springboot-3-deploy-multiple-springboot-application-nodes-fail-it":
Copy link
Member

Choose a reason for hiding this comment

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

If we are going to add other tests to this script, we should rename it probably.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants