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-6994] Add ModelTestControllerVersion.EAP_XP_5 #6175

Merged
merged 1 commit into from
Sep 12, 2024
Merged

[WFCORE-6994] Add ModelTestControllerVersion.EAP_XP_5 #6175

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
3 changes: 2 additions & 1 deletion model-test/src/main/java/org/jboss/as/model/test/ModelTestControllerVersion.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,8 @@ public enum ModelTestControllerVersion {
//EAP releases
EAP_7_4_0("7.4.0.GA-redhat-00005", true, "23.0.0", "15.0.2.Final-redhat-00001", "7.4.0"),
EAP_8_0_0("8.0.0.GA-redhat-00011", true, "29.0.0", "21.0.5.Final-redhat-00001", "8.0.0"),
EAP_XP_4("4.0.0.GA-redhat-00003", true, "23.0.0", "15.0.26.Final-redhat-00001", "xp4");
EAP_XP_4("4.0.0.GA-redhat-00003", true, "23.0.0", "15.0.26.Final-redhat-00001", "xp4"),
EAP_XP_5("5.0.0.GA-redhat-00005", true, "29.0.0", "21.0.5.Final-redhat-00001", "xp5");
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think this is going to require additional changes, if not in core, in the code that needs to reference these artifacts since the EAP maven G for XP artifacts changed from org.jboss.eap to org.jboss.eap.xp

Copy link
Collaborator

Choose a reason for hiding this comment

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

Also, I wonder, won't be necessary to have the expected controllers available from https://github.com/wildfly/wildfly-legacy-test/?

Copy link
Collaborator

@yersan yersan Sep 12, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Also, I wonder, won't be necessary to have the expected controllers available from https://github.com/wildfly/wildfly-legacy-test/?

Ok, I think I partially got this, it is using model controllers for 29.0.0, which were introduced in EAP_8_0_0, so there is no need to add new ones in https://github.com/wildfly/wildfly-legacy-test.

The only pending detail is about resources as https://github.com/wildfly/wildfly-legacy-test/blob/main/tools/src/main/resources/legacy-models/standalone-resource-definition-xp4.dmr, and if we will need the -xp5 variants, but I am not fully sure that we are still using these files in the tests. If so, I guess we can add them later

@bstansberry WDYT?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

TBH I don't remember. @kabir what do these files do?

Jason got a subsystem transformer test to work using just this branch, so maybe it's not necessary. Or maybe it didn't really work but just seemed like it did.

Copy link
Contributor

Choose a reason for hiding this comment

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

I could only find reference to '.dmr' in a few files in subsystem-test, so I don't think this is relevant for the core tests.

Looking at TransforrmationUtils.java, it has this code

    static ResourceDefinition getResourceDefinition(KernelServices kernelServices, ModelVersion modelVersion, String mainSubsystemName) throws IOException {
        //Look for the file in the org.jboss.as.subsystem.test package - this is where we used to store them before the split
        ResourceDefinition rd = TransformationUtils.loadSubsystemDefinitionFromFile(TransformationUtils.class.getClass(), mainSubsystemName, modelVersion);

        if (rd == null) {
            //This is the 'new' post-split way. First check for a cached .dmr file. This which also allows people
            //to override the file for the very rare cases where the rd needed touching up (probably only the case for 7.1.x descriptions).
            File file = getDmrFile(kernelServices, modelVersion, mainSubsystemName);
            if (!file.exists()) {
                generateLegacySubsystemResourceRegistrationDmr(kernelServices, modelVersion, mainSubsystemName, file.toPath());
            }
            //System.out.println("Using legacy resource definition dmr: " + file);
            rd = TransformationUtils.loadSubsystemDefinitionFromFile(kernelServices.getTestClass(), mainSubsystemName, modelVersion);
        }
        return rd;
    }

The first part tries to load from a .dmr file, and then the second part queries the legacy controller for the model and saves to a 'cache' file.

So as far as I can tell we should be fine without. If not, we will find out and can add them :-)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks @kabir. So it sounds like it's a performance improvement thing.

Copy link
Contributor

Choose a reason for hiding this comment

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

@bstansberry I think the original .dmr stuff was mine, and then someone (perhaps @ctomc) realised we could just grab it from the legacy model and evolved it


private final String mavenGavVersion;
private final String testControllerVersion;
Expand Down
Loading