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