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

Add all positive cases for GlassFish runtime #304

Closed
wants to merge 7 commits into from

Conversation

adan-jawad
Copy link
Contributor

No description provided.

@eclipse-starter-bot
Copy link

Can one of the admins verify this patch?

Copy link
Contributor

@m-reza-rahman m-reza-rahman left a comment

Choose a reason for hiding this comment

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

This is organized incorrectly. At the top level, there should be two jobs: one for the Archetype and one for the UI. For the Archetype, things should first be organized by Java SE version, corresponding to invocations with that version. For each Java SE version, tests should then be organized by runtime, Jakarta EE version, profile then Docker flag.

@m-reza-rahman m-reza-rahman marked this pull request as draft June 24, 2024 18:11
@m-reza-rahman m-reza-rahman changed the title Add all positive cases for Glassfish runtime Add all positive cases for GlassFish runtime Jun 26, 2024
@adan-jawad adan-jawad marked this pull request as ready for review June 27, 2024 13:24
@adan-jawad
Copy link
Contributor Author

Hello, I have added descriptions for docker support and organized the cases in the way specified. I have restored the old cases from the original file and have added my cases between the start and end comments for every java version for easy readability. I'll remove these comments once you review this.

Copy link
Contributor

@m-reza-rahman m-reza-rahman left a comment

Choose a reason for hiding this comment

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

Please order by SE version (starting with the lowest), runtime/none (alphabetic, starting with none), EE version (starting with the lowest), profile (full, web, core), and Docker (no, yes).

So like this:

  • Run Archetype for EE 8 Web Profile, SE 8, Payara
  • Run Archetype for EE 8 Web Profile, SE 8, Payara, with Docker

Where Docker is really not supported (a warning is generated by the Archetype), we should not treat that as a "positive case". That's a negative case we need to test for later. GlassFish currently does not support Docker.

Please order your part and the file overall - including existing cases. Confusing to read and evaluate otherwise for me.

@m-reza-rahman m-reza-rahman marked this pull request as draft June 27, 2024 23:46
@m-reza-rahman
Copy link
Contributor

Please resolve conflicts with latest commits to master.

@adan-jawad
Copy link
Contributor Author

I am a bit confused. Do I delete the cases where docker is not supported, or do I just remove that from the description?

@m-reza-rahman
Copy link
Contributor

For now, kindly remove the cases where Docker is not supported and set them aside. These need to be added later as explicitly tested negative cases.

@adan-jawad adan-jawad marked this pull request as ready for review July 2, 2024 15:00
@m-reza-rahman
Copy link
Contributor

Please resolve all merge conflicts.

@m-reza-rahman m-reza-rahman marked this pull request as draft July 2, 2024 15:18
@adan-jawad adan-jawad marked this pull request as ready for review July 2, 2024 15:31
@m-reza-rahman
Copy link
Contributor

I will review if I have some time during the work week. Otherwise, may need to wait until the weekend.

@adan-jawad
Copy link
Contributor Author

I will review if I have some time during the work week. Otherwise, may need to wait until the weekend.

Sure.

@@ -34,6 +34,9 @@ jobs:
- name: Run Archetype for EE 8 Web Profile, SE 8
run: |
mvn archetype:generate -DinteractiveMode=false -DaskForDefaultPropertyValues=false -DarchetypeGroupId=org.eclipse.starter -DarchetypeArtifactId=jakarta-starter -DarchetypeVersion=2.3.0-SNAPSHOT -DjakartaVersion=8 -Dprofile=web -DjavaVersion=8 -DoutputDirectory=app -Dgoals="clean package"

rm -rf app
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is this needed?

- name: Run Archetype for EE 8, SE 8, GlassFish
run: |
mvn archetype:generate -DinteractiveMode=false -DaskForDefaultPropertyValues=false -DarchetypeGroupId=org.eclipse.starter -DarchetypeArtifactId=jakarta-starter -DarchetypeVersion=2.3.0-SNAPSHOT -DjakartaVersion=8 -Dprofile=full -DjavaVersion=8 -Druntime=glassfish -DoutputDirectory=app/glassfish -Dgoals="clean package"
rm -rf app/glassfish


- name: Run Archetype for EE 8 Web Profile, SE 8, GlassFish
Copy link
Contributor

Choose a reason for hiding this comment

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

This is a duplicate. Why is it needed?

@@ -133,6 +144,10 @@ jobs:

- name: Run Archetype for EE 8, SE 8, WildFly
run: |

Copy link
Contributor

Choose a reason for hiding this comment

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

Why was this added?

- name: Run Archetype for EE 10 Web Profile, SE 17, WildFly
run: |
mvn archetype:generate -DinteractiveMode=false -DaskForDefaultPropertyValues=false -DarchetypeGroupId=org.eclipse.starter -DarchetypeArtifactId=jakarta-starter -DarchetypeVersion=2.3.0-SNAPSHOT -DjakartaVersion=10 -Dprofile=web -DjavaVersion=17 -Druntime=wildfly -Ddocker=yes -DoutputDirectory=app/wildfly -Dgoals="clean package"
rm -rf app/wildfly

- name: Run Archetype for EE 8, SE 17, WildFly
run: |

Copy link
Contributor

Choose a reason for hiding this comment

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

Why was this added?

- name: Set up Java SE 11
uses: actions/setup-java@v3
with:
distribution: "temurin"
java-version: 11


Copy link
Contributor

Choose a reason for hiding this comment

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

Where are the SE 8 cases for GlassFish?

Copy link
Contributor

@m-reza-rahman m-reza-rahman left a comment

Choose a reason for hiding this comment

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

My guess is that this PR has some improper merge conflict resolution issues. Please re-inspect and fix.

@m-reza-rahman m-reza-rahman marked this pull request as draft July 2, 2024 21:22
@adan-jawad adan-jawad marked this pull request as ready for review July 6, 2024 14:21
@@ -9,23 +9,23 @@ jobs:
build-archetype:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v3
- uses: actions/checkout@v4
Copy link
Contributor

Choose a reason for hiding this comment

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

Please pull the latest from master. This diff should disappear.

with:
distribution: "temurin"
java-version: 8

- name: Cache Maven packages
uses: actions/cache@v3
uses: actions/cache@v4
Copy link
Contributor

Choose a reason for hiding this comment

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

Please pull the latest from master. This diff should disappear.

rm -rf app/wildfly

- name: Set up Java SE 11
uses: actions/setup-java@v3
uses: actions/setup-java@v4
Copy link
Contributor

Choose a reason for hiding this comment

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

Please pull the latest from master. This diff should disappear.

rm -rf app/wildfly

- name: Set up Java SE 17
uses: actions/setup-java@v3
uses: actions/setup-java@v4
Copy link
Contributor

Choose a reason for hiding this comment

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

Please pull the latest from master. This diff should disappear.

build-ui:
runs-on: ubuntu-20.04
steps:
- uses: actions/checkout@v3
- uses: actions/checkout@v4
Copy link
Contributor

Choose a reason for hiding this comment

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

Please pull the latest from master. This diff should disappear.

- name: Setup Java
uses: actions/setup-java@v3
uses: actions/setup-java@v4
Copy link
Contributor

Choose a reason for hiding this comment

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

Please pull the latest from master. This diff should disappear.

with:
distribution: "temurin"
java-version: 11

- name: Cache Maven packages
uses: actions/cache@v3
uses: actions/cache@v4
Copy link
Contributor

Choose a reason for hiding this comment

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

Please pull the latest from master. This diff should disappear.

with:
path: ~/.m2
key: ${{ runner.os }}-m2-${{ hashFiles('**/pom.xml') }}
restore-keys: ${{ runner.os }}-m2

- name: Build Archetype
run: mvn clean install --file archetype/pom.xml

Copy link
Contributor

Choose a reason for hiding this comment

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

Why is this white space change needed?

@@ -125,7 +125,7 @@ jobs:
run: |
mvn archetype:generate -DinteractiveMode=false -DaskForDefaultPropertyValues=false -DarchetypeGroupId=org.eclipse.starter -DarchetypeArtifactId=jakarta-starter -DarchetypeVersion=2.3.0-SNAPSHOT -DjakartaVersion=9.1 -Dprofile=web -DjavaVersion=8 -Druntime="open-liberty" -Ddocker=yes -DoutputDirectory="app/open-liberty" -Dgoals="clean package"
rm -rf app/open-liberty

Copy link
Contributor

Choose a reason for hiding this comment

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

Why is this white space change needed?

@@ -291,7 +335,7 @@ jobs:
run: |
mvn archetype:generate -DinteractiveMode=false -DaskForDefaultPropertyValues=false -DarchetypeGroupId=org.eclipse.starter -DarchetypeArtifactId=jakarta-starter -DarchetypeVersion=2.3.0-SNAPSHOT -DjakartaVersion=10 -Dprofile=core -DjavaVersion=11 -Druntime="open-liberty" -Ddocker=yes -DoutputDirectory="app/open-liberty" -Dgoals="clean package"
rm -rf app/open-liberty

Copy link
Contributor

Choose a reason for hiding this comment

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

Why is this white space change needed?

@@ -497,7 +586,7 @@ jobs:
run: |
mvn archetype:generate -DinteractiveMode=false -DaskForDefaultPropertyValues=false -DarchetypeGroupId=org.eclipse.starter -DarchetypeArtifactId=jakarta-starter -DarchetypeVersion=2.3.0-SNAPSHOT -DjakartaVersion=10 -Dprofile=core -DjavaVersion=17 -Druntime="open-liberty" -Ddocker=yes -DoutputDirectory="app/open-liberty" -Dgoals="clean package"
rm -rf app/open-liberty

Copy link
Contributor

Choose a reason for hiding this comment

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

Why is this white space change needed?

run: mvn clean package --file ui/pom.xml
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is this white space change needed?

@@ -139,6 +139,8 @@ jobs:
- name: Run Archetype for EE 8, SE 8, WildFly, with Docker
run: |
mvn archetype:generate -DinteractiveMode=false -DaskForDefaultPropertyValues=false -DarchetypeGroupId=org.eclipse.starter -DarchetypeArtifactId=jakarta-starter -DarchetypeVersion=2.3.0-SNAPSHOT -DjakartaVersion=8 -Dprofile=full -DjavaVersion=8 -Druntime=wildfly -Ddocker=yes -DoutputDirectory=app/wildfly -Dgoals="clean package"
docker build -t test-image app/wildfly/jakartaee-hello-world
Copy link
Contributor

Choose a reason for hiding this comment

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

Please remove this. This is not related to GlassFish.

- name: Set up Java SE 8
uses: actions/setup-java@v3
uses: actions/setup-java@v4
Copy link
Contributor

Choose a reason for hiding this comment

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

Please pull the latest from master. This diff should disappear.

@@ -149,10 +151,12 @@ jobs:
- name: Run Archetype for EE 8 Web Profile, SE 8, WildFly, with Docker
run: |
mvn archetype:generate -DinteractiveMode=false -DaskForDefaultPropertyValues=false -DarchetypeGroupId=org.eclipse.starter -DarchetypeArtifactId=jakarta-starter -DarchetypeVersion=2.3.0-SNAPSHOT -DjakartaVersion=8 -Dprofile=web -DjavaVersion=8 -Druntime=wildfly -Ddocker=yes -DoutputDirectory=app/wildfly -Dgoals="clean package"
docker build -t test-image app/wildfly/jakartaee-hello-world
Copy link
Contributor

Choose a reason for hiding this comment

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

Please remove this. This is not related to GlassFish.

@@ -310,6 +354,8 @@ jobs:
- name: Run Archetype for EE 8, SE 11, WildFly, with Docker
run: |
mvn archetype:generate -DinteractiveMode=false -DaskForDefaultPropertyValues=false -DarchetypeGroupId=org.eclipse.starter -DarchetypeArtifactId=jakarta-starter -DarchetypeVersion=2.3.0-SNAPSHOT -DjakartaVersion=8 -Dprofile=full -DjavaVersion=11 -Druntime=wildfly -Ddocker=yes -DoutputDirectory=app/wildfly -Dgoals="clean package"
docker build -t test-image app/wildfly/jakartaee-hello-world
Copy link
Contributor

Choose a reason for hiding this comment

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

Please remove this. This is not related to GlassFish.

@@ -320,6 +366,8 @@ jobs:
- name: Run Archetype for EE 8 Web Profile, SE 11, WildFly, with Docker
run: |
mvn archetype:generate -DinteractiveMode=false -DaskForDefaultPropertyValues=false -DarchetypeGroupId=org.eclipse.starter -DarchetypeArtifactId=jakarta-starter -DarchetypeVersion=2.3.0-SNAPSHOT -DjakartaVersion=8 -Dprofile=web -DjavaVersion=11 -Druntime=wildfly -Ddocker=yes -DoutputDirectory=app/wildfly -Dgoals="clean package"
docker build -t test-image app/wildfly/jakartaee-hello-world
Copy link
Contributor

Choose a reason for hiding this comment

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

Please remove this. This is not related to GlassFish.

@@ -330,6 +378,8 @@ jobs:
- name: Run Archetype for EE 10, SE 11, WildFly, with Docker
run: |
mvn archetype:generate -DinteractiveMode=false -DaskForDefaultPropertyValues=false -DarchetypeGroupId=org.eclipse.starter -DarchetypeArtifactId=jakarta-starter -DarchetypeVersion=2.3.0-SNAPSHOT -DjakartaVersion=10 -Dprofile=full -DjavaVersion=11 -Druntime=wildfly -Ddocker=yes -DoutputDirectory=app/wildfly -Dgoals="clean package"
docker build -t test-image app/wildfly/jakartaee-hello-world
Copy link
Contributor

Choose a reason for hiding this comment

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

Please remove this. This is not related to GlassFish.

@@ -340,6 +390,8 @@ jobs:
- name: Run Archetype for EE 10 Web Profile, SE 11, WildFly, with Docker
run: |
mvn archetype:generate -DinteractiveMode=false -DaskForDefaultPropertyValues=false -DarchetypeGroupId=org.eclipse.starter -DarchetypeArtifactId=jakarta-starter -DarchetypeVersion=2.3.0-SNAPSHOT -DjakartaVersion=10 -Dprofile=web -DjavaVersion=11 -Druntime=wildfly -Ddocker=yes -DoutputDirectory=app/wildfly -Dgoals="clean package"
docker build -t test-image app/wildfly/jakartaee-hello-world
Copy link
Contributor

Choose a reason for hiding this comment

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

Please remove this. This is not related to GlassFish.

@@ -350,10 +402,12 @@ jobs:
- name: Run Archetype for EE 10 Core Profile, SE 11, WildFly, with Docker
run: |
mvn archetype:generate -DinteractiveMode=false -DaskForDefaultPropertyValues=false -DarchetypeGroupId=org.eclipse.starter -DarchetypeArtifactId=jakarta-starter -DarchetypeVersion=2.3.0-SNAPSHOT -DjakartaVersion=10 -Dprofile=core -DjavaVersion=11 -Druntime=wildfly -Ddocker=yes -DoutputDirectory=app/wildfly -Dgoals="clean package"
docker build -t test-image app/wildfly/jakartaee-hello-world
Copy link
Contributor

Choose a reason for hiding this comment

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

Please remove this. This is not related to GlassFish.

@@ -516,6 +605,8 @@ jobs:
- name: Run Archetype for EE 8, SE 17, WildFly, with Docker
run: |
mvn archetype:generate -DinteractiveMode=false -DaskForDefaultPropertyValues=false -DarchetypeGroupId=org.eclipse.starter -DarchetypeArtifactId=jakarta-starter -DarchetypeVersion=2.3.0-SNAPSHOT -DjakartaVersion=8 -Dprofile=full -DjavaVersion=17 -Druntime=wildfly -Ddocker=yes -DoutputDirectory=app/wildfly -Dgoals="clean package"
docker build -t test-image app/wildfly/jakartaee-hello-world
Copy link
Contributor

Choose a reason for hiding this comment

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

Please remove this. This is not related to GlassFish.

@@ -526,6 +617,8 @@ jobs:
- name: Run Archetype for EE 8 Web Profile, SE 17, WildFly, with Docker
run: |
mvn archetype:generate -DinteractiveMode=false -DaskForDefaultPropertyValues=false -DarchetypeGroupId=org.eclipse.starter -DarchetypeArtifactId=jakarta-starter -DarchetypeVersion=2.3.0-SNAPSHOT -DjakartaVersion=8 -Dprofile=web -DjavaVersion=17 -Druntime=wildfly -Ddocker=yes -DoutputDirectory=app/wildfly -Dgoals="clean package"
docker build -t test-image app/wildfly/jakartaee-hello-world
Copy link
Contributor

Choose a reason for hiding this comment

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

Please remove this. This is not related to GlassFish.

@@ -536,6 +629,8 @@ jobs:
- name: Run Archetype for EE 10, SE 17, WildFly, with Docker
run: |
mvn archetype:generate -DinteractiveMode=false -DaskForDefaultPropertyValues=false -DarchetypeGroupId=org.eclipse.starter -DarchetypeArtifactId=jakarta-starter -DarchetypeVersion=2.3.0-SNAPSHOT -DjakartaVersion=10 -Dprofile=full -DjavaVersion=17 -Druntime=wildfly -Ddocker=yes -DoutputDirectory=app/wildfly -Dgoals="clean package"
docker build -t test-image app/wildfly/jakartaee-hello-world
Copy link
Contributor

Choose a reason for hiding this comment

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

Please remove this. This is not related to GlassFish.

@@ -546,6 +641,8 @@ jobs:
- name: Run Archetype for EE 10 Web Profile, SE 17, WildFly, with Docker
run: |
mvn archetype:generate -DinteractiveMode=false -DaskForDefaultPropertyValues=false -DarchetypeGroupId=org.eclipse.starter -DarchetypeArtifactId=jakarta-starter -DarchetypeVersion=2.3.0-SNAPSHOT -DjakartaVersion=10 -Dprofile=web -DjavaVersion=17 -Druntime=wildfly -Ddocker=yes -DoutputDirectory=app/wildfly -Dgoals="clean package"
docker build -t test-image app/wildfly/jakartaee-hello-world
Copy link
Contributor

Choose a reason for hiding this comment

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

Please remove this. This is not related to GlassFish.

@@ -556,25 +653,26 @@ jobs:
- name: Run Archetype for EE 10 Core Profile, SE 17, WildFly, with Docker
run: |
mvn archetype:generate -DinteractiveMode=false -DaskForDefaultPropertyValues=false -DarchetypeGroupId=org.eclipse.starter -DarchetypeArtifactId=jakarta-starter -DarchetypeVersion=2.3.0-SNAPSHOT -DjakartaVersion=10 -Dprofile=core -DjavaVersion=17 -Druntime=wildfly -Ddocker=yes -DoutputDirectory=app/wildfly -Dgoals="clean package"
docker build -t test-image app/wildfly/jakartaee-hello-world
Copy link
Contributor

Choose a reason for hiding this comment

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

Please remove this. This is not related to GlassFish.

Copy link
Contributor

@m-reza-rahman m-reza-rahman left a comment

Choose a reason for hiding this comment

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

Please address specific in-place feedback. Then I can review hopefully one last time before merge.

@m-reza-rahman m-reza-rahman marked this pull request as draft July 7, 2024 21:57
@adan-jawad
Copy link
Contributor Author

Hello Reza, I am going to create a new pull request and I'm closing this one

@adan-jawad adan-jawad closed this Jul 13, 2024
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.

None yet

3 participants