[ 
https://issues.apache.org/jira/browse/MDEP-979?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17955174#comment-17955174
 ] 

ASF GitHub Bot commented on MDEP-979:
-------------------------------------

elharo commented on code in PR #532:
URL: 
https://github.com/apache/maven-dependency-plugin/pull/532#discussion_r2115591341


##########
src/test/java/org/apache/maven/plugins/dependency/AbstractDependencyMojoTestCase.java:
##########
@@ -35,33 +36,52 @@
 import org.eclipse.aether.RepositorySystemSession;
 import org.eclipse.aether.repository.LocalRepository;
 import org.eclipse.aether.repository.LocalRepositoryManager;
+import org.junit.Before;
 import org.sonatype.plexus.build.incremental.DefaultBuildContext;
 
 public abstract class AbstractDependencyMojoTestCase extends 
AbstractMojoTestCase {
 
     protected File testDir;
-
     protected DependencyArtifactStubFactory stubFactory;
 
-    protected void setUp(String testDirectoryName, boolean createFiles) throws 
Exception {
-        setUp(testDirectoryName, createFiles, true);
-    }
-
-    protected void setUp(String testDirectoryName, boolean createFiles, 
boolean flattenedPath) throws Exception {
-        // required for mojo lookups to work
+    @Override
+    @Before
+    public void setUp() throws Exception {
+        // Initialize mojo lookups as required by AbstractMojoTestCase
         super.setUp();
 
+        // Create a unique temporary test directory to avoid parallel test 
conflicts
+        String uniqueDirName = "test-dependency" + UUID.randomUUID();
+        testDir = Files.createTempDirectory(uniqueDirName).toFile();
+        testDir.deleteOnExit();
+
+        // Initialize stub factory with default settings
+        stubFactory = new DependencyArtifactStubFactory(testDir, true, true);
+    }
+
+    /**
+     * Allows subclasses to customize the setup with specific test directory 
name and stub factory settings.
+     *
+     * @param testDirectoryName the name for the temporary test directory.

Review Comment:
   no period



##########
src/test/java/org/apache/maven/plugins/dependency/AbstractDependencyMojoTestCase.java:
##########
@@ -35,33 +36,52 @@
 import org.eclipse.aether.RepositorySystemSession;
 import org.eclipse.aether.repository.LocalRepository;
 import org.eclipse.aether.repository.LocalRepositoryManager;
+import org.junit.Before;
 import org.sonatype.plexus.build.incremental.DefaultBuildContext;
 
 public abstract class AbstractDependencyMojoTestCase extends 
AbstractMojoTestCase {
 
     protected File testDir;
-
     protected DependencyArtifactStubFactory stubFactory;
 
-    protected void setUp(String testDirectoryName, boolean createFiles) throws 
Exception {
-        setUp(testDirectoryName, createFiles, true);
-    }
-
-    protected void setUp(String testDirectoryName, boolean createFiles, 
boolean flattenedPath) throws Exception {
-        // required for mojo lookups to work
+    @Override
+    @Before
+    public void setUp() throws Exception {
+        // Initialize mojo lookups as required by AbstractMojoTestCase
         super.setUp();
 
+        // Create a unique temporary test directory to avoid parallel test 
conflicts
+        String uniqueDirName = "test-dependency" + UUID.randomUUID();
+        testDir = Files.createTempDirectory(uniqueDirName).toFile();
+        testDir.deleteOnExit();
+
+        // Initialize stub factory with default settings
+        stubFactory = new DependencyArtifactStubFactory(testDir, true, true);
+    }
+
+    /**
+     * Allows subclasses to customize the setup with specific test directory 
name and stub factory settings.
+     *
+     * @param testDirectoryName the name for the temporary test directory.
+     * @param createFiles whether to create files in the stub factory.
+     * @param flattenedPath whether to use flattened paths in the stub factory.
+     * @throws Exception If setup fails.
+     */
+    protected final void customizeSetUp(String testDirectoryName, boolean 
createFiles, boolean flattenedPath)

Review Comment:
   Probably setUp should be calling this method instead of duplicating this code



##########
src/test/java/org/apache/maven/plugins/dependency/AbstractDependencyMojoTestCase.java:
##########
@@ -35,33 +36,52 @@
 import org.eclipse.aether.RepositorySystemSession;
 import org.eclipse.aether.repository.LocalRepository;
 import org.eclipse.aether.repository.LocalRepositoryManager;
+import org.junit.Before;
 import org.sonatype.plexus.build.incremental.DefaultBuildContext;
 
 public abstract class AbstractDependencyMojoTestCase extends 
AbstractMojoTestCase {
 
     protected File testDir;
-
     protected DependencyArtifactStubFactory stubFactory;
 
-    protected void setUp(String testDirectoryName, boolean createFiles) throws 
Exception {
-        setUp(testDirectoryName, createFiles, true);
-    }
-
-    protected void setUp(String testDirectoryName, boolean createFiles, 
boolean flattenedPath) throws Exception {
-        // required for mojo lookups to work
+    @Override
+    @Before
+    public void setUp() throws Exception {
+        // Initialize mojo lookups as required by AbstractMojoTestCase
         super.setUp();
 
+        // Create a unique temporary test directory to avoid parallel test 
conflicts
+        String uniqueDirName = "test-dependency" + UUID.randomUUID();

Review Comment:
   I don't think you need UUIDs here. JUnit can create temporary directories 
with @TempDir. Or do we not have JUnit 5 here? Either way, let's makes sure we 
only create one temporary directory per test. 



##########
src/test/java/org/apache/maven/plugins/dependency/TestCollectMojo.java:
##########
@@ -29,13 +29,17 @@
 import org.apache.maven.plugins.dependency.utils.DependencySilentLog;
 import org.apache.maven.plugins.dependency.utils.DependencyStatusSets;
 import org.apache.maven.project.MavenProject;
+import org.junit.Before;
 
 public class TestCollectMojo extends AbstractDependencyMojoTestCase {
 
     @Override
-    protected void setUp() throws Exception {
-        // required for mojo lookups to work
-        super.setUp("markers", false);
+    @Before
+    public void setUp() throws Exception {
+        // Call superclass setup (initializes mojo lookups and default test 
directory)
+        super.setUp();
+        customizeSetUp("markers", false, true);

Review Comment:
   not needed, super.setUp should invoke this method which can be overridden 
here. 





> Clean up AbstractDependencyMojoTestCase setUp
> ---------------------------------------------
>
>                 Key: MDEP-979
>                 URL: https://issues.apache.org/jira/browse/MDEP-979
>             Project: Maven Dependency Plugin (Moved to GitHub Issues)
>          Issue Type: Task
>            Reporter: Elliotte Rusty Harold
>            Priority: Minor
>
> The setUp methods here are a mess and do not follow JUnit conventions. 
> Furthermore, subclasses do not always properly invoke super.setUp



--
This message was sent by Atlassian Jira
(v8.20.10#820010)

Reply via email to