[issue-1272] GitHub action setup #1304

gaol · 2024-04-23T09:22:12Z

Based on Jim's initial github action setup from Initial change to start the github action build Initial change to start the github action build #1274
Check changes to main branch only and ignore changes in non code paths
Check unit tests only for now. ./mvnw clean package

gaol · 2024-04-24T03:59:56Z

@jimma @fabiobrz may I ask you to review ? thanks ! This will enable github action just like what .circleci does now for the unit tests. The integration tests will be fixed by following up issues, and once they are done, the github action will be updated to cover integration tests as well.

.github/workflows/maven.yml

rhusar · 2024-04-24T07:48:12Z

.github/workflows/maven.yml

+    strategy:
+      fail-fast: false
+      matrix:
+        os: [ubuntu-latest]


I am not sure we need to create a platform matrix as this will require linux containers, so this won't ever work on windows and probably neither on macos.

i.e. I would just do runs-on: ubuntu-latest and move on

I guess matrix aims at supporting more than just one JDK, as it was in the initial PR: https://github.com/arquillian/arquillian-cube/pull/1274/files#diff-5dbf1a803ecc13ff945a08ed3eb09149a83615e83f15320550af8e3a91976446R19

But just my take.
A question, though: wouldn't be better to have them working on linux and file a separate tracker for expanding? Short living (hopefully) and easy to review PRs would be the reason.
WDYT?

JDK matrix for sure - but os matrix not really useful.

fabiobrz

Hi @gaol - and thanks for this PR.
I dropped some minor comments, and I'd like to know whether we do have some run reference for the action we are implementing: maybe here? https://github.com/gaol/arquillian-cube/actions

WDYT?

fabiobrz · 2024-04-24T08:43:41Z

.github/workflows/maven.yml

+          driver: docker
+          container runtime: containerd
+          minikube version: 'v1.30.1'
+          kubernetes version: 'v1.27.1'


Just asking whether we'd want to use the latest K8s minor, or at least micro? See https://kubernetes.io/releases/

Or is there a reason for referencing 1.27.1?

Sure, we can use the latest version I guess. It was there from original PR. will update it to 1.30.0 which is the latest one I can see. also I will update the actions-setup-minikube to latest one to support it.

fabiobrz · 2024-04-24T08:48:22Z

.github/workflows/maven.yml

+    strategy:
+      fail-fast: false
+      matrix:
+        os: [ubuntu-latest]


I guess matrix aims at supporting more than just one JDK, as it was in the initial PR: https://github.com/arquillian/arquillian-cube/pull/1274/files#diff-5dbf1a803ecc13ff945a08ed3eb09149a83615e83f15320550af8e3a91976446R19

But just my take.
A question, though: wouldn't be better to have them working on linux and file a separate tracker for expanding? Short living (hopefully) and easy to review PRs would be the reason.
WDYT?

gaol · 2024-04-24T08:59:55Z

I'd like to know whether we do have some run reference for the action we are implementing: maybe here? https://github.com/gaol/arquillian-cube/actions

WDYT?

Yes, there is: https://github.com/gaol/arquillian-cube/tree/github_action_setup_own_test, which is the place I test the github action execution.

wouldn't be better to have them working on linux and file a separate tracker for expanding? Short living (hopefully) and easy to review PRs would be the reason.

Sure, now the plan is to stay with runs-on: ubuntu-latest, and having the jdk matrix to 11 only, we can expand to more jdk versions later.

fabiobrz · 2024-04-24T09:32:33Z

Yes, there is: https://github.com/gaol/arquillian-cube/tree/github_action_setup_own_test, which is the place I test the github action execution.

Thanks @gaol - Ok, so we'd have to wait for a green bar there, right? IIUC the last commit is failing the CI checks: https://github.com/gaol/arquillian-cube/actions/runs/8814197921/job/24193526839

gaol · 2024-04-24T09:38:35Z

yes, @fabiobrz , I think there is a temporary failure (failed with apt-get update -y), will re-run it later and comment here once I get green result.

gaol · 2024-04-24T14:32:27Z

Updated according to the feed backs:

only use runner: ubuntu-latest
jdk matrix is 11 for now
minikube version: v1.32.0 for manusa/actions-setup-minikube@v2.10.0
kubernetes version: v1.30.0 for manusa/actions-setup-minikube@v2.10.0

The reference github action execution is: https://github.com/gaol/arquillian-cube/actions/runs/8818141575/job/24206240475

@fabiobrz @rhusar

fabiobrz

Unit tests in the K8s category are executed and passing along with the Docker ones. I am approving, let's expand to ITs and OpenShift from here. Thanks @gaol !

I'll merge this once @rhusar will approve too.

rhusar

Few more things.

rhusar · 2024-04-24T16:30:33Z

.github/workflows/maven.yml

@@ -0,0 +1,74 @@
+name: Arquillian-Cube Maven Build


Please drop the dash, the project name is "Arquillian Cube" (no dash).

Nevertheless I always struggle how to name these. The project name seems redundant. The maven build is also redundant, since that's the only build system we use here. Also, build is a bit of a misnomer as primarily this is used to testing...

rhusar · 2024-04-24T16:30:50Z

.github/workflows/maven.yml

+      - uses: actions/upload-artifact@v3
+        if: failure()
+        with:
+          name: server-logs-${{ matrix.os }}-${{ matrix.java }}


The ${{ matrix.os }} would fail to resolve now.

rhusar · 2024-04-24T16:30:54Z

.github/workflows/maven.yml

+      - uses: actions/upload-artifact@v3
+        if: failure()
+        with:
+          name: surefire-reports-${{ matrix.os }}-${{ matrix.java }}


The ${{ matrix.os }} would fail to resolve now.

rhusar · 2024-04-24T16:31:23Z

.github/workflows/maven.yml

+      - uses: actions/upload-artifact@v3
+        if: failure()
+        with:
+          name: server-logs-${{ matrix.os }}-${{ matrix.java }}


Also any reason why this action is duplicated? I would just collect this in one zip with multiple paths.

They are different artifacts, one is all from target/urefire-reports/, the other is from all *.log files, the 2 artifacts separate the information. but I think it is fine as well to zip all into one artifact.

* Monitor changes on main branch * Ignore changes in non-code paths * Specify env FAILSAFE_GROUPS to run group of tests * Pre fetch dependnecies in Maven build * Check unit tests only for now

gaol · 2024-04-25T04:38:41Z

@rhusar thanks, fixed as the following:

Rename the workflow name to Arquillian Cube Test, please feel free to propose new names if you like.
Combine all log files and surefire-reports files into one artifact to upload on failure.
Drop the use of matrix.os

The last execution is: https://github.com/gaol/arquillian-cube/actions/runs/8826949457/job/24233479032

rhusar

Looks good to me, thanks!

gaol mentioned this pull request Apr 23, 2024

[Issue:1272] Add github action workflow yml to build #1276

Closed

gaol marked this pull request as draft April 23, 2024 10:59

fabiobrz force-pushed the main branch from ed5caeb to 9b16aa8 Compare April 23, 2024 16:52

gaol force-pushed the github_action_setup branch from d1a8759 to 1a9f7b4 Compare April 24, 2024 03:51

[arquillian#1272]:Add github action workflow

ee7e459

gaol force-pushed the github_action_setup branch from 1a9f7b4 to ee42828 Compare April 24, 2024 03:56

gaol marked this pull request as ready for review April 24, 2024 03:57

jimma reviewed Apr 24, 2024

View reviewed changes

.github/workflows/maven.yml Outdated Show resolved Hide resolved

gaol changed the title ~~GitHub action setup~~ [issue-1272] GitHub action setup Apr 24, 2024

rhusar reviewed Apr 24, 2024

View reviewed changes

fabiobrz reviewed Apr 24, 2024

View reviewed changes

gaol force-pushed the github_action_setup branch from 8239bb1 to 0c6b317 Compare April 24, 2024 14:18

fabiobrz previously approved these changes Apr 24, 2024

View reviewed changes

rhusar suggested changes Apr 24, 2024

View reviewed changes

gaol added 2 commits April 25, 2024 12:19

[Issue:1272] Setup github action

8f08d70

* Monitor changes on main branch * Ignore changes in non-code paths * Specify env FAILSAFE_GROUPS to run group of tests * Pre fetch dependnecies in Maven build * Check unit tests only for now

Remove travis as we don't use it anymore

eca206c

gaol dismissed fabiobrz’s stale review via eca206c April 25, 2024 04:35

gaol force-pushed the github_action_setup branch from 0c6b317 to eca206c Compare April 25, 2024 04:35

rhusar approved these changes Apr 25, 2024

View reviewed changes

fabiobrz approved these changes Apr 25, 2024

View reviewed changes

fabiobrz merged commit 2dac962 into arquillian:main Apr 25, 2024
1 check passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[issue-1272] GitHub action setup #1304

[issue-1272] GitHub action setup #1304

gaol commented Apr 23, 2024

gaol commented Apr 24, 2024

rhusar Apr 24, 2024

rhusar Apr 24, 2024

gaol Apr 24, 2024

fabiobrz Apr 24, 2024

rhusar Apr 24, 2024

fabiobrz left a comment

fabiobrz Apr 24, 2024

gaol Apr 24, 2024

fabiobrz Apr 24, 2024

gaol commented Apr 24, 2024

fabiobrz commented Apr 24, 2024 •

edited

Loading

gaol commented Apr 24, 2024

gaol commented Apr 24, 2024

fabiobrz left a comment •

edited

Loading

rhusar left a comment

rhusar Apr 24, 2024 •

edited

Loading

rhusar Apr 24, 2024

gaol Apr 25, 2024

rhusar Apr 24, 2024

rhusar Apr 24, 2024

gaol Apr 25, 2024

gaol commented Apr 25, 2024

rhusar left a comment

[issue-1272] GitHub action setup #1304

[issue-1272] GitHub action setup #1304

Conversation

gaol commented Apr 23, 2024

gaol commented Apr 24, 2024

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

fabiobrz 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

gaol commented Apr 24, 2024

fabiobrz commented Apr 24, 2024 • edited Loading

gaol commented Apr 24, 2024

gaol commented Apr 24, 2024

fabiobrz left a comment • edited Loading

Choose a reason for hiding this comment

rhusar left a comment

Choose a reason for hiding this comment

rhusar Apr 24, 2024 • edited 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

gaol commented Apr 25, 2024

rhusar left a comment

Choose a reason for hiding this comment

fabiobrz commented Apr 24, 2024 •

edited

Loading

fabiobrz left a comment •

edited

Loading

rhusar Apr 24, 2024 •

edited

Loading