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-17651] Add a new "Getting Started" archetype. #26

Closed
wants to merge 24 commits into from

Conversation

darranl
Copy link
Contributor

@darranl darranl commented Feb 19, 2023

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

Starting off as a draft as a couple of additional points still to consider:

  • Check CDI works
  • Look at the test profiles
  • Consider do we need Helm charts?

@jmesnil
Copy link
Member

jmesnil commented Feb 21, 2023

Consider do we need Helm charts?

We would need a Helm Chart to deploy the application on Kubernetes (instead of a plain container image deployment).
However this implies that the user pushes the container image that is created by the wildfly:image plugin to a public container registry (ghcr.io or quay.io)

The Helm chart could be as simple as:

image:
  name: <name of the image in the container registry>
build:
  enabled: false
deploy:
  route:
    enabled: false

The underlying question is how far do you want to push the getting started experience?

If its's up to local development, the image is fine.
If it's up to kubernetes deployment, then we need the configuration/templating to push the image to a container registry and a Helm chart to deploy on a k8s cluster

@darranl
Copy link
Contributor Author

darranl commented Feb 21, 2023

@jmesnil I think after this the next discussion is documentation of user stories, as the Helm chart is so simple I don't think it fits in the archetype, instead it feels like it is a next user story re using a Helm Chart to deploy the project.

jmesnil and others added 2 commits August 9, 2023 13:53
* Rely on the provisioned server + deployments to run the integration tests
* Use a single managed Arquillian container to run the tests
* Do not use profiles for testing, with this settings, there is no reason that integration tests should be skipped.

Signed-off-by: Jeff Mesnil <jmesnil@redhat.com>
@darranl
Copy link
Contributor Author

darranl commented Aug 9, 2023

Thanks @jmesnil I have merged that first PR

@jmesnil
Copy link
Member

jmesnil commented Aug 9, 2023

@darranl thanks! If you look at the GitHub action, you will now have the execution of the mvn verify for a generated project (eg https://github.com/wildfly/wildfly-archetypes/actions/runs/5808924000/job/15746706526#step:4:4206).

to use the same package and prefix that the application classes.

Signed-off-by: Jeff Mesnil <jmesnil@redhat.com>
Update packaging of the projects tests
@darranl
Copy link
Contributor Author

darranl commented Aug 9, 2023

@jmesnil second PR merged ;-) thank you

jmesnil and others added 13 commits August 9, 2023 15:19
Signed-off-by: Jeff Mesnil <jmesnil@redhat.com>
* Use `org.wildfly.examples` for the `groupId` and `package` property
* Use `getting-started` for the `artifactId` property
* Use `1.0.0-SNAPSHOT` for the `version` property

By default, the project will use these value to genereate the project and the command is simply:

```
mvn archetype:generate -DarchetypeGroupId=org.wildfly.archetype -DarchetypeArtifactId=wildfly-getting-started-archetype
```

It is still possible to update these values with a system property (eg `-DgroupId=org.acme`) or interactively by rejecting the properties configuration and specify new values.

Signed-off-by: Jeff Mesnil <jmesnil@redhat.com>
Use the artifactId instead of the hard-coded `login-form`

Signed-off-by: Jeff Mesnil <jmesnil@redhat.com>
Use name instead of runtimeName to deploy the application.
RuntimeName is deprecated and would not work well with wildfly:dev goal

Signed-off-by: Jeff Mesnil <jmesnil@redhat.com>
Signed-off-by: Jeff Mesnil <jmesnil@redhat.com>
This lets the user know where that page is coming from.
If their app is not a frontend, they will likely want to get rid
of this page (and associated css resources).

Signed-off-by: Jeff Mesnil <jmesnil@redhat.com>
this is so fast that it looks weird

Signed-off-by: Jeff Mesnil <jmesnil@redhat.com>
Add a CDI injection
Add an unit test to test the CDI service

This sets up the user to do both unit testing (before creating the war) and integration testing (against the deployed app)

Signed-off-by: Jeff Mesnil <jmesnil@redhat.com>
Signed-off-by: Jeff Mesnil <jmesnil@redhat.com>
Signed-off-by: Jeff Mesnil <jmesnil@redhat.com>
@jmesnil
Copy link
Member

jmesnil commented Aug 16, 2023

@darranl you need to bump the wildfly-maven-plugin to 4.2.0.Final to be able to provision WildFly 29.

@darranl
Copy link
Contributor Author

darranl commented Aug 16, 2023

@jmesnil sorry I had made that change locally, hadn't pushed the last commit.

<!-- other plugin versions -->
<version.compiler.plugin>3.9.0</version.compiler.plugin>
<version.surefire.plugin>2.22.2</version.surefire.plugin>
<version.failsafe.plugin>2.22.2</version.failsafe.plugin>
Copy link
Member

Choose a reason for hiding this comment

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

can be bumped to 3.1.2

<version.compiler.plugin>3.9.0</version.compiler.plugin>
<version.surefire.plugin>2.22.2</version.surefire.plugin>
<version.failsafe.plugin>2.22.2</version.failsafe.plugin>
<version.war.plugin>3.3.2</version.war.plugin>
Copy link
Member

Choose a reason for hiding this comment

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

can be bumped to 3.4.0

</plugin>

<!--Build configuration for the WAR plugin: -->
<plugin>
Copy link
Member

Choose a reason for hiding this comment

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

@darranl the indentation is all over the place (I definitely mess it up in all my commits). Before merging, this pom.xml should be reindented.

Copy link
Member

Choose a reason for hiding this comment

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

done in jmesnil@d792dd3


<!-- other plugin versions -->
<version.compiler.plugin>3.9.0</version.compiler.plugin>
<version.surefire.plugin>2.22.2</version.surefire.plugin>
Copy link
Member

Choose a reason for hiding this comment

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

afaict, this version is useless as the surefire plugin is not explicitly configured in this pom.xml.

*/
public class ${defaultClassPrefix}ServiceTest {

${defaultClassPrefix}Service service = new ${defaultClassPrefix}Service();
Copy link
Member

@jmesnil jmesnil Aug 17, 2023

Choose a reason for hiding this comment

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

This is actually a stupid idea of mine to write such unit test for a CDI bean. This will break if the bean use any injection itself.

We should remove this test file completely

Copy link
Member

Choose a reason for hiding this comment

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

or set it up as an Arquillian integration tests with a deployment, etc.

Copy link
Member

Choose a reason for hiding this comment

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

done in darranl#7

jmesnil and others added 3 commits August 18, 2023 10:05
The changes in arquillian.xml was making the test fails but this was not captured by the verify goal

Signed-off-by: Jeff Mesnil <jmesnil@redhat.com>
Signed-off-by: Jeff Mesnil <jmesnil@redhat.com>
@jmesnil
Copy link
Member

jmesnil commented Sep 5, 2023

@darranl starting with #40, all dependencies are configured in the parent pom.xml and propagated to the generated archetypes. This requires an update in your PR to move the generated pom.xml in a separate directory and some escaping of its variables. This should be very similar to what was done for the jakartaee-webapp archetype (starting from https://github.com/wildfly/wildfly-archetypes/pull/40/files#diff-03e5203acbea22fcd42193af53307eb5b7ac9ffd50ede79de2bf4ad7b4e4cd64)

@jmesnil
Copy link
Member

jmesnil commented Sep 6, 2023

@darranl starting with #40, all dependencies are configured in the parent pom.xml and propagated to the generated archetypes

done in jmesnil@d792dd3

<dependency>
<groupId>org.jboss.resteasy</groupId>
<artifactId>resteasy-client</artifactId>
<version>6.2.4.Final</version>
Copy link
Contributor

Choose a reason for hiding this comment

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

For consistency, I would move this version to the properties section

@darranl
Copy link
Contributor Author

darranl commented Sep 29, 2023

FYI I am going to close this PR as I opened #47 as a continuation after merging in main and syncing from Jeff's topic branch.

@darranl darranl closed this Sep 29, 2023
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.

4 participants