[
https://issues.apache.org/jira/browse/MDEP-979?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17954342#comment-17954342
]
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_r2109220394
##########
src/test/java/org/apache/maven/plugins/dependency/AbstractDependencyMojoTestCase.java:
##########
@@ -35,26 +35,49 @@
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);
+ @Before
+ public void setUp() throws Exception {
+ // Initialize mojo lookups as required by AbstractMojoTestCase
+ super.setUp();
+
+ // Create temporary test directory
+ testDir = Files.createTempDirectory("test-dependency").toFile();
+ testDir.deleteOnExit();
+
+ // Initialize stub factory with default settings
+ stubFactory = new DependencyArtifactStubFactory(testDir, true, true);
}
- protected void setUp(String testDirectoryName, boolean createFiles,
boolean flattenedPath) throws Exception {
- // required for mojo lookups to work
- super.setUp();
+ /**
+ * 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:
the name for the temporary test directory
##########
src/test/java/org/apache/maven/plugins/dependency/AbstractDependencyMojoTestCase.java:
##########
@@ -35,26 +35,49 @@
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);
+ @Before
+ public void setUp() throws Exception {
+ // Initialize mojo lookups as required by AbstractMojoTestCase
+ super.setUp();
+
+ // Create temporary test directory
+ testDir = Files.createTempDirectory("test-dependency").toFile();
+ testDir.deleteOnExit();
+
+ // Initialize stub factory with default settings
+ stubFactory = new DependencyArtifactStubFactory(testDir, true, true);
}
- protected void setUp(String testDirectoryName, boolean createFiles,
boolean flattenedPath) throws Exception {
- // required for mojo lookups to work
- super.setUp();
+ /**
+ * 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 void customizeSetUp(String testDirectoryName, boolean
createFiles, boolean flattenedPath)
+ throws Exception {
+ // Clean up existing test directory if present
+ if (testDir != null) {
+ FileUtils.deleteDirectory(testDir);
+ assertFalse(testDir.exists());
Review Comment:
probably don't need this assert. If you do need to check this, it should be
an exception, not an assert
##########
src/test/java/org/apache/maven/plugins/dependency/AbstractDependencyMojoTestCase.java:
##########
@@ -35,26 +35,49 @@
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);
+ @Before
+ public void setUp() throws Exception {
Review Comment:
@Override
##########
src/test/java/org/apache/maven/plugins/dependency/fromConfiguration/TestUnpackMojo.java:
##########
@@ -38,15 +38,19 @@
import
org.apache.maven.plugins.dependency.utils.markers.UnpackFileMarkerHandler;
import org.apache.maven.project.MavenProject;
import org.codehaus.plexus.archiver.manager.ArchiverManager;
+import org.junit.Before;
import static org.junit.Assert.assertNotEquals;
public class TestUnpackMojo extends AbstractDependencyMojoTestCase {
- UnpackMojo mojo;
+ private UnpackMojo mojo;
- protected void setUp() throws Exception {
- super.setUp("unpack", true, false);
+ @Before
+ public void setUp() throws Exception {
Review Comment:
@Override
##########
src/test/java/org/apache/maven/plugins/dependency/AbstractDependencyMojoTestCase.java:
##########
@@ -35,26 +35,49 @@
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);
+ @Before
+ public void setUp() throws Exception {
+ // Initialize mojo lookups as required by AbstractMojoTestCase
+ super.setUp();
+
+ // Create temporary test directory
+ testDir = Files.createTempDirectory("test-dependency").toFile();
+ testDir.deleteOnExit();
+
+ // Initialize stub factory with default settings
+ stubFactory = new DependencyArtifactStubFactory(testDir, true, true);
}
- protected void setUp(String testDirectoryName, boolean createFiles,
boolean flattenedPath) throws Exception {
- // required for mojo lookups to work
- super.setUp();
+ /**
+ * 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 void customizeSetUp(String testDirectoryName, boolean
createFiles, boolean flattenedPath)
+ throws Exception {
+ // Clean up existing test directory if present
+ if (testDir != null) {
Review Comment:
This could be very wonky when tests are run in parallel. Tests should clean
up after themselves, not when the next test method is set up.
##########
src/test/java/org/apache/maven/plugins/dependency/fromDependencies/TestCopyDependenciesMojo2.java:
##########
@@ -46,15 +46,19 @@
import org.apache.maven.plugins.dependency.utils.ResolverUtil;
import org.apache.maven.project.MavenProject;
import org.eclipse.aether.RepositorySystem;
+import org.junit.Before;
public class TestCopyDependenciesMojo2 extends AbstractDependencyMojoTestCase {
private CopyDependenciesMojo mojo;
- @Override
- protected void setUp() throws Exception {
- // required for mojo lookups to work
- super.setUp("copy-dependencies", true);
+ @Before
+ public void setUp() throws Exception {
Review Comment:
@Override
##########
src/test/java/org/apache/maven/plugins/dependency/TestSkip.java:
##########
@@ -37,9 +38,12 @@
public class TestSkip extends AbstractDependencyMojoTestCase {
- @Override
- protected void setUp() throws Exception {
+ @Before
+ public void setUp() throws Exception {
Review Comment:
@Override
##########
src/test/java/org/apache/maven/plugins/dependency/fromDependencies/TestIncludeExcludeUnpackDependenciesMojo.java:
##########
@@ -38,9 +39,11 @@ public class TestIncludeExcludeUnpackDependenciesMojo
extends AbstractDependency
private UnpackDependenciesMojo mojo;
- protected void setUp() throws Exception {
- // required for mojo lookups to work
- super.setUp("unpack-dependencies", true);
+ @Before
+ public void setUp() throws Exception {
Review Comment:
@Override
##########
src/test/java/org/apache/maven/plugins/dependency/fromConfiguration/TestArtifactItem.java:
##########
@@ -22,11 +22,15 @@
import org.apache.maven.artifact.Artifact;
import org.apache.maven.plugins.dependency.AbstractDependencyMojoTestCase;
+import org.junit.Before;
public class TestArtifactItem extends AbstractDependencyMojoTestCase {
- protected void setUp() throws Exception {
- setUp("artifactItems", false);
+ @Before
+ public void setUp() throws Exception {
Review Comment:
@Override
##########
src/test/java/org/apache/maven/plugins/dependency/AbstractDependencyMojoTestCase.java:
##########
@@ -35,26 +35,49 @@
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);
+ @Before
+ public void setUp() throws Exception {
+ // Initialize mojo lookups as required by AbstractMojoTestCase
+ super.setUp();
+
+ // Create temporary test directory
+ testDir = Files.createTempDirectory("test-dependency").toFile();
+ testDir.deleteOnExit();
+
+ // Initialize stub factory with default settings
+ stubFactory = new DependencyArtifactStubFactory(testDir, true, true);
}
- protected void setUp(String testDirectoryName, boolean createFiles,
boolean flattenedPath) throws Exception {
- // required for mojo lookups to work
- super.setUp();
+ /**
+ * 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.
Review Comment:
whether to use flattened paths in the stub factory
##########
src/test/java/org/apache/maven/plugins/dependency/utils/TestDependencyStatusSets.java:
##########
@@ -19,12 +19,15 @@
package org.apache.maven.plugins.dependency.utils;
import org.apache.maven.plugins.dependency.AbstractDependencyMojoTestCase;
+import org.junit.Before;
public class TestDependencyStatusSets extends AbstractDependencyMojoTestCase {
- protected void setUp() throws Exception {
- // required for mojo lookups to work
- super.setUp("dss", true);
+ @Before
+ public void setUp() throws Exception {
Review Comment:
@Override
##########
src/test/java/org/apache/maven/plugins/dependency/AbstractDependencyMojoTestCase.java:
##########
@@ -35,26 +35,49 @@
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);
+ @Before
+ public void setUp() throws Exception {
+ // Initialize mojo lookups as required by AbstractMojoTestCase
+ super.setUp();
+
+ // Create temporary test directory
+ testDir = Files.createTempDirectory("test-dependency").toFile();
+ testDir.deleteOnExit();
+
+ // Initialize stub factory with default settings
+ stubFactory = new DependencyArtifactStubFactory(testDir, true, true);
}
- protected void setUp(String testDirectoryName, boolean createFiles,
boolean flattenedPath) throws Exception {
- // required for mojo lookups to work
- super.setUp();
+ /**
+ * 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 void customizeSetUp(String testDirectoryName, boolean
createFiles, boolean flattenedPath)
Review Comment:
Is this meant to be invoked or overridden? If the former, then make it
final. But consider calling this from setUp and letting subclasses override it
instead.
##########
src/test/java/org/apache/maven/plugins/dependency/utils/translators/TestClassifierTypeTranslator.java:
##########
@@ -48,9 +49,11 @@ public class TestClassifierTypeTranslator extends
AbstractDependencyMojoTestCase
private ArtifactHandlerManager artifactHandlerManager;
- @Override
- protected void setUp() throws Exception {
- super.setUp("classifiertype-translator", false);
+ @Before
+ public void setUp() throws Exception {
Review Comment:
@Override
##########
src/test/java/org/apache/maven/plugins/dependency/fromDependencies/TestUnpackDependenciesMojo.java:
##########
@@ -48,11 +49,13 @@ public class TestUnpackDependenciesMojo extends
AbstractDependencyMojoTestCase {
private static final String UNPACKABLE_FILE_PATH =
"target/test-classes/unit/unpack-dependencies-test/" +
UNPACKABLE_FILE;
- UnpackDependenciesMojo mojo;
+ private UnpackDependenciesMojo mojo;
- protected void setUp() throws Exception {
- // required for mojo lookups to work
- super.setUp("unpack-dependencies", true, false);
+ @Before
+ public void setUp() throws Exception {
Review Comment:
@Override
##########
src/test/java/org/apache/maven/plugins/dependency/TestGetMojo.java:
##########
@@ -43,13 +43,17 @@
import org.eclipse.jetty.server.handler.ContextHandler;
import org.eclipse.jetty.server.handler.ResourceHandler;
import org.eclipse.jetty.util.security.Constraint;
+import org.junit.Before;
public class TestGetMojo extends AbstractDependencyMojoTestCase {
private GetMojo mojo;
- protected void setUp() throws Exception {
- // required for mojo lookups to work
- super.setUp("markers", false);
+ @Before
+ public void setUp() throws Exception {
Review Comment:
should this have an @Override?
##########
src/test/java/org/apache/maven/plugins/dependency/AbstractDependencyMojoTestCase.java:
##########
@@ -35,26 +35,49 @@
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);
+ @Before
+ public void setUp() throws Exception {
+ // Initialize mojo lookups as required by AbstractMojoTestCase
+ super.setUp();
+
+ // Create temporary test directory
+ testDir = Files.createTempDirectory("test-dependency").toFile();
+ testDir.deleteOnExit();
+
+ // Initialize stub factory with default settings
+ stubFactory = new DependencyArtifactStubFactory(testDir, true, true);
}
- protected void setUp(String testDirectoryName, boolean createFiles,
boolean flattenedPath) throws Exception {
- // required for mojo lookups to work
- super.setUp();
+ /**
+ * 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.
Review Comment:
whether to create files in the stub factory
##########
src/test/java/org/apache/maven/plugins/dependency/fromConfiguration/TestIncludeExcludeUnpackMojo.java:
##########
@@ -43,9 +44,11 @@ public class TestIncludeExcludeUnpackMojo extends
AbstractDependencyMojoTestCase
private UnpackMojo mojo;
- protected void setUp() throws Exception {
- // required for mojo lookups to work
- super.setUp("unpack", true, false);
+ @Before
+ public void setUp() throws Exception {
Review Comment:
@Override
> 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)
