Skip to content

Add integration test for multi-release JAR detection#563

Open
elharo wants to merge 2 commits into
apache:masterfrom
elharo:fix-557
Open

Add integration test for multi-release JAR detection#563
elharo wants to merge 2 commits into
apache:masterfrom
elharo:fix-557

Conversation

@elharo

@elharo elharo commented Jul 19, 2026

Copy link
Copy Markdown
Contributor

Adds an integration test that verifies the Multi-Release: true manifest entry is added when the class output contains META-INF/versions/ files.

The detection logic in AbstractJarMojo uses File.separatorChar to build the path prefix, which matches the platform separator used by DirectoryScanner. This test passes on all platforms (Linux, macOS, Windows).

@elharo elharo added the bug Something isn't working label Jul 19, 2026
Adds an integration test that verifies the Multi-Release: true manifest
entry is present when META-INF/versions/ files exist in the class output.
This test passes on Linux/macOS but fails on Windows due to
File.separatorChar mismatch (the prefix check uses backslash on Windows
but FileSetManager always returns forward-slash paths).

Issue: apache#557
@elharo
elharo marked this pull request as draft July 20, 2026 10:14
@elharo elharo changed the title Multi-release JAR detection broken on Windows (File.separatorChar mismatch) Add integration test for multi-release JAR detection Jul 20, 2026
@elharo
elharo marked this pull request as ready for review July 20, 2026 10:26

Copilot AI 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.

Pull request overview

Note

Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.

Adds a Maven Invoker integration test to validate that the built JAR is marked as multi-release (Multi-Release: true) when META-INF/versions/ content is present, improving cross-platform confidence (including Windows).

Changes:

  • Added a new Maven Invoker IT project (MJAR-557-detect-mjar) that packages META-INF/versions/9/... content.
  • Added a verify.groovy script to assert Multi-Release manifest attribute is set to true.
  • Updated root pom.xml excludes to avoid RAT/header checks on the added plain text fixture.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
src/it/MJAR-557-detect-mjar/verify.groovy Verifies the built JAR manifest contains Multi-Release: true.
src/it/MJAR-557-detect-mjar/src/main/resources/META-INF/versions/9/test.txt Adds a META-INF/versions/9/ fixture to trigger MRJAR detection.
src/it/MJAR-557-detect-mjar/pom.xml Defines the IT Maven project that builds a JAR using the plugin under test.
src/it/MJAR-557-detect-mjar/invoker.properties Configures Invoker execution constraints and test description.
pom.xml Excludes the fixture text file from checks (e.g., RAT/license header validation).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +26 to +29
def mrjar = new JarFile(jarFile)
def manifest = mrjar.manifest.mainAttributes

assert manifest.getValue(Attributes.Name.MULTI_RELEASE) == "true"
def mrjar = new JarFile(jarFile)
def manifest = mrjar.manifest.mainAttributes

assert manifest.getValue(Attributes.Name.MULTI_RELEASE) == "true"
@@ -0,0 +1 @@
Test file for multi-release JAR detection
@elharo

elharo commented Jul 20, 2026

Copy link
Copy Markdown
Contributor Author

Addressed Copilot review comments:

  1. Resource leak: Wrapped JarFile in withCloseable to ensure the handle is released.
  2. Assertion message: Added a diagnostic message to the assertion showing the actual Multi-Release value on failure.
  3. PR description: Removed the module-info.class example since the test uses a text file (any entry under META-INF/versions/ triggers detection).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants