[WFLY-19062] Add preview stability support for Jakarta MVC to standard WildFly #17766

bstansberry · 2024-03-27T02:09:10Z

https://issues.redhat.com/browse/WFLY-19062

This also incorporates fixes for:

https://issues.redhat.com/browse/WFLY-19170 (see also #17764)
https://issues.redhat.com/browse/WFLY-19163 (see also #17753)

More information about the wildfly-bot[bot]

jfdenise · 2024-03-27T10:00:58Z

testsuite/shared/src/main/java/org/jboss/as/test/shared/LayersTestBase.java

@@ -83,7 +83,13 @@ public abstract class LayersTestBase {
     * Included in the return value of {@link #getExpectedUnusedInAllLayers()}
     * only when testing provisioning from the wildfly feature pack.
     */
-    public static final String[] NO_LAYER_WILDFLY = {};
+    public static final String[] NO_LAYER_WILDFLY = {
+            // Preview stability 'mvc-krazo' layer cannot be provisioned in OOTB standard wildfly


When reading the comment, I thought at default standalone config. <perhaps adding a mention of layer.

jfdenise · 2024-03-27T10:02:49Z

testsuite/layers-expansion/pom.xml

@@ -760,6 +760,7 @@
                                                <layer>messaging-activemq</layer>
                                                <layer>micrometer</layer>
                                                <layer>mod_cluster</layer>
+                                                <layer>mvc-krazo</layer>


Perhaps add a comment that says that being preview, nothing is provisioned although present. I understand that this comment would have to be removed when promoting this subsystem.

This one is a mistake. It should not be in this execution until it will actually be provisioned. Adding it here would be part of promoting the feature to community.

But commenting it out with a comment is better than deleting it. :)

Done. (I moved the whole thing lower into the section with 'wildfly' FP only layers.)

jfdenise · 2024-03-27T10:03:10Z

testsuite/layers-expansion/pom.xml

@@ -869,6 +870,7 @@
                                                <layer>messaging-activemq</layer>
                                                <layer>micrometer</layer>
                                                <layer>mod_cluster</layer>
+                                                <layer>mvc-krazo</layer>


Perhaps add a comment that says that being preview, nothing is provisioned although present. I understand that this comment would have to be removed when promoting this subsystem.

I think this is provisioned as this execution uses wildfly-preview; i'll check.

I think that is the wildfly full all layers for HA.

I misread where in the code this line is. +1 -- this is the same as your other comment above and I'll do the same re commenting it out.

@jfdenise Done. (I moved the whole thing lower into the section with 'wildfly' FP only layers.)

scottmarlow · 2024-03-28T16:59:37Z

Pasting from https://issues.redhat.com/browse/WFLY-19062:

Adding it to wildfly-ee instead is also a possibility to consider, but as Jakarta MVC is not yet a part of any Jakarta EE platform or profile, I think it's reasonable to consider it as more subject to incompatible change than most wildfly-ee technologies, and thus a better fit for 'wildfly'.

👍 That makes sense!

scottmarlow · 2024-03-28T17:04:32Z

boms/common-ee/pom.xml

@@ -4808,6 +4808,37 @@
                <version>${version.org.reactivestreams}</version>
            </dependency>

+            <dependency>


Should non-EE artifacts be added to boms/common-ee or boms/common-expansion?

@scottmarlow Non-EE artifacts can go in common-ee. But I realize now these particular ones should be in common-expansion not common-ee.

The 'ee' refers to the 'wildfly-ee' feature pack, which is much more than EE; it's just the name we chose for the 'base' FP back in the WF 19/20 days we when split out the 'expansion' stuff. Down stream the wildfly-ee feature pack becomes the 'eap' feature pack used for base EAP (which is much more the Jakarta EE), while the 'wildfly' feature pack becomes the 'eap-xp' FP used for EAP XP. Upstream I refer to things specific to the 'wildfly' feature pack (boms, testsuite modules etc) as 'expansion'. But the feature pack itself is just 'wildfly' because in community 99%+ of people only deal with that FP directly, with wildfly-ee as an internal detail.

The boms/xxx-ee modules are meant to relate to wildfly-ee, while boms/xxx-expansion relate to the 'wildfly' feature pack. These deps are only relevant to the 'wildfly' FP so I will move them to boms/common-expansion.

Thanks!

@scottmarlow I've moved these to common-expansion. While doing it I realized one of them wasn't needed at all so I removed that.

When doing that I also rebased on current main and took the opportunity to squash down from three commits to two.

scottmarlow · 2024-03-28T17:23:22Z

Pasting from eclipse-ee4j/krazo#352:

We could implement a PoC for a global WildFly module, so we can run the TCK against it and submit it as PR to WildFly itself.

@erdlet could it make sense for the WildFly project to run the Eclipse Krazo TCK against the https://github.com/eclipse-ee4j/krazo/tree/master/tck? Perhaps via the https://ci.eclipse.org/krazo/job/tck-wildfly job?

erdlet · 2024-03-28T17:44:22Z

Hi @scottmarlow

thanks for integratin Krazo into Wildfly.

TBH I retired from Krazo,so I guess its better to ask @ivargrimstad or @chkal.

Sorry that I can't help you more

bstansberry · 2024-03-28T20:45:51Z

@scottmarlow We have a TCK runner for this:

https://github.com/wildfly-extras/mvc-krazo-feature-pack/tree/main/testsuite/tck

That can be run against any WildFly release by passing system props to maven. The README in that module describes that. Snapshots can be tested locally that way. WildFly Preview 31 passes that, and I'm confident this PR does too. (An earlier variant did but I've tweaked some irrelevant stuff since I last tried.)

I'm going to add a github action to that repo that will take a GH repo and tag/branch/sha as params, values meant to point to a wildfly repo. It will then check out that repo and build wildfly, making the resulting maven repository from the build available to a call to run the TCK. This will allow testing WF snapshots on GH instead of locally. That action could then be called from some broader infrastructure, e.g. if we wanted some action in wildfly-tck-runners to call everything, or perhaps a jenkins job.

Note that I expect that repo may some day go away and the various pieces get moved, e.g. the bulk of it into this repo and the tck runner as a subtree in wildfly-tck-runners. But for now I like having it external as it gives me experience of the issues that come with maintaining something like this externally to the main WF code base. That something we'll be doing more of over time and it's good to build expertise.

darranl · 2024-04-02T10:31:23Z

Just adding the "Critical" label so we can discuss.

This primarily consists of shifting the mvc-krazo extension/subsystem integration from the wildfly-preview feature pack to the wildfly feature pack Manual mode testing is added since the server needs to run at --stability=preview. This will be converted to the basic testsuite once WFCORE-6728 is available for use

bstansberry · 2024-04-03T14:18:19Z

TCK results are here: https://github.com/wildfly-extras/mvc-krazo-feature-pack/actions/runs/8539883565/

scottmarlow · 2024-04-03T14:44:07Z

Nice to see the TCK tests passed! Tests run: 138, Failures: 0, Errors: 0, Skipped: 0

jamezp

I"m approving this because I don't think my comments should blocking merging and can be changed in a follow up PR.

jamezp · 2024-04-03T14:35:28Z

...on/manualmode-expansion/src/test/java/org/wildfly/test/manual/mvc/MVCKrazoSmokeTestCase.java

+    @Test
+    @InSequence(-1)


This should not block merging, but any reason not to do this in a @Before method?

jamezp · 2024-04-03T14:40:53Z

...on/manualmode-expansion/src/test/java/org/wildfly/test/manual/mvc/MVCKrazoSmokeTestCase.java

+    public void test() throws Exception {
+        // TODO migrate this to testsuite/integration/basic and inject the http listener URL
+        //URL url = new URL(this.url.toExternalForm() + "mvc-smoke/test");
+        URL url = new URL("http", TestSuiteEnvironment.getServerAddress(), 8080, "/" + DEPLOYMENT_NAME + "/mvc-smoke/test");


This should be:

URL url = new URL("http", TestSuiteEnvironment.getHttpAddress(), TestSuiteEnvironment.getHttpPort(), "/" + DEPLOYMENT_NAME + "/mvn-smoke/test");

jamezp · 2024-04-03T14:41:23Z

...on/manualmode-expansion/src/test/java/org/wildfly/test/manual/mvc/MVCKrazoSmokeTestCase.java

+    @Test
+    @InSequence(10)


Same as above having this as an @After should work.

bstansberry added Feature PR provides a new feature missing-reqs Features missing one or more of the pre-merge requirements 32.x WildFly 32 labels Mar 27, 2024

wildfly-bot bot requested a review from yersan March 27, 2024 02:09

github-actions bot added the deps-ok Dependencies have been checked, and there are no significant changes label Mar 27, 2024

jfdenise approved these changes Mar 27, 2024

View reviewed changes

bstansberry mentioned this pull request Mar 27, 2024

[WFLY-19164] Move MVCKrazoSmokeTestCase to testsuite/preview/manualmode #17753

Closed

bstansberry force-pushed the WFLY-19062 branch from 3a0a0f7 to 5fdd68c Compare March 27, 2024 14:53

scottmarlow reviewed Mar 28, 2024

View reviewed changes

scottmarlow mentioned this pull request Mar 28, 2024

PoC of WildFly module providing Krazo eclipse-ee4j/krazo#352

Open

bstansberry force-pushed the WFLY-19062 branch from 5fdd68c to 4ace70c Compare March 28, 2024 21:03

darranl added the Critical Doesn't block a release but deserves higher priority than most. Use sparingly. label Apr 2, 2024

bstansberry added 2 commits April 2, 2024 20:32

[WFLY-19163] Create testsuite/preview/manualmode

f519522

bstansberry force-pushed the WFLY-19062 branch from 4ace70c to a53eba0 Compare April 3, 2024 02:34

scottmarlow approved these changes Apr 3, 2024

View reviewed changes

jamezp approved these changes Apr 3, 2024

View reviewed changes

darranl merged commit 4b8b19f into wildfly:main Apr 3, 2024
15 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[WFLY-19062] Add preview stability support for Jakarta MVC to standard WildFly #17766

[WFLY-19062] Add preview stability support for Jakarta MVC to standard WildFly #17766

bstansberry commented Mar 27, 2024 •

edited by wildfly-bot bot

Loading

jfdenise Mar 27, 2024

jfdenise Mar 27, 2024

bstansberry Mar 27, 2024

bstansberry Mar 27, 2024

jfdenise Mar 27, 2024

bstansberry Mar 27, 2024

jfdenise Mar 27, 2024

bstansberry Mar 27, 2024

bstansberry Mar 27, 2024

scottmarlow commented Mar 28, 2024

scottmarlow Mar 28, 2024

bstansberry Mar 28, 2024 •

edited

Loading

bstansberry Mar 28, 2024

scottmarlow commented Mar 28, 2024

erdlet commented Mar 28, 2024

bstansberry commented Mar 28, 2024

darranl commented Apr 2, 2024

bstansberry commented Apr 3, 2024

scottmarlow commented Apr 3, 2024

jamezp left a comment

jamezp Apr 3, 2024

jamezp Apr 3, 2024

jamezp Apr 3, 2024

[WFLY-19062] Add preview stability support for Jakarta MVC to standard WildFly #17766

[WFLY-19062] Add preview stability support for Jakarta MVC to standard WildFly #17766

Conversation

bstansberry commented Mar 27, 2024 • edited by wildfly-bot bot Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

scottmarlow commented Mar 28, 2024

Choose a reason for hiding this comment

bstansberry Mar 28, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

scottmarlow commented Mar 28, 2024

erdlet commented Mar 28, 2024

bstansberry commented Mar 28, 2024

darranl commented Apr 2, 2024

bstansberry commented Apr 3, 2024

scottmarlow commented Apr 3, 2024

jamezp left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bstansberry commented Mar 27, 2024 •

edited by wildfly-bot bot

Loading

bstansberry Mar 28, 2024 •

edited

Loading