You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@maven.apache.org by "sparsick (via GitHub)" <gi...@apache.org> on 2025/10/23 10:47:15 UTC

[PR] chore: migrate junit3/4 to junit 5 [maven-checkstyle-plugin]

sparsick opened a new pull request, #639:
URL: https://github.com/apache/maven-checkstyle-plugin/pull/639

   AbstractMojoTestCase based tests will be migrated in a separate PR.
   
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] chore: migrate junit3/4 to junit 5 [maven-checkstyle-plugin]

Posted by "sparsick (via GitHub)" <gi...@apache.org>.
sparsick commented on code in PR #639:
URL: https://github.com/apache/maven-checkstyle-plugin/pull/639#discussion_r2493471943


##########
src/test/java/org/apache/maven/plugins/checkstyle/CheckstyleReportTest.java:
##########
@@ -19,137 +19,880 @@
 package org.apache.maven.plugins.checkstyle;
 
 import java.io.File;
+import java.io.IOException;
 import java.nio.charset.StandardCharsets;
 import java.nio.file.Files;
+import java.util.Collections;
+import java.util.List;
 import java.util.ResourceBundle;
 
-import org.apache.maven.artifact.DependencyResolutionRequiredException;
+import org.apache.maven.api.di.Provides;
+import org.apache.maven.api.plugin.testing.InjectMojo;
+import org.apache.maven.api.plugin.testing.MojoParameter;
+import org.apache.maven.api.plugin.testing.MojoTest;
 import org.apache.maven.doxia.tools.SiteTool;
+import org.apache.maven.model.Plugin;
+import org.apache.maven.plugin.LegacySupport;
+import org.apache.maven.plugin.MojoExecution;
+import org.apache.maven.plugin.descriptor.MojoDescriptor;
 import org.apache.maven.plugin.descriptor.PluginDescriptor;
+import org.apache.maven.plugin.testing.ArtifactStubFactory;
+import org.apache.maven.plugin.testing.stubs.MavenProjectStub;
+import org.apache.maven.project.DefaultProjectBuildingRequest;
+import org.apache.maven.project.MavenProject;
+import org.apache.maven.project.ProjectBuilder;
+import org.apache.maven.project.ProjectBuildingRequest;
+import org.eclipse.aether.DefaultRepositorySystemSession;
+import org.eclipse.aether.RepositorySystemSession;
+import org.eclipse.aether.internal.impl.SimpleLocalRepositoryManagerFactory;
+import org.eclipse.aether.repository.LocalRepository;
+import org.eclipse.aether.repository.NoLocalRepositoryManagerException;
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+
+import javax.inject.Inject;
+
+import static org.apache.maven.api.plugin.testing.MojoExtension.getVariableValueFromObject;
+import static org.codehaus.plexus.testing.PlexusExtension.getBasedir;
+import static org.codehaus.plexus.testing.PlexusExtension.getTestFile;
+import static org.junit.jupiter.api.Assertions.*;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+import static org.junit.jupiter.api.Assertions.fail;
 
 /**
  * @author Edwin Punzalan
  */
-public class CheckstyleReportTest extends AbstractCheckstyleTestCase {
-    public void testNoSource() throws Exception {
-        File generatedReport = generateReport(getGoal(), "no-source-plugin-config.xml");
-        assertFalse(new File(generatedReport.getAbsolutePath()).exists());
-    }
+@MojoTest
+public class CheckstyleReportTest {
 
-    public void testMinConfiguration() throws Exception {
-        generateReport("min-plugin-config.xml");
-    }
+    private ArtifactStubFactory artifactStubFactory;
+    /**
+     * The project to test.
+     */
+    private MavenProject testMavenProject;
 
-    public void testCustomConfiguration() throws Exception {
-        generateReport("custom-plugin-config.xml");
-    }
+    @Inject
+    private MojoExecution mojoExecution;
 
-    public void testUseFile() throws Exception {
-        generateReport("useFile-plugin-config.xml");
-    }
+    @Inject
+    private PluginDescriptor pluginDescriptor;
 
-    public void testNoRulesSummary() throws Exception {
-        generateReport("no-rules-plugin-config.xml");
-    }
+    @Inject
+    private LegacySupport  legacySupport;
 
-    public void testNoSeveritySummary() throws Exception {
-        generateReport("no-severity-plugin-config.xml");
+    @BeforeEach
+    public void setUp() throws Exception {
+        artifactStubFactory = new DependencyArtifactStubFactory(getTestFile("target"), true, false);
+        artifactStubFactory.getWorkingDir().mkdirs();
     }
 
-    public void testNoFilesSummary() throws Exception {
-        generateReport("no-files-plugin-config.xml");
-    }
+    @Provides
+    private MojoExecution getMockMojoExecution() {
+        MojoDescriptor md = new MojoDescriptor();
+        md.setGoal("checkstyle");
 
-    public void testFailOnError() {
-        try {
-            generateReport("fail-on-error-plugin-config.xml");
+        MojoExecution me = new MojoExecution(md);
 
-            fail("Must throw exception on errors");
-        } catch (Exception e) {
-            assertNotNull(e.getMessage());
-        }
+        PluginDescriptor pd = new PluginDescriptor();
+        Plugin p = new Plugin();
+        p.setGroupId("org.apache.maven.plugins");
+        p.setArtifactId("maven-checkstyle-plugin");
+        pd.setPlugin(p);
+        md.setPluginDescriptor(pd);
+
+        return me;
     }
 
-    public void testDependencyResolutionException() {
-        try {
-            generateReport("dep-resolution-exception-plugin-config.xml");
-
-            fail("Must throw exception on errors");
-        } catch (Exception e) {
-            if (!(e.getCause().getCause().getCause() instanceof DependencyResolutionRequiredException)) {
-                e.printStackTrace();
-                fail("Must throw exception DependencyResolutionRequiredException on errors and not "
-                        + e.getClass().getName() + ", " + e.getMessage());
-            }
-        }
+    @Provides
+    private PluginDescriptor getMockPluginDescriptor() {
+        PluginDescriptor descriptorStub = new PluginDescriptor();
+        descriptorStub.setGroupId("org.apache.maven.plugins");
+        descriptorStub.setArtifactId("maven-checkstyle-plugin");
+        return descriptorStub;
     }
 
-    public void testTestSourceDirectory() throws Exception {
-        generateReport("test-source-directory-plugin-config.xml");
+    @InjectMojo(goal="checkstyle" , pom ="src/test/resources/plugin-configs/no-source-plugin-config.xml")
+    @MojoParameter(name = "siteDirectory", value = "src/site")
+    @Test
+    public void testNoSource(CheckstyleReport mojo) throws Exception {
+        mojo.execute();
+
+        File outputDir = mojo.getReportOutputDirectory();
+        String filename = mojo.getOutputName() + ".html";
+        File generatedReport = new File(outputDir, filename);
+        assertFalse(new File(generatedReport.getAbsolutePath()).exists());
     }
 
-    private void generateReport(String pluginXml) throws Exception {
-        File pluginXmlFile = new File(getBasedir(), "src/test/resources/plugin-configs/" + pluginXml);
+
+    @InjectMojo(goal="checkstyle" , pom ="src/test/resources/plugin-configs/min-plugin-config.xml")
+    @MojoParameter(name = "siteDirectory", value = "src/site")
+    @Test
+    public void testMinConfiguration(CheckstyleReport mojo) throws Exception {
         ResourceBundle bundle =
-                ResourceBundle.getBundle("checkstyle-report", SiteTool.DEFAULT_LOCALE, this.getClassLoader());
+                ResourceBundle.getBundle("checkstyle-report", SiteTool.DEFAULT_LOCALE, getClassLoader());
+//
+//        LegacySupport legacySupport = lookup(LegacySupport.class);
+//        legacySupport.setSession(newMavenSession(new MavenProjectStub()));
+        DefaultRepositorySystemSession repoSession =
+                (DefaultRepositorySystemSession) legacySupport.getRepositorySession();
+        repoSession.setLocalRepositoryManager(new SimpleLocalRepositoryManagerFactory()
+                .newInstance(repoSession, new LocalRepository(artifactStubFactory.getWorkingDir())));

Review Comment:
   FYI: m-pmd-plugin has similar test code.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] chore: migrate junit3/4 to junit 5 [maven-checkstyle-plugin]

Posted by "sparsick (via GitHub)" <gi...@apache.org>.
sparsick commented on PR #639:
URL: https://github.com/apache/maven-checkstyle-plugin/pull/639#issuecomment-3489718174

   > I will split it tomorrow.
   
   I revert the MojoTest related changes and put in on a separate [PR](https://github.com/apache/maven-checkstyle-plugin/pull/645)


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] chore: migrate junit3/4 to junit 5 [maven-checkstyle-plugin]

Posted by "sparsick (via GitHub)" <gi...@apache.org>.
sparsick commented on code in PR #639:
URL: https://github.com/apache/maven-checkstyle-plugin/pull/639#discussion_r2493303065


##########
src/test/java/org/apache/maven/plugins/checkstyle/exec/CheckstyleExecutorTest.java:
##########
@@ -18,11 +18,19 @@
  */
 package org.apache.maven.plugins.checkstyle.exec;
 
-import org.apache.maven.plugin.testing.AbstractMojoTestCase;
+import org.apache.maven.api.plugin.testing.MojoTest;
+import org.junit.jupiter.api.Test;
 
-public class CheckstyleExecutorTest extends AbstractMojoTestCase {
+import javax.inject.Inject;
+
+import static org.junit.jupiter.api.Assertions.assertNotNull;
+
+@MojoTest
+public class CheckstyleExecutorTest {
+    @Inject
+    private CheckstyleExecutor executor;
+    @Test
     public void testDefaultConfig() throws Exception {
-        CheckstyleExecutor executor = (CheckstyleExecutor) lookup(CheckstyleExecutor.class);
-        assertNotNull("project null.", executor);
+        assertNotNull(executor, "project null.");
     }

Review Comment:
   This migration is moved to [PR](https://github.com/apache/maven-checkstyle-plugin/pull/645)



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] chore: migrate junit3/4 to junit 5 [maven-checkstyle-plugin]

Posted by "slawekjaranowski (via GitHub)" <gi...@apache.org>.
slawekjaranowski commented on code in PR #639:
URL: https://github.com/apache/maven-checkstyle-plugin/pull/639#discussion_r2508998645


##########
src/test/java/org/apache/maven/plugins/checkstyle/CheckstyleReportTest.java:
##########
@@ -26,40 +26,57 @@
 import org.apache.maven.artifact.DependencyResolutionRequiredException;
 import org.apache.maven.doxia.tools.SiteTool;
 import org.apache.maven.plugin.descriptor.PluginDescriptor;
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
 
 /**
  * @author Edwin Punzalan
  */
 public class CheckstyleReportTest extends AbstractCheckstyleTestCase {

Review Comment:
   `AbstractCheckstyleTestCase` still extends `AbstractMojoTestCase` so it is JUnit3 test, so we can not use annotations from JUnit5



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] chore: migrate junit3/4 to junit 5 [maven-checkstyle-plugin]

Posted by "slawekjaranowski (via GitHub)" <gi...@apache.org>.
slawekjaranowski commented on PR #639:
URL: https://github.com/apache/maven-checkstyle-plugin/pull/639#issuecomment-3487977476

   I would like to split such PR  - one update in ITs and next with unit tests updates


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] chore: migrate junit3/4 to junit 5 [maven-checkstyle-plugin]

Posted by "slawekjaranowski (via GitHub)" <gi...@apache.org>.
slawekjaranowski commented on code in PR #639:
URL: https://github.com/apache/maven-checkstyle-plugin/pull/639#discussion_r2492198355


##########
src/test/java/org/apache/maven/plugins/checkstyle/exec/CheckstyleExecutorTest.java:
##########
@@ -18,11 +18,19 @@
  */
 package org.apache.maven.plugins.checkstyle.exec;
 
-import org.apache.maven.plugin.testing.AbstractMojoTestCase;
+import org.apache.maven.api.plugin.testing.MojoTest;
+import org.junit.jupiter.api.Test;
 
-public class CheckstyleExecutorTest extends AbstractMojoTestCase {
+import javax.inject.Inject;
+
+import static org.junit.jupiter.api.Assertions.assertNotNull;
+
+@MojoTest
+public class CheckstyleExecutorTest {
+    @Inject
+    private CheckstyleExecutor executor;
+    @Test
     public void testDefaultConfig() throws Exception {
-        CheckstyleExecutor executor = (CheckstyleExecutor) lookup(CheckstyleExecutor.class);
-        assertNotNull("project null.", executor);
+        assertNotNull(executor, "project null.");
     }

Review Comment:
   It looks like PlexusTest not MojoTest 



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] chore: migrate junit3/4 to junit 5 [maven-checkstyle-plugin]

Posted by "sparsick (via GitHub)" <gi...@apache.org>.
sparsick commented on code in PR #639:
URL: https://github.com/apache/maven-checkstyle-plugin/pull/639#discussion_r2509922525


##########
src/test/java/org/apache/maven/plugins/checkstyle/CheckstyleReportTest.java:
##########
@@ -26,40 +26,57 @@
 import org.apache.maven.artifact.DependencyResolutionRequiredException;
 import org.apache.maven.doxia.tools.SiteTool;
 import org.apache.maven.plugin.descriptor.PluginDescriptor;
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
 
 /**
  * @author Edwin Punzalan
  */
 public class CheckstyleReportTest extends AbstractCheckstyleTestCase {

Review Comment:
   revert the mix of junit3 and junit 5



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] chore: migrate junit3/4 to junit 5 [maven-checkstyle-plugin]

Posted by "slawekjaranowski (via GitHub)" <gi...@apache.org>.
slawekjaranowski commented on code in PR #639:
URL: https://github.com/apache/maven-checkstyle-plugin/pull/639#discussion_r2492208665


##########
src/test/java/org/apache/maven/plugins/checkstyle/CheckstyleReportTest.java:
##########
@@ -19,137 +19,880 @@
 package org.apache.maven.plugins.checkstyle;
 
 import java.io.File;
+import java.io.IOException;
 import java.nio.charset.StandardCharsets;
 import java.nio.file.Files;
+import java.util.Collections;
+import java.util.List;
 import java.util.ResourceBundle;
 
-import org.apache.maven.artifact.DependencyResolutionRequiredException;
+import org.apache.maven.api.di.Provides;
+import org.apache.maven.api.plugin.testing.InjectMojo;
+import org.apache.maven.api.plugin.testing.MojoParameter;
+import org.apache.maven.api.plugin.testing.MojoTest;
 import org.apache.maven.doxia.tools.SiteTool;
+import org.apache.maven.model.Plugin;
+import org.apache.maven.plugin.LegacySupport;
+import org.apache.maven.plugin.MojoExecution;
+import org.apache.maven.plugin.descriptor.MojoDescriptor;
 import org.apache.maven.plugin.descriptor.PluginDescriptor;
+import org.apache.maven.plugin.testing.ArtifactStubFactory;
+import org.apache.maven.plugin.testing.stubs.MavenProjectStub;
+import org.apache.maven.project.DefaultProjectBuildingRequest;
+import org.apache.maven.project.MavenProject;
+import org.apache.maven.project.ProjectBuilder;
+import org.apache.maven.project.ProjectBuildingRequest;
+import org.eclipse.aether.DefaultRepositorySystemSession;
+import org.eclipse.aether.RepositorySystemSession;
+import org.eclipse.aether.internal.impl.SimpleLocalRepositoryManagerFactory;
+import org.eclipse.aether.repository.LocalRepository;
+import org.eclipse.aether.repository.NoLocalRepositoryManagerException;
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+
+import javax.inject.Inject;
+
+import static org.apache.maven.api.plugin.testing.MojoExtension.getVariableValueFromObject;
+import static org.codehaus.plexus.testing.PlexusExtension.getBasedir;
+import static org.codehaus.plexus.testing.PlexusExtension.getTestFile;
+import static org.junit.jupiter.api.Assertions.*;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+import static org.junit.jupiter.api.Assertions.fail;
 
 /**
  * @author Edwin Punzalan
  */
-public class CheckstyleReportTest extends AbstractCheckstyleTestCase {
-    public void testNoSource() throws Exception {
-        File generatedReport = generateReport(getGoal(), "no-source-plugin-config.xml");
-        assertFalse(new File(generatedReport.getAbsolutePath()).exists());
-    }
+@MojoTest
+public class CheckstyleReportTest {
 
-    public void testMinConfiguration() throws Exception {
-        generateReport("min-plugin-config.xml");
-    }
+    private ArtifactStubFactory artifactStubFactory;
+    /**
+     * The project to test.
+     */
+    private MavenProject testMavenProject;
 
-    public void testCustomConfiguration() throws Exception {
-        generateReport("custom-plugin-config.xml");
-    }
+    @Inject
+    private MojoExecution mojoExecution;
 
-    public void testUseFile() throws Exception {
-        generateReport("useFile-plugin-config.xml");
-    }
+    @Inject
+    private PluginDescriptor pluginDescriptor;
 
-    public void testNoRulesSummary() throws Exception {
-        generateReport("no-rules-plugin-config.xml");
-    }
+    @Inject
+    private LegacySupport  legacySupport;
 
-    public void testNoSeveritySummary() throws Exception {
-        generateReport("no-severity-plugin-config.xml");
+    @BeforeEach
+    public void setUp() throws Exception {
+        artifactStubFactory = new DependencyArtifactStubFactory(getTestFile("target"), true, false);
+        artifactStubFactory.getWorkingDir().mkdirs();
     }
 
-    public void testNoFilesSummary() throws Exception {
-        generateReport("no-files-plugin-config.xml");
-    }
+    @Provides
+    private MojoExecution getMockMojoExecution() {
+        MojoDescriptor md = new MojoDescriptor();
+        md.setGoal("checkstyle");
 
-    public void testFailOnError() {
-        try {
-            generateReport("fail-on-error-plugin-config.xml");
+        MojoExecution me = new MojoExecution(md);
 
-            fail("Must throw exception on errors");
-        } catch (Exception e) {
-            assertNotNull(e.getMessage());
-        }
+        PluginDescriptor pd = new PluginDescriptor();
+        Plugin p = new Plugin();
+        p.setGroupId("org.apache.maven.plugins");
+        p.setArtifactId("maven-checkstyle-plugin");
+        pd.setPlugin(p);
+        md.setPluginDescriptor(pd);
+
+        return me;
     }
 
-    public void testDependencyResolutionException() {
-        try {
-            generateReport("dep-resolution-exception-plugin-config.xml");
-
-            fail("Must throw exception on errors");
-        } catch (Exception e) {
-            if (!(e.getCause().getCause().getCause() instanceof DependencyResolutionRequiredException)) {
-                e.printStackTrace();
-                fail("Must throw exception DependencyResolutionRequiredException on errors and not "
-                        + e.getClass().getName() + ", " + e.getMessage());
-            }
-        }
+    @Provides
+    private PluginDescriptor getMockPluginDescriptor() {
+        PluginDescriptor descriptorStub = new PluginDescriptor();
+        descriptorStub.setGroupId("org.apache.maven.plugins");
+        descriptorStub.setArtifactId("maven-checkstyle-plugin");
+        return descriptorStub;
     }
 
-    public void testTestSourceDirectory() throws Exception {
-        generateReport("test-source-directory-plugin-config.xml");
+    @InjectMojo(goal="checkstyle" , pom ="src/test/resources/plugin-configs/no-source-plugin-config.xml")
+    @MojoParameter(name = "siteDirectory", value = "src/site")
+    @Test
+    public void testNoSource(CheckstyleReport mojo) throws Exception {
+        mojo.execute();
+
+        File outputDir = mojo.getReportOutputDirectory();
+        String filename = mojo.getOutputName() + ".html";
+        File generatedReport = new File(outputDir, filename);
+        assertFalse(new File(generatedReport.getAbsolutePath()).exists());
     }
 
-    private void generateReport(String pluginXml) throws Exception {
-        File pluginXmlFile = new File(getBasedir(), "src/test/resources/plugin-configs/" + pluginXml);
+
+    @InjectMojo(goal="checkstyle" , pom ="src/test/resources/plugin-configs/min-plugin-config.xml")
+    @MojoParameter(name = "siteDirectory", value = "src/site")
+    @Test
+    public void testMinConfiguration(CheckstyleReport mojo) throws Exception {
         ResourceBundle bundle =
-                ResourceBundle.getBundle("checkstyle-report", SiteTool.DEFAULT_LOCALE, this.getClassLoader());
+                ResourceBundle.getBundle("checkstyle-report", SiteTool.DEFAULT_LOCALE, getClassLoader());
+//
+//        LegacySupport legacySupport = lookup(LegacySupport.class);
+//        legacySupport.setSession(newMavenSession(new MavenProjectStub()));
+        DefaultRepositorySystemSession repoSession =
+                (DefaultRepositorySystemSession) legacySupport.getRepositorySession();
+        repoSession.setLocalRepositoryManager(new SimpleLocalRepositoryManagerFactory()
+                .newInstance(repoSession, new LocalRepository(artifactStubFactory.getWorkingDir())));

Review Comment:
   Legacy staff is not used by `AbstractMavenReport` ... 
   but AbstractMavenReport needs a remote repository to resolve site skin, so it looks like more integration test
   
   There is side effect with such old staff that test is working
   
   I need to discover how to achieve similar effect with new way 



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] chore: migrate junit3/4 to junit 5 [maven-checkstyle-plugin]

Posted by "sparsick (via GitHub)" <gi...@apache.org>.
sparsick commented on PR #639:
URL: https://github.com/apache/maven-checkstyle-plugin/pull/639#issuecomment-3509653412

   @slawekjaranowski please take a look at this PR. Then I could merge both related PRs.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] chore: migrate junit3/4 to junit 5 [maven-checkstyle-plugin]

Posted by "slawekjaranowski (via GitHub)" <gi...@apache.org>.
slawekjaranowski commented on code in PR #639:
URL: https://github.com/apache/maven-checkstyle-plugin/pull/639#discussion_r2508995959


##########
src/test/java/org/apache/maven/plugins/checkstyle/exec/CheckstyleExecutorTest.java:
##########
@@ -19,10 +19,13 @@
 package org.apache.maven.plugins.checkstyle.exec;
 
 import org.apache.maven.plugin.testing.AbstractMojoTestCase;
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.Test;
 
 public class CheckstyleExecutorTest extends AbstractMojoTestCase {
+    @Test

Review Comment:
   Still extends `AbstractMojoTestCase` so it is JUnit3 test, so we can not use annotations from JUnit5



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] chore: migrate junit3/4 to junit 5 [maven-checkstyle-plugin]

Posted by "sparsick (via GitHub)" <gi...@apache.org>.
sparsick commented on code in PR #639:
URL: https://github.com/apache/maven-checkstyle-plugin/pull/639#discussion_r2499643133


##########
src/test/java/org/apache/maven/plugins/checkstyle/CheckstyleReportTest.java:
##########
@@ -19,137 +19,880 @@
 package org.apache.maven.plugins.checkstyle;
 
 import java.io.File;
+import java.io.IOException;
 import java.nio.charset.StandardCharsets;
 import java.nio.file.Files;
+import java.util.Collections;
+import java.util.List;
 import java.util.ResourceBundle;
 
-import org.apache.maven.artifact.DependencyResolutionRequiredException;
+import org.apache.maven.api.di.Provides;
+import org.apache.maven.api.plugin.testing.InjectMojo;
+import org.apache.maven.api.plugin.testing.MojoParameter;
+import org.apache.maven.api.plugin.testing.MojoTest;
 import org.apache.maven.doxia.tools.SiteTool;
+import org.apache.maven.model.Plugin;
+import org.apache.maven.plugin.LegacySupport;
+import org.apache.maven.plugin.MojoExecution;
+import org.apache.maven.plugin.descriptor.MojoDescriptor;
 import org.apache.maven.plugin.descriptor.PluginDescriptor;
+import org.apache.maven.plugin.testing.ArtifactStubFactory;
+import org.apache.maven.plugin.testing.stubs.MavenProjectStub;
+import org.apache.maven.project.DefaultProjectBuildingRequest;
+import org.apache.maven.project.MavenProject;
+import org.apache.maven.project.ProjectBuilder;
+import org.apache.maven.project.ProjectBuildingRequest;
+import org.eclipse.aether.DefaultRepositorySystemSession;
+import org.eclipse.aether.RepositorySystemSession;
+import org.eclipse.aether.internal.impl.SimpleLocalRepositoryManagerFactory;
+import org.eclipse.aether.repository.LocalRepository;
+import org.eclipse.aether.repository.NoLocalRepositoryManagerException;
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+
+import javax.inject.Inject;
+
+import static org.apache.maven.api.plugin.testing.MojoExtension.getVariableValueFromObject;
+import static org.codehaus.plexus.testing.PlexusExtension.getBasedir;
+import static org.codehaus.plexus.testing.PlexusExtension.getTestFile;
+import static org.junit.jupiter.api.Assertions.*;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+import static org.junit.jupiter.api.Assertions.fail;
 
 /**
  * @author Edwin Punzalan
  */
-public class CheckstyleReportTest extends AbstractCheckstyleTestCase {
-    public void testNoSource() throws Exception {
-        File generatedReport = generateReport(getGoal(), "no-source-plugin-config.xml");
-        assertFalse(new File(generatedReport.getAbsolutePath()).exists());
-    }
+@MojoTest
+public class CheckstyleReportTest {
 
-    public void testMinConfiguration() throws Exception {
-        generateReport("min-plugin-config.xml");
-    }
+    private ArtifactStubFactory artifactStubFactory;
+    /**
+     * The project to test.
+     */
+    private MavenProject testMavenProject;
 
-    public void testCustomConfiguration() throws Exception {
-        generateReport("custom-plugin-config.xml");
-    }
+    @Inject
+    private MojoExecution mojoExecution;
 
-    public void testUseFile() throws Exception {
-        generateReport("useFile-plugin-config.xml");
-    }
+    @Inject
+    private PluginDescriptor pluginDescriptor;
 
-    public void testNoRulesSummary() throws Exception {
-        generateReport("no-rules-plugin-config.xml");
-    }
+    @Inject
+    private LegacySupport  legacySupport;
 
-    public void testNoSeveritySummary() throws Exception {
-        generateReport("no-severity-plugin-config.xml");
+    @BeforeEach
+    public void setUp() throws Exception {
+        artifactStubFactory = new DependencyArtifactStubFactory(getTestFile("target"), true, false);
+        artifactStubFactory.getWorkingDir().mkdirs();
     }
 
-    public void testNoFilesSummary() throws Exception {
-        generateReport("no-files-plugin-config.xml");
-    }
+    @Provides
+    private MojoExecution getMockMojoExecution() {
+        MojoDescriptor md = new MojoDescriptor();
+        md.setGoal("checkstyle");
 
-    public void testFailOnError() {
-        try {
-            generateReport("fail-on-error-plugin-config.xml");
+        MojoExecution me = new MojoExecution(md);
 
-            fail("Must throw exception on errors");
-        } catch (Exception e) {
-            assertNotNull(e.getMessage());
-        }
+        PluginDescriptor pd = new PluginDescriptor();
+        Plugin p = new Plugin();
+        p.setGroupId("org.apache.maven.plugins");
+        p.setArtifactId("maven-checkstyle-plugin");
+        pd.setPlugin(p);
+        md.setPluginDescriptor(pd);
+
+        return me;
     }
 
-    public void testDependencyResolutionException() {
-        try {
-            generateReport("dep-resolution-exception-plugin-config.xml");
-
-            fail("Must throw exception on errors");
-        } catch (Exception e) {
-            if (!(e.getCause().getCause().getCause() instanceof DependencyResolutionRequiredException)) {
-                e.printStackTrace();
-                fail("Must throw exception DependencyResolutionRequiredException on errors and not "
-                        + e.getClass().getName() + ", " + e.getMessage());
-            }
-        }
+    @Provides
+    private PluginDescriptor getMockPluginDescriptor() {
+        PluginDescriptor descriptorStub = new PluginDescriptor();
+        descriptorStub.setGroupId("org.apache.maven.plugins");
+        descriptorStub.setArtifactId("maven-checkstyle-plugin");
+        return descriptorStub;
     }
 
-    public void testTestSourceDirectory() throws Exception {
-        generateReport("test-source-directory-plugin-config.xml");
+    @InjectMojo(goal="checkstyle" , pom ="src/test/resources/plugin-configs/no-source-plugin-config.xml")
+    @MojoParameter(name = "siteDirectory", value = "src/site")
+    @Test
+    public void testNoSource(CheckstyleReport mojo) throws Exception {
+        mojo.execute();
+
+        File outputDir = mojo.getReportOutputDirectory();
+        String filename = mojo.getOutputName() + ".html";
+        File generatedReport = new File(outputDir, filename);
+        assertFalse(new File(generatedReport.getAbsolutePath()).exists());
     }
 
-    private void generateReport(String pluginXml) throws Exception {
-        File pluginXmlFile = new File(getBasedir(), "src/test/resources/plugin-configs/" + pluginXml);
+
+    @InjectMojo(goal="checkstyle" , pom ="src/test/resources/plugin-configs/min-plugin-config.xml")
+    @MojoParameter(name = "siteDirectory", value = "src/site")
+    @Test
+    public void testMinConfiguration(CheckstyleReport mojo) throws Exception {
         ResourceBundle bundle =
-                ResourceBundle.getBundle("checkstyle-report", SiteTool.DEFAULT_LOCALE, this.getClassLoader());
+                ResourceBundle.getBundle("checkstyle-report", SiteTool.DEFAULT_LOCALE, getClassLoader());
+//
+//        LegacySupport legacySupport = lookup(LegacySupport.class);
+//        legacySupport.setSession(newMavenSession(new MavenProjectStub()));
+        DefaultRepositorySystemSession repoSession =
+                (DefaultRepositorySystemSession) legacySupport.getRepositorySession();
+        repoSession.setLocalRepositoryManager(new SimpleLocalRepositoryManagerFactory()
+                .newInstance(repoSession, new LocalRepository(artifactStubFactory.getWorkingDir())));

Review Comment:
   Solve in a separate [PR](https://github.com/apache/maven-checkstyle-plugin/pull/645) 



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] chore: migrate junit3/4 to junit 5 [maven-checkstyle-plugin]

Posted by "slawekjaranowski (via GitHub)" <gi...@apache.org>.
slawekjaranowski commented on PR #639:
URL: https://github.com/apache/maven-checkstyle-plugin/pull/639#issuecomment-3509774019

   We should not mix JUnit 3/4 and 5 in one test ..


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] chore: migrate junit3/4 to junit 5 [maven-checkstyle-plugin]

Posted by "sparsick (via GitHub)" <gi...@apache.org>.
sparsick merged PR #639:
URL: https://github.com/apache/maven-checkstyle-plugin/pull/639


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] chore: migrate junit3/4 to junit 5 [maven-checkstyle-plugin]

Posted by "sparsick (via GitHub)" <gi...@apache.org>.
sparsick commented on PR #639:
URL: https://github.com/apache/maven-checkstyle-plugin/pull/639#issuecomment-3488017322

   I will split it tomorrow.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org