Skip to content

[WFCORE-7310] ear-exclusions-cascaded-to-subdeployments doesn't work correctly #6467

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

Merged
merged 1 commit into from
Aug 1, 2025

Conversation

mskacelik
Copy link
Contributor

@mskacelik mskacelik commented Jul 28, 2025

https://issues.redhat.com/browse/WFCORE-7310

The fix is in that I used subDeploymentMap, which holds info for all the "global" sub-deployments, instead of result.getSubDeploymentSpecifications(), which only contains the explicitly set sub-deployments.

Tested with the reproducer in the issue:
image
And also tested with integration tests within WF:
wildfly/wildfly@main...mskacelik:wildfly:WFCORE-7310-TEST (WIP, Proof of Concept)
The test alone might spark another discussion if there is even a need for the original explicit subsystem test.

Test PR: wildfly/wildfly#19095

@wildfly-ci
Copy link

Hello, mskacelik. I'm waiting for one of the admins to verify this patch with /ok-to-test in a comment.

@yersan
Copy link
Collaborator

yersan commented Jul 29, 2025

/ok-to-test

@yersan
Copy link
Collaborator

yersan commented Jul 29, 2025

Hello @mskacelik , thanks for the PR. We need to have the testcase in place in order to fully verify this. I suggest to go ahead by creating the testcase PR (without the wildfly core version bump) and let it to fail as expected. Then, we can use our CI to run an Integration Job that verifies both of your topic branches, the one for core and the one for wildfly, and that one should pass.

@yersan yersan added the 30.x label Jul 29, 2025
@wildfly-ci

This comment was marked as outdated.

@wildfly-ci

This comment was marked as outdated.

@mskacelik
Copy link
Contributor Author

mskacelik commented Jul 29, 2025

Will do a bit of investigation on why the other tests failed.

@wildfly-ci

This comment was marked as outdated.

@mskacelik
Copy link
Contributor Author

@yersan hopefully fixed now, and created a PR for the tests as you wanted wildfly/wildfly#19095
The error for the test failures was my fault, due to my not understanding a trivial for loop and its implications. Some tests seem to still fail, but they seem to be failing even before the given commit.

@yersan
Copy link
Collaborator

yersan commented Jul 30, 2025

Windows - JDK 17 (Pull Request) - merge failed test can be ignored since they are not related to this change. They are being addressed at https://issues.redhat.com/browse/WFCORE-7303

I have to take a closer look at tghis, after your changes I was expecting a possible failure on the EarJbossStructureCascadeExclusionsTestCase, but it passed.

@mskacelik
Copy link
Contributor Author

I have to take a closer look at tghis, after your changes I was expecting a possible failure on the EarJbossStructureCascadeExclusionsTestCase, but it passed.

It actually should have passed, the PR only changes that the cascade is happening on all sub-deployments (implicit), so this includes the explicit (as described in the .xml file).

@yersan
Copy link
Collaborator

yersan commented Jul 31, 2025

@yersan yersan added the ready-for-merge This PR is ready to be merged and fulfills all requirements label Aug 1, 2025
@mskacelik
Copy link
Contributor Author

Linux Integration Job: https://ci.wildfly.org/buildConfiguration/WF_WildFlyCoreIntegrationExperiments/512668

Windows Integration Job: https://ci.wildfly.org/buildConfiguration/WF_WildFlyCoreIntegrationExperimentsWindows/512670

Linux TC jobs were successful. But the Windows ones, some tests regarding permissions seem to fail, but I am not really sure why that is.

@yersan yersan merged commit febdcd7 into wildfly:main Aug 1, 2025
12 of 13 checks passed
@yersan
Copy link
Collaborator

yersan commented Aug 1, 2025

Thanks @mskacelik

@yersan
Copy link
Collaborator

yersan commented Aug 1, 2025

Linux TC jobs were successful. But the Windows ones, some tests regarding permissions seem to fail, but I am not really sure why that is.

@mskacelik
They are unrelated. Our CI is a bit sensible and we suffer from flaky tests, the trick there is to vompare whether tehy passed on other OS or inspecting the CI test history and verify that the faliled log trace has been produced on other PRs, meaning the current one is unrelated.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
30.x ready-for-merge This PR is ready to be merged and fulfills all requirements
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants