Skip to content

Upgrade to Java 8 and add GitHub Actions CI - #64

Merged
julman99 merged 6 commits into
masterfrom
feature/github-actions-ci
Jan 7, 2026
Merged

Upgrade to Java 8 and add GitHub Actions CI#64
julman99 merged 6 commits into
masterfrom
feature/github-actions-ci

Conversation

@julman99

@julman99 julman99 commented Jan 7, 2026

Copy link
Copy Markdown
Owner

Summary

This PR upgrades the minimum Java version and adds GitHub Actions CI for automated testing.

Changes

Java Version Upgrade

  • Minimum Java version: Upgraded from Java 1.6 to Java 8
  • Build JDK: Using JDK 25 (latest LTS) for compilation
  • maven-compiler-plugin: Updated from 3.1 to 3.14.1 for JDK 25 compatibility

Test Fixes for Modern JDK

  • Made inner test classes static in HooksTest.java to avoid reflection issues
  • Marked Gson field as transient to prevent serialization of internal ThreadLocal fields
  • These changes fix InaccessibleObjectException errors on JDK 16+

GitHub Actions CI

  • Added .github/workflows/ci.yml with a CI pipeline that:
    • Triggers on PRs and pushes to master/main branches
    • Builds and tests with JDK 25
    • Uploads test results as artifacts

Branch Protection Setup

After merging, configure branch protection rules in Settings → Branches:

  1. Add rule for master branch
  2. Enable "Require status checks to pass before merging"
  3. Select the Run Tests job as a required check

This will block PR merging if tests fail.

Summary by CodeRabbit

  • Chores
    • Added a CI workflow to run automated tests on pull requests and merges, with test results archived.
    • Updated build configuration to a newer Java/compiler baseline (requires modern JDK for builds and tests).

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai

coderabbitai Bot commented Jan 7, 2026

Copy link
Copy Markdown
📝 Walkthrough

Walkthrough

Adds a GitHub Actions CI workflow that runs Maven verify on PRs and pushes to master/main using Temurin JDK 25 and uploads surefire reports; updates pom.xml (maven-compiler-plugin bumped and source/target set to Java 8); updates .java-version to 25; makes inner test classes static and marks a field transient in HooksTest.java.

Changes

Cohort / File(s) Summary
GitHub Actions CI Workflow
.github/workflows/ci.yml
New workflow triggered on pull_request and push (master/main); single job on ubuntu-latest that checks out code, sets up Temurin JDK 25 with Maven cache, runs mvn -B verify --file pom.xml, and always uploads target/surefire-reports/ as test-results artifact (7-day retention).
Build / Java version
pom.xml, .java-version
pom.xml: upgraded maven-compiler-plugin to 3.14.1 and changed compiler source/target from 1.6 to 8. .java-version: value changed from 1.7 to 25.
Tests — structural changes
src/test/java/io/gsonfire/gson/HooksTest.java
Converted inner classes A and B to static; marked the gson field in A as transient. No public API/signature changes.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant Dev as Developer (push/PR)
  participant GH as GitHub
  participant Runner as Actions Runner (ubuntu-latest)
  participant Maven as Maven (inside runner)
  participant Store as Artifact Store

  Dev->>GH: push / open PR
  GH->>Runner: trigger ci.yml job
  Runner->>Runner: checkout repo
  Runner->>Runner: setup Temurin JDK 25 + Maven cache
  Runner->>Maven: mvn -B verify --file pom.xml
  alt tests run
    Maven-->>Runner: generate target/surefire-reports/
  end
  Runner->>Store: upload `target/surefire-reports/` as `test-results` (retention=7d)
  note right of Store: upload step runs regardless of job result
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐇 I nibbled at the build-tree root,
JDK twenty-five boots up to boot,
Maven sings and tests report,
Static hops in the test cohort,
A carrot artifact—soft and cute 🥕

🚥 Pre-merge checks | ✅ 1 | ❌ 2
❌ Failed checks (2 warnings)
Check name Status Explanation Resolution
Title check ⚠️ Warning The title 'Upgrade to Java 8 and add GitHub Actions CI' partially captures the changeset but misrepresents the main scope. While CI addition is accurate, Java upgrade actually spans from 1.6→8 (compiler) and 1.7→25 (.java-version), with structural test fixes also included—making the title incomplete and slightly misleading about the primary objective. Revise to reflect the complete scope: 'Add GitHub Actions CI workflow and update Java toolchain' or similar, prioritizing the CI addition as the primary change per PR objectives while acknowledging the broader updates.
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (1 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

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

✨ Finishing touches
  • 📝 Generate docstrings

📜 Recent review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between df1f718 and 666200c.

📒 Files selected for processing (3)
  • .java-version
  • pom.xml
  • src/test/java/io/gsonfire/gson/HooksTest.java
✅ Files skipped from review due to trivial changes (1)
  • .java-version
🚧 Files skipped from review as they are similar to previous changes (1)
  • pom.xml
🔇 Additional comments (3)
src/test/java/io/gsonfire/gson/HooksTest.java (3)

118-118: LGTM! Correct fix for JDK 16+ compatibility.

Making class A static removes the implicit reference to the outer HooksTest instance, which resolves serialization issues on newer JDKs. Since A doesn't access any members of the outer class, this change is appropriate and maintains test correctness.


121-121: LGTM! Appropriate use of transient.

Marking the gson field as transient is correct—Gson instances contain complex internal state and circular references that should not be serialized. The field is properly populated via the @PostDeserialize hook (line 141) rather than through deserialization, so excluding it from serialization is the right approach.


157-157: LGTM! Consistent with class A changes.

Making class B static is consistent with the class A changes and necessary for JDK 16+ compatibility. Since B extends A (now static) and doesn't access any outer class members, this change is appropriate.


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: 0

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

14-16: Consider adding fail-fast: false to complete all matrix jobs.

By default, fail-fast is true, which cancels remaining matrix jobs when one fails. For test matrices across Java versions, it's often valuable to see the full picture of which versions pass or fail.

♻️ Suggested change
     strategy:
+      fail-fast: false
       matrix:
         java-version: [8, 11, 17, 21]
pom.xml (1)

43-44: Consider upgrading maven-compiler-plugin to a more recent version.

Version 3.1 (released April 2013) is outdated. The latest stable version is 3.14.1 (September 2025), offering numerous improvements including better JDK support, enhanced error messages, and bug fixes accumulated over 12+ years of development.

📜 Review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between e5d7f82 and 432f7c6.

📒 Files selected for processing (2)
  • .github/workflows/ci.yml
  • pom.xml
🧰 Additional context used
🪛 GitHub Actions: CI
pom.xml

[error] 1-1: BUILD FAILURE: Maven surefire test suite failed due to the above test errors. See target/surefire-reports for details. Command 'mvn -B verify --file pom.xml' exited with non-zero status.

🔇 Additional comments (3)
.github/workflows/ci.yml (2)

1-8: Well-structured CI workflow with good trigger configuration.

The trigger configuration is appropriate - running on PRs to catch issues before merge and on pushes to track master/main health.


18-38: Steps are well-configured with proper action versions and caching.

Good practices observed:

  • Using latest action versions (v4)
  • Maven dependency caching for faster builds
  • if: always() on artifact upload ensures test results are captured even on failure
  • 7-day retention is reasonable for debugging purposes
pom.xml (1)

46-47: Java 1.7 source/target is compatible with the CI matrix.

The change to Java 1.7 source/target compatibility is appropriate and will work correctly with all JDK versions (8, 11, 17, 21) configured in the CI workflow.

@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

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
pom.xml (1)

140-141: Upgrade JUnit to fix known security vulnerability.

JUnit 4.13.1 has a known vulnerability (CVE-2020-15250) in the TemporaryFolder rule. Although this is test-scoped, it's recommended to upgrade to 4.13.2 which fixes this issue.

🔒 Proposed fix
         <dependency>
             <groupId>junit</groupId>
             <artifactId>junit</artifactId>
-            <version>4.13.1</version>
+            <version>4.13.2</version>
             <scope>test</scope>
         </dependency>
🤖 Fix all issues with AI agents
In @pom.xml:
- Line 44: Replace the old maven-compiler-plugin version 3.1 with 3.14.1 in the
pom (update the <version> under the maven-compiler-plugin plugin definition) and
switch any <source>/<target> configuration to the <release> setting (e.g.,
<release>25</release>) so the plugin uses the --release flag for JDK 25
compatibility; ensure the plugin entry references artifactId
maven-compiler-plugin and that the new version is applied to your build/plugins
or pluginManagement section.
🧹 Nitpick comments (1)
pom.xml (1)

95-102: Consider updating maven-javadoc-plugin and deprecated parameter.

The <additionalparam> configuration element was deprecated in maven-javadoc-plugin 3.0.0 and replaced with <additionalOptions>. Consider updating the plugin version and parameter name.

♻️ Proposed update
             <plugin>
                 <artifactId>maven-javadoc-plugin</artifactId>
-                <version>2.9.1</version>
+                <version>3.6.3</version>
                 <executions>
                     <execution>
                         <id>attach-javadocs</id>
                         <goals><goal>jar</goal></goals>
                         <configuration>
-                            <additionalparam>-Xdoclint:none</additionalparam>
+                            <additionalOptions>-Xdoclint:none</additionalOptions>
                         </configuration>
                     </execution>
                 </executions>
             </plugin>
📜 Review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between c73e53c and df1f718.

📒 Files selected for processing (2)
  • .github/workflows/ci.yml
  • pom.xml
🚧 Files skipped from review as they are similar to previous changes (1)
  • .github/workflows/ci.yml
🔇 Additional comments (1)
pom.xml (1)

46-47: Java 8 upgrade looks good, but verify the cross-compilation strategy.

The upgrade from Java 1.6 to Java 8 is a positive modernization step. However, ensure that compiling with JDK 25 (as mentioned in the commit message) while targeting Java 8 bytecode works correctly across all test matrix environments (Java 8, 11, 17, 21).

Consider using the --release 8 flag instead of source and target for more reliable cross-compilation, as it ensures the compiled code uses only APIs available in Java 8. This requires maven-compiler-plugin 3.6 or newer (see previous comment about updating the plugin).

Comment thread pom.xml Outdated
@@ -43,8 +43,8 @@
<artifactId>maven-compiler-plugin</artifactId>
<version>3.1</version>

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 | 🟠 Major

🧩 Analysis chain

🌐 Web query:

maven-compiler-plugin latest version 2026 JDK 25

💡 Result:

Latest stable Maven Compiler Plugin (as of Jan 7, 2026): 3.14.1. [1][2]

JDK 25 support: 3.14.1 includes fixes to enable working with JDK 25, but some users still report JDK‑25 compatibility errors in certain setups. If you need bleeding‑edge support, there is a 4.0.0‑beta line (requires newer Maven/JDK) you can evaluate. Test your build (or use toolchains) and upgrade to 3.14.1 (or try 4.x beta) if you encounter JDK‑25 issues. [3][4]

Sources:

  • Plugin download / current stable: maven‑compiler‑plugin 3.14.1. [1]
  • Plugin docs / compile goal shows 3.14.1 as current stable. [2]
  • Release discussion noting fixes for JDK 25 and ongoing issues. [3]
  • 4.0.0‑beta docs (4.x branch). [4]

🌐 Web query:

maven-compiler-plugin 3.1 JDK 25 support compatibility

💡 Result:

Short answer: No — maven-compiler-plugin 3.1 (released 2013) does not support Java 25 or the modern --release handling for recent Java versions. Use a recent plugin release (3.6+ for early Java 9 support and preferably a current 3.13+/4.x release) and set <maven.compiler.release>25 (or 25) while running Maven on a JDK 25. [1][2][3]

Sources:

  • maven-compiler-plugin 3.1 plugin page (release 2013). [1]
  • Compiler plugin docs on using --release / maven.compiler.release (current archives and 4.x docs). [2][3]

🌐 Web query:

maven-compiler-plugin --release flag minimum version requirement

💡 Result:

The maven-compiler-plugin added the (javac --release) option in version 3.6. Note: the javac --release JVM option itself requires JDK 9+, and plugin 3.13.0 added special handling so the plugin can accept the release property when running on JDK 8 (it will map to source/target). [1][2]

Sources:

  • Plugin documentation (compile mojo) — since 3.6. [1]
  • Plugin docs for release usage / JDK‑8 behavior (3.13.0 notes). [2]

Update maven-compiler-plugin to 3.14.1 for JDK 25 compatibility.

The maven-compiler-plugin version 3.1 (from 2013) does not support JDK 25. The CI workflow uses JDK 25 for compilation, so this plugin must be upgraded to avoid build failures or incorrect bytecode generation.

🔧 Recommended update
-                <version>3.1</version>
+                <version>3.14.1</version>

Additionally, use the --release flag instead of source/target for better cross-compilation guarantees (requires plugin 3.6+):

                 <configuration>
-                    <source>8</source>
-                    <target>8</target>
+                    <release>8</release>
                 </configuration>
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
<version>3.1</version>
<version>3.14.1</version>
🤖 Prompt for AI Agents
In @pom.xml at line 44, Replace the old maven-compiler-plugin version 3.1 with
3.14.1 in the pom (update the <version> under the maven-compiler-plugin plugin
definition) and switch any <source>/<target> configuration to the <release>
setting (e.g., <release>25</release>) so the plugin uses the --release flag for
JDK 25 compatibility; ensure the plugin entry references artifactId
maven-compiler-plugin and that the new version is applied to your build/plugins
or pluginManagement section.

@julman99
julman99 force-pushed the feature/github-actions-ci branch from 0448a6f to 2ae16a4 Compare January 7, 2026 11:32
@julman99 julman99 changed the title Add GitHub Actions CI workflow for automated testing Upgrade to Java 8 and add GitHub Actions CI Jan 7, 2026
@julman99
julman99 merged commit 5d3ab45 into master Jan 7, 2026
2 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