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

[WFLY-18479][WFLY-18493][WFLY-18523] the starting point for the CY202… #696

Merged
merged 1 commit into from
Sep 25, 2023

Conversation

emmartins
Copy link
Contributor

@emmartins emmartins commented Aug 30, 2023

…3Q3 QS enhancements, for now targeting only helloworld and microprofile-config

Issues: https://issues.redhat.com/browse/WFLY-18479, https://issues.redhat.com/browse/WFLY-18493, https://issues.redhat.com/browse/WFLY-18523

The following enhancements are included:

  • maven project version properties rework
  • maven project plugin management
  • server provisioning for non MP QS
  • unified openshift build for all QS, including MP ones, using the standard server provisioning (no bootable jar)
  • common functionality to provide basic runtime testing for all QS, including MP ones (integration-testing maven profile, Abstract + BasicRuntimeIT, github actions CI)
  • project CI as specified by WFLY-18523
  • reworked asciidoc sections wrt build, run and test for bare metal, server provisioning/bootable-jar, openshift

@emmartins
Copy link
Contributor Author

WiP, not for merge

@emmartins emmartins force-pushed the post-eap8-qs-enhance branch 8 times, most recently from de40f87 to a7ca0de Compare September 7, 2023 18:04
@emmartins emmartins mentioned this pull request Sep 18, 2023
@emmartins emmartins force-pushed the post-eap8-qs-enhance branch 3 times, most recently from 3f1684e to 6fdb03e Compare September 21, 2023 13:38
@emmartins emmartins changed the title Initial work for the 'Post EAP 8' QS enhancements, for now targeting … [WFLY-18479][WFLY-18493][WFLY-18523] the starting point for the CY202… Sep 21, 2023
@emmartins
Copy link
Contributor Author

The PoC was cleaned and the commit now has the JIRA key tags, IMHO this is now ready to be merged, and thus unlock the work on all other QS.

@emmartins
Copy link
Contributor Author

emmartins commented Sep 21, 2023

@zhantemirov @ehsavoie @xstefank fyi I would like to merge this tomorrow, I know that's not much time for a review but by now you guys should be aware of what are the enhancements, and of course later we will have time for optimizations/bugfixes, what matters most now is to provide the base for the enhancement of all QS, as defined by https://issues.redhat.com/browse/WFLY-18411

As an example there were existent arquillian tests on microprofile-config, which I simply disabled here. Further work is needed by Martin, to rework those since managed arquilian is not OpenShift compatible. We can simply keep WFLY-18493 open till a second PR is added there by Martin.

Copy link
Member

@xstefank xstefank left a comment

Choose a reason for hiding this comment

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

LGTM, some small suggestions

<version>${version.plugin.wildfly-jar}</version>
</plugin>
</plugins>
</pluginManagement>
Copy link
Member

Choose a reason for hiding this comment

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

why pluginManagement?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

to be able to use plugins on CLI (e.g. wildfly:deploy, wildfly-jar:start), up till now this was being inherited from parent but the QS pom.xml should have all the user app needs. In our parent we should only hide what is WFLY devs only (e.g. checkstyle)

<layers>
<layer>jaxrs-server</layer>
<layer>cloud-server</layer>
Copy link
Member

Choose a reason for hiding this comment

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

this is intended? Layers should be the same IMO, or I don't understand why it is required.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The openshift profile now uses standard server provisioning for all, not bootable-jar for MP apps, and I was told the cloud-server layer is what should be used.

final HttpResponse<String> response = client.send(request, HttpResponse.BodyHandlers.ofString());
assertEquals(200, response.statusCode());
}
}
Copy link
Member

Choose a reason for hiding this comment

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

Consider refactoring this class into a separate module that can be shared between all QSs.

Copy link
Contributor Author

@emmartins emmartins Sep 22, 2023

Choose a reason for hiding this comment

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

This is pretty much the thing I have thought but in the end could not commit myself too go for it... I know this will add a bit of weird maintenance, after all we are talking about 49 quickstarts, but:

  • each quickstart should be a fully independent project, we should include in its module all the user app needs
  • it's really just one class, it would be weird to go for an SPI only containing it
  • I can't imagine we changing it much

Honestly I think I would prefer to merge the abstract logic on the tests classes, than creating a test SPI (which is more of an internal testing decision)

…3Q3 QS enhancements, for now targeting only helloworld and microprofile-config

The following enhancements are included:
* maven project version properties rework
* maven project plugin management
* server provisioning for non MP QS
* unified openshift build for all QS, including MP ones, using the standard server provisioning (no bootable jar)
* common functionality to provide basic runtime testing for all QS, including MP ones (integration-testing maven profile, Abstract + BasicRuntimeIT, github actions CI)
* project CI as specified by WFLY-18523
* reworked asciidoc sections wrt build, run and test for bare metal, server provisioning/bootable-jar, openshift
@emmartins emmartins merged commit 5698e3a into wildfly:main Sep 25, 2023
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.

2 participants