Skip to content

Fix check for ear dependency for dependencies of type ejb#1942

Merged
cherylking merged 5 commits intoOpenLiberty:mainfrom
arend-von-reinersdorff:patch-1
Dec 4, 2025
Merged

Fix check for ear dependency for dependencies of type ejb#1942
cherylking merged 5 commits intoOpenLiberty:mainfrom
arend-von-reinersdorff:patch-1

Conversation

@arend-von-reinersdorff
Copy link
Contributor

Fixes #1941

This extended check is already used in DeployMojoSupport.addSkinnyArtifactLib()

I tested the fix locally. There is a test project dealing with skinny wars already: https://github.com/OpenLiberty/ci.maven/tree/main/liberty-maven-plugin/src/it/dev-it/resources/multi-module-projects/multipleLibertyModules Maybe it could be extended with an EJB dependency? But I don't know how to run these tests.

@venmanyarun
Copy link
Contributor

venmanyarun commented Nov 24, 2025

Hi @arend-von-reinersdorff
Please refer https://github.com/OpenLiberty/ci.maven/blob/main/README.md#build to run tests in local
Yes you need to update tests
https://github.com/OpenLiberty/ci.maven/tree/main/liberty-maven-plugin/src/it/dev-it/resources/multi-module-projects/multipleLibertyModules -> add an ejb module here and update dependencies in ear-skinny-modules module , war module etc
and you need to update ear-project-it and add ejb module for skinnywar-ear and update the necessary tests too

Also take latest update from main to resolve PR check failure issue

Copy link
Contributor

@venmanyarun venmanyarun left a comment

Choose a reason for hiding this comment

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

I see that you have updated the test project , but no changes in

public class MultipleLibertyModulesSkinnyModulesTest extends BaseMultiModuleTest {

and
https://github.com/OpenLiberty/ci.maven/blob/5378fc245f517e284898b7b1815da3ba841bf84f/liberty-maven-plugin/src/it/dev-it/resources/multi-module-projects/multipleLibertyModules/ear-skinny-modules/src/test/java/it/io/openliberty/guides/multimodules/ConverterAppIT.java

You need to update ConverterAppIT to see whether the ear.xml is containing the ejb in the correct position

@arend-von-reinersdorff
Copy link
Contributor Author

I see that you have updated the test project , but no changes in

public class MultipleLibertyModulesSkinnyModulesTest extends BaseMultiModuleTest {

and https://github.com/OpenLiberty/ci.maven/blob/5378fc245f517e284898b7b1815da3ba841bf84f/liberty-maven-plugin/src/it/dev-it/resources/multi-module-projects/multipleLibertyModules/ear-skinny-modules/src/test/java/it/io/openliberty/guides/multimodules/ConverterAppIT.java

You need to update ConverterAppIT to see whether the ear.xml is containing the ejb in the correct position

Hi @venmanyarun, thanks for looking it over. I adapted ConverterAppIT now and it behaves as expected: Succeeds with the main code change and fails without it.

There are other failing tests in the dev-it directory, but they mostly also fail for me when running them on main.

@venmanyarun
Copy link
Contributor

venmanyarun commented Dec 1, 2025

@arend-von-reinersdorff
Thank you very much for the contribution!!

Looks good to me.. @cherylking please check

@venmanyarun
Copy link
Contributor

Hi @arend-von-reinersdorff
can you confirm whether you have followed the process for contributor agreement
Please read through the document and signoff on the CLA

@arend-von-reinersdorff
Copy link
Contributor Author

Hi @arend-von-reinersdorff can you confirm whether you have followed the process for contributor agreement Please read through the document and signoff on the CLA

Hi @venmanyarun, submitted the CLA via email now

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.

Looks good to me. Thanks for the contribution!

@cherylking
Copy link
Member

Confirmed we received the CLA and it has been approved. Thank you!

@cherylking cherylking merged commit 6307066 into OpenLiberty:main Dec 4, 2025
23 of 24 checks passed
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.

Deployment is wrong for skinny war with EJB dependency

3 participants