Skip to content

ci: add Java 17 Maven verification - #2

Merged
wisdommen merged 1 commit into
masterfrom
feature/java17-ci-correction
May 26, 2026
Merged

ci: add Java 17 Maven verification#2
wisdommen merged 1 commit into
masterfrom
feature/java17-ci-correction

Conversation

@wisdommen

@wisdommen wisdommen commented May 26, 2026

Copy link
Copy Markdown
Contributor

Summary

  • add module-local Java 17 Maven CI for UltiLogin
  • align AccountData with UltiTools BaseDataEntity data operator contract
  • update tests/helpers to use string account ids

Verification

  • clean git archive overlay with JAVA_HOME=/usr/lib/jvm/java-17-openjdk-amd64
  • workflow YAML/static forbidden-coupling scan ok
  • mvn -B test: BUILD SUCCESS, 458 tests
  • mvn -B package: BUILD SUCCESS, 458 tests

Scope

  • no parent POM migration
  • no Maven reactor/child-module/submodule assumptions
  • PROJECT.md intentionally not staged

Summary by CodeRabbit

  • Chores

    • Added continuous integration pipeline for automated testing and builds on code push and pull requests.
  • Tests

    • Updated test suite to support internal data model improvements.

Review Change Stack

Align AccountData with the UltiTools BaseDataEntity<String> data operator contract so Java 17 verification passes.
@coderabbitai

coderabbitai Bot commented May 26, 2026

Copy link
Copy Markdown
📝 Walkthrough

Walkthrough

This PR adds Maven CI/CD infrastructure and refactors the AccountData entity to use string-based identifiers instead of integers. The CI workflow automates test and package builds on all pushes and pull requests. The entity type change cascades to test helper methods and validation assertions, ensuring the mocked and tested data aligns with the new string ID contract.

Changes

Maven CI Workflow

Layer / File(s) Summary
Maven CI workflow configuration
.github/workflows/maven-ci.yml
New GitHub Actions workflow named "Maven CI" that triggers on all pushes and pull requests, sets up JDK 17 with Maven caching, runs mvn test, and then mvn package on Ubuntu.

AccountData String ID Migration

Layer / File(s) Summary
Entity generic type update
src/main/java/com/ultikits/plugins/login/entity/AccountData.java
AccountData extends BaseDataEntity<String> instead of BaseDataEntity<Integer>, changing the entity's ID type contract.
Test helper and assertions
src/test/java/com/ultikits/plugins/login/UltiLoginTestHelper.java, src/test/java/com/ultikits/plugins/login/entity/AccountDataTest.java
Test helper createSampleAccount now stringifies the UUID hash code, and AccountDataTest assertions validate string ID values to match the updated entity.

🎯 2 (Simple) | ⏱️ ~10 minutes

🐰 A workflow springs up to test our code,
While IDs change from nums to strings abode,
With helpers updated and tests all in line,
The path to quality now runs just fine! ✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'ci: add Java 17 Maven verification' accurately describes the main change: adding a Maven CI workflow. However, it overlooks the significant type system change (AccountData → AccountData) affecting core entity semantics.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feature/java17-ci-correction

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🧹 Nitpick comments (1)
.github/workflows/maven-ci.yml (1)

27-28: ⚡ Quick win

Redundant test execution increases CI time.

The workflow runs mvn -B test separately, but the subsequent mvn -B package step already includes the test phase by default. This means tests run twice, doubling the test execution time.

If separate test reporting is not required, consider removing this step.

♻️ Remove redundant test step

If you only need the package artifact and test results once:

-      - name: Run tests
-        run: mvn -B test
-
       - name: Build package
         run: mvn -B package

Alternatively, if you want to skip tests during packaging (after running them separately):

       - name: Run tests
         run: mvn -B test

-      - name: Build package
-        run: mvn -B package
+      - name: Build package (skip tests)
+        run: mvn -B package -DskipTests
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.github/workflows/maven-ci.yml around lines 27 - 28, The "Run tests" step
that executes "mvn -B test" is redundant because the later "mvn -B package"
already runs the test phase; remove the entire "Run tests" step (the job/step
named "Run tests" that runs "mvn -B test") to avoid double execution, or if you
intentionally want a separate test run, change the packaging step ("mvn -B
package") to skip tests by adding -DskipTests=true so tests only run once.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In @.github/workflows/maven-ci.yml:
- Around line 20-25: Update the CI workflow to explicitly document that the
build uses JDK 17 but targets Java 8 bytecode (matching pom.xml's
<java.version>1.8</java.version> and maven-compiler-plugin source/target) by
adding a short comment or job step description near the Setup JDK step, and
confirm tests are intended to run under JDK 17; additionally replace the
unpinned actions referenced (actions/checkout@v4 and actions/setup-java@v4) with
their corresponding full commit SHAs to satisfy the unpinned-action policy so
the workflow pins exact commits.

---

Nitpick comments:
In @.github/workflows/maven-ci.yml:
- Around line 27-28: The "Run tests" step that executes "mvn -B test" is
redundant because the later "mvn -B package" already runs the test phase; remove
the entire "Run tests" step (the job/step named "Run tests" that runs "mvn -B
test") to avoid double execution, or if you intentionally want a separate test
run, change the packaging step ("mvn -B package") to skip tests by adding
-DskipTests=true so tests only run once.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 779607a6-a9b3-4fa7-90d5-0c7e79d8b47d

📥 Commits

Reviewing files that changed from the base of the PR and between cbc3491 and f29e439.

📒 Files selected for processing (4)
  • .github/workflows/maven-ci.yml
  • src/main/java/com/ultikits/plugins/login/entity/AccountData.java
  • src/test/java/com/ultikits/plugins/login/UltiLoginTestHelper.java
  • src/test/java/com/ultikits/plugins/login/entity/AccountDataTest.java

Comment on lines +20 to +25
- name: Set up JDK 17
uses: actions/setup-java@v4
with:
distribution: temurin
java-version: '17'
cache: maven

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Verify the Java version configuration in pom.xml and confirm JDK 17 compatibility

echo "=== Java version property in pom.xml ==="
grep -A 2 "<java.version>" pom.xml

echo -e "\n=== Maven compiler plugin configuration ==="
grep -A 10 "maven-compiler-plugin" pom.xml | grep -E "(source|target|release)"

echo -e "\n=== Verification ==="
echo "CI uses JDK 17 but pom.xml specifies java.version=1.8"
echo "This will compile to Java 8 bytecode using JDK 17 toolchain"
echo "Confirm this is the intended configuration"

Repository: UltiKits/UltiLogin

Length of output: 602


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "=== CI workflow: .github/workflows/maven-ci.yml uses/actions ==="
sed -n '1,120p' .github/workflows/maven-ci.yml | rg -n "uses:"

echo -e "\n=== CI workflow: JDK step context (around line 20-25) ==="
nl -ba .github/workflows/maven-ci.yml | sed -n '15,35p'

echo -e "\n=== pom.xml: java.version + compiler plugin block ==="
nl -ba pom.xml | rg -n "<java\.version>|maven-compiler-plugin|<source>|\<target\>|<release>|<toolchain>|maven\.compiler\." -n | head -n 80 || true

echo -e "\n=== pom.xml: show compiler plugin section (best-effort) ==="
# Try to print a bounded region around maven-compiler-plugin if present
line=$(rg -n "maven-compiler-plugin" pom.xml | head -n1 | cut -d: -f1 || true)
if [[ -n "${line}" ]]; then
  start=$((line-20))
  end=$((line+80))
  nl -ba pom.xml | sed -n "${start},${end}p"
else
  echo "maven-compiler-plugin not found in pom.xml"
fi

Repository: UltiKits/UltiLogin

Length of output: 323


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "=== CI workflow: .github/workflows/maven-ci.yml uses/actions ==="
sed -n '1,120p' .github/workflows/maven-ci.yml | rg -n "uses:"

echo -e "\n=== CI workflow: show lines ~15-40 ==="
cat -n .github/workflows/maven-ci.yml | sed -n '15,45p'

echo -e "\n=== pom.xml: relevant Java/version/compiler settings (best-effort) ==="
cat -n pom.xml | rg -n "<java\.version>|maven-compiler-plugin|<source>|\<target\>|<release>|<toolchain>|maven\.compiler\."

Repository: UltiKits/UltiLogin

Length of output: 1002


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "=== pom.xml: java.version property ==="
rg -n "<java\.version>" pom.xml
echo
rg -n "maven-compiler-plugin" pom.xml
echo
echo "=== pom.xml: compiler source/target/release/toolchain sections (context) ==="
for pat in "<source>" "<target>" "<release>" "toolchain" "<maven.compiler" "maven.compiler"; do
  echo "--- pattern: $pat ---"
  rg -nF "$pat" pom.xml || true
done

echo
echo "=== pom.xml: show around first maven-compiler-plugin occurrence ==="
line=$(rg -n "maven-compiler-plugin" pom.xml | head -n1 | cut -d: -f1 || true)
if [[ -n "${line}" ]]; then
  start=$((line-25))
  end=$((line+120))
  sed -n "${start},${end}p" pom.xml | nl -ba | head -n 170
else
  echo "maven-compiler-plugin not found in pom.xml"
fi

Repository: UltiKits/UltiLogin

Length of output: 707


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "=== Workflow actions in .github/workflows/maven-ci.yml ==="
sed -n '1,120p' .github/workflows/maven-ci.yml | rg -n "uses:"

echo
echo "=== pom.xml: show maven-compiler-plugin section with line numbers (bounded) ==="
line=$(rg -n "maven-compiler-plugin" pom.xml | head -n1 | cut -d: -f1)
start=$((line-20))
end=$((line+60))
if [[ "$start" -lt 1 ]]; then start=1; fi
sed -n "${start},${end}p" pom.xml | cat -n

Repository: UltiKits/UltiLogin

Length of output: 2871


Document the Java strategy (JDK 17 build targeting Java 8) and pin GitHub Actions

  • .github/workflows/maven-ci.yml uses JDK 17, while pom.xml sets <java.version>1.8</java.version> and maven-compiler-plugin compiles with <source>${java.version}</source> / <target>${java.version}</target>—this should be explicitly documented as “build with JDK 17, target Java 8 bytecode” (and ensure running tests on JDK 17 is intended).
  • Both actions/checkout@v4 and actions/setup-java@v4 are only pinned to major tags; pin them to full commit SHAs to satisfy the “unpinned action” policy/static analysis.
🧰 Tools
🪛 zizmor (1.25.2)

[error] 21-21: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)

(unpinned-uses)

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.github/workflows/maven-ci.yml around lines 20 - 25, Update the CI workflow
to explicitly document that the build uses JDK 17 but targets Java 8 bytecode
(matching pom.xml's <java.version>1.8</java.version> and maven-compiler-plugin
source/target) by adding a short comment or job step description near the Setup
JDK step, and confirm tests are intended to run under JDK 17; additionally
replace the unpinned actions referenced (actions/checkout@v4 and
actions/setup-java@v4) with their corresponding full commit SHAs to satisfy the
unpinned-action policy so the workflow pins exact commits.

@wisdommen
wisdommen merged commit 623271d into master May 26, 2026
3 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.

1 participant