[ 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)