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
--
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-unsubscr...@maven.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
