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

Perform testing using module-path #143

Merged
merged 1 commit into from
Oct 1, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ jobs:
- name: Maven version
run: |
mkdir -p ./.mvn
echo '-e -B -DtrimStackTrace=false' > ./.mvn/maven.config
echo '-e -B -ntp -DtrimStackTrace=false' > ./.mvn/maven.config
mvn --version
mkdir -p target

Expand Down
3 changes: 2 additions & 1 deletion .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -9,4 +9,5 @@
/nbproject/
.idea
*.iml
/test-output/
/test-output/
/src/test/java/module-info.java
182 changes: 118 additions & 64 deletions pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -123,33 +123,103 @@
</execution>
</executions>
</plugin>
<!-- Hack to extract dependencies for Surefire plugin -->
<!-- Aim of this config is to test the project three times. -->
<!-- First, with both main and test code on the classpath (test lifecycle phase). -->
<!-- Then, with main code on the modulepath and test code on the classpath (test lifecycle phase). -->
<!-- Then, with both main and test code on the modulepath (integration-test lifecycle phase). -->
<!-- The last of these is achieved by a Maven workaround/hack. -->
<!-- When maven-compiler-plugin testCompile runs with two module-info.java files with the same module name -->
<!-- the code in both src/test/java and src/main/java is compiled into target/test-classes. -->
<!-- This is a bug, but we can use it to our advantage - all we have to do is patch in src/main/resources -->
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-dependency-plugin</artifactId>
<artifactId>maven-surefire-plugin</artifactId>
<executions>
<!-- test with both main and test code on the classpath -->
<execution>
<id>default-test</id>
<goals>
<goal>test</goal>
</goals>
<configuration>
<useModulePath>false</useModulePath>
</configuration>
</execution>
<!-- test with main code on the modulepath and test code on the classpath -->
<execution>
<id>test-module-classpath</id>
<goals>
<goal>test</goal>
</goals>
<phase>test</phase>
<configuration>
<useModulePath>true</useModulePath>
</configuration>
</execution>
<!-- test with both main and test code on the modulepath (as a single patched module using src/test/java/module-info.java) -->
<!-- by this point, target/test-classes contains the complied form of src/test/java and src/main/java -->
<!-- patch-module ise used to add the missing src/main/resources without copying it -->
<execution>
<id>test-module-whitebox</id>
<goals>
<goal>test</goal>
</goals>
<phase>integration-test</phase>
<configuration>
<argLine>--patch-module org.joda.money=src/main/resources</argLine>
</configuration>
</execution>
</executions>
</plugin>
<!-- Avoid issues with IntelliJ and Eclipse by adding the module-info dynamically -->
<!-- Tests in either IDE will operate as classpath tests -->
<!-- Tests in Maven will operate as whitebox tests -->
<!-- This approach keeps IDEs happy, wheras using build-helper-plugin does not -->
<plugin>
<groupId>com.coderplus.maven.plugins</groupId>
<artifactId>copy-rename-maven-plugin</artifactId>
<version>${copy-rename-maven-plugin.version}</version>
<executions>
<execution>
<id>activate-module-info</id>
<phase>pre-integration-test</phase>
<goals>
<goal>copy</goal>
</goals>
<configuration>
<sourceFile>src/test-whitebox/module-info.java</sourceFile>
<destinationFile>src/test/java/module-info.java</destinationFile>
</configuration>
</execution>
<execution>
<id>copy-dependencies</id>
<phase>compile</phase>
<id>deactivate-module-info</id>
<phase>post-integration-test</phase>
<goals>
<goal>copy-dependencies</goal>
<goal>rename</goal>
</goals>
<configuration>
<outputDirectory>${project.build.directory}/dependencies</outputDirectory>
<overWriteReleases>true</overWriteReleases>
<overWriteIfNewer>true</overWriteIfNewer>
<sourceFile>src/test/java/module-info.java</sourceFile>
<destinationFile>src/test-whitebox/module-info.java</destinationFile>
</configuration>
</execution>
</executions>
</plugin>
<!-- Surefire plugin is broken, https://issues.apache.org/jira/browse/SUREFIRE-1501 -->
<!-- An additional testCompile step. -->
<!-- This recompiles the test code after src/test/java/module-info.java has been added as a test source by build-helper -->
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-surefire-plugin</artifactId>
<configuration>
<argLine>--add-modules org.joda.convert --module-path ${project.build.directory}/dependencies ${argLine}</argLine>
</configuration>
<artifactId>maven-compiler-plugin</artifactId>
<executions>
<execution>
<id>test-module-whitebox</id>
<goals>
<goal>testCompile</goal>
</goals>
<phase>pre-integration-test</phase>
</execution>
</executions>
</plugin>

<!-- Setup OSGi bundle data -->
<plugin>
<groupId>org.apache.felix</groupId>
Expand All @@ -168,6 +238,7 @@
<Export-Package>${joda.osgi.packages}</Export-Package>
<Require-Capability>${joda.osgi.require.capability}</Require-Capability>
</instructions>
<supportIncrementalBuild>true</supportIncrementalBuild>
</configuration>
</execution>
</executions>
Expand Down Expand Up @@ -438,44 +509,6 @@
</dependency>
</dependencies>
</plugin>
<!-- for Eclipse -->
<plugin>
<groupId>org.eclipse.m2e</groupId>
<artifactId>lifecycle-mapping</artifactId>
<version>1.0.0</version>
<configuration>
<lifecycleMappingMetadata>
<pluginExecutions>
<pluginExecution>
<pluginExecutionFilter>
<groupId>org.apache.felix</groupId>
<artifactId>maven-bundle-plugin</artifactId>
<versionRange>[2.5.4,)</versionRange>
<goals>
<goal>manifest</goal>
</goals>
</pluginExecutionFilter>
<action>
<ignore />
</action>
</pluginExecution>
<pluginExecution>
<pluginExecutionFilter>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-dependency-plugin</artifactId>
<versionRange>[3.1.1,)</versionRange>
<goals>
<goal>copy-dependencies</goal>
</goals>
</pluginExecutionFilter>
<action>
<ignore />
</action>
</pluginExecution>
</pluginExecutions>
</lifecycleMappingMetadata>
</configuration>
</plugin>
</plugins>
</pluginManagement>
</build>
Expand Down Expand Up @@ -631,6 +664,26 @@

<!-- ==================================================================== -->
<profiles>
<!-- Hack to allow IntelliJ testing to work, as it doesn't seems to handle 'requires static' properly -->
<profile>
<id>intellij-idea</id>
<activation>
<property>
<name>idea.maven.embedder.version</name>
</property>
</activation>
<build>
<plugins>
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-surefire-plugin</artifactId>
<configuration>
<argLine>--add-modules org.joda.convert</argLine>
</configuration>
</plugin>
</plugins>
</build>
</profile>
<!-- Check for incompatible changes, use `mvn revapi:check -Dcompat -DoldVersion=xxx` -->
<profile>
<id>compat</id>
Expand Down Expand Up @@ -777,32 +830,33 @@
<maven-checkstyle-plugin.version>3.4.0</maven-checkstyle-plugin.version>
<maven-clean-plugin.version>3.4.0</maven-clean-plugin.version>
<maven-compiler-plugin.version>3.13.0</maven-compiler-plugin.version>
<maven-deploy-plugin.version>3.1.2</maven-deploy-plugin.version>
<maven-dependency-plugin.version>3.7.1</maven-dependency-plugin.version>
<maven-deploy-plugin.version>3.1.3</maven-deploy-plugin.version>
<maven-dependency-plugin.version>3.8.0</maven-dependency-plugin.version>
<maven-enforcer-plugin.version>3.5.0</maven-enforcer-plugin.version>
<maven-gpg-plugin.version>3.2.5</maven-gpg-plugin.version>
<maven-install-plugin.version>3.1.2</maven-install-plugin.version>
<maven-gpg-plugin.version>3.2.6</maven-gpg-plugin.version>
<maven-install-plugin.version>3.1.3</maven-install-plugin.version>
<maven-jar-plugin.version>3.4.2</maven-jar-plugin.version>
<maven-javadoc-plugin.version>3.8.0</maven-javadoc-plugin.version>
<maven-jxr-plugin.version>3.4.0</maven-jxr-plugin.version>
<maven-plugin-plugin.version>3.13.1</maven-plugin-plugin.version>
<maven-pmd-plugin.version>3.24.0</maven-pmd-plugin.version>
<maven-project-info-reports-plugin.version>3.6.2</maven-project-info-reports-plugin.version>
<maven-javadoc-plugin.version>3.10.0</maven-javadoc-plugin.version>
<maven-jxr-plugin.version>3.5.0</maven-jxr-plugin.version>
<maven-plugin-plugin.version>3.15.0</maven-plugin-plugin.version>
<maven-pmd-plugin.version>3.25.0</maven-pmd-plugin.version>
<maven-project-info-reports-plugin.version>3.7.0</maven-project-info-reports-plugin.version>
<maven-release-plugin.version>3.1.1</maven-release-plugin.version>
<maven-repository-plugin.version>2.4</maven-repository-plugin.version>
<maven-resources-plugin.version>3.3.1</maven-resources-plugin.version>
<maven-site-plugin.version>3.12.1</maven-site-plugin.version>
<maven-source-plugin.version>3.3.1</maven-source-plugin.version>
<maven-surefire-plugin.version>3.3.1</maven-surefire-plugin.version>
<maven-surefire-report-plugin.version>3.3.1</maven-surefire-report-plugin.version>
<maven-surefire-plugin.version>3.5.0</maven-surefire-plugin.version>
<maven-surefire-report-plugin.version>3.5.0</maven-surefire-report-plugin.version>
<maven-toolchains-plugin.version>3.2.0</maven-toolchains-plugin.version>
<github-api.version>1.323</github-api.version>
<copy-rename-maven-plugin.version>1.0</copy-rename-maven-plugin.version>
<github-api.version>1.326</github-api.version>
<github-release-plugin.version>1.6.0</github-release-plugin.version>
<jacoco-maven-plugin.version>0.8.12</jacoco-maven-plugin.version>
<nexus-staging-maven-plugin.version>1.6.13</nexus-staging-maven-plugin.version>
<revapi-maven-plugin.version>0.11.1</revapi-maven-plugin.version>
<revapi-java.version>0.15.1</revapi-java.version>
<spotbugs-maven-plugin.version>4.8.6.2</spotbugs-maven-plugin.version>
<spotbugs-maven-plugin.version>4.8.6.4</spotbugs-maven-plugin.version>

<!-- Properties for maven-compiler-plugin -->
<maven.compiler.source>21</maven.compiler.source>
Expand Down
33 changes: 33 additions & 0 deletions src/test-whitebox/module-info.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
/*
* Copyright 2009-present, Stephen Colebourne
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

/**
* Joda-Money test module.
*/
open module org.joda.money {

// mandatory for testing
requires org.joda.convert;

// all packages are exported
exports org.joda.money;
exports org.joda.money.format;

requires transitive org.junit.jupiter.api;
requires transitive org.junit.jupiter.engine;
requires transitive org.junit.jupiter.params;
requires transitive org.assertj.core;
}
56 changes: 56 additions & 0 deletions src/test/java/org/joda/money/TestModulepath.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
/*
* Copyright 2009-present, Stephen Colebourne
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package org.joda.money;

import static java.util.stream.Collectors.joining;

import java.util.ArrayList;

import org.junit.jupiter.api.Test;

/**
* Test that the classpath/modulepath is correctly set.
*/
class TestModulepath {

@Test
void dumpPaths() {
var classpath = System.getProperty("java.class.path", "");
var modulepath = System.getProperty("jdk.module.path", "");
System.out.println("Classpath: " + describePath(classpath));
System.out.println("Modulepath: " + describePath(modulepath));
}
Comment on lines +29 to +35
Copy link

@coderabbitai coderabbitai bot Oct 1, 2024

Choose a reason for hiding this comment

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

⚠️ Potential issue

Improve test method implementation and naming.

  1. Consider renaming the method to follow the typical test method naming convention, e.g., testClasspathAndModulepathConfiguration().
  2. Instead of using System.out.println(), consider using assertions to verify the expected classpath and modulepath configurations. This will make the test more robust and self-verifying.
  3. You might want to add specific assertions to check for the presence of expected elements in the classpath and modulepath.

Example:

@Test
void testClasspathAndModulepathConfiguration() {
    String classpath = System.getProperty("java.class.path", "");
    String modulepath = System.getProperty("jdk.module.path", "");
    
    assertThat(describePath(classpath)).contains("target/classes", "target/test-classes");
    assertThat(describePath(modulepath)).contains("joda-convert", "junit-jupiter");
}

Copy link
Member Author

Choose a reason for hiding this comment

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

The same test code is run three times and three different outcomes are expected. As such, it isn't possible to write this with asserts as per normal testing standards.

Copy link

Choose a reason for hiding this comment

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

@jodastephen Understood, thank you for the explanation. Since the test code is run three times with different expected outcomes, using standard assertions isn't feasible here.

(;ω;)


private static String describePath(String path) {
if (path.isEmpty()) {
return "<empty>";
}
var list = new ArrayList<String>();
if (path.contains("target/classes") || path.contains("target\\classes")) {
list.add("target/classes");
}
if (path.contains("target/test-classes") || path.contains("target\\test-classes")) {
list.add("target/test-classes");
}
if (path.contains("joda-convert")) {
list.add("joda-convert");
}
if (path.contains("junit-jupiter")) {
list.add("junit-jupiter");
}
return list.isEmpty() ? path : list.stream().collect(joining(" "));
}
}