Copilot commented on code in PR #313:
URL:
https://github.com/apache/maven-remote-resources-plugin/pull/313#discussion_r4287837300
##########
src/test/java/org/apache/maven/plugin/resources/remote/RemoteResourcesMojoTest.java:
##########
@@ -346,69 +390,36 @@ private static void assertContainsInOrder(String actual,
List<String> expected)
int fromIndex = 0;
for (String s : expected) {
int idx = actual.indexOf(s, fromIndex);
- assertTrue("expected \"" + s + "\" to occur in order in: " +
actual, idx >= 0);
+ assertTrue(idx >= 0, "expected \"" + s + "\" to occur in order in:
" + actual);
fromIndex = idx + s.length();
}
}
- protected MavenProjectResourcesStub createTestProject(final String
testName) throws Exception {
+ private MavenProjectResourcesStub createTestProject(final String testName)
throws Exception {
// this will automatically create the isolated
// test environment
return new MavenProjectResourcesStub(testName);
}
- protected void setupDefaultProject(final MavenProjectResourcesStub
project) throws Exception {
+ private void setupDefaultProject(final MavenProjectResourcesStub project)
throws Exception {
// put this on the root dir
project.addFile("pom.xml", MavenProjectBuildStub.ROOT_FILE);
project.setInceptionYear("2007");
// start creating the environment
project.setupBuildEnvironment();
}
- protected BundleRemoteResourcesMojo lookupBundleMojo() throws Exception {
- File pomFile = new File(getBasedir(), DEFAULT_BUNDLE_POM_PATH);
- BundleRemoteResourcesMojo mojo = (BundleRemoteResourcesMojo)
lookupMojo("bundle", pomFile);
-
- assertNotNull(mojo);
-
- return mojo;
- }
-
- protected BundleRemoteResourcesMojo
lookupBundleMojoWithDefaultSettings(final MavenProject project)
+ private void configureBundleMojo(
+ BundleRemoteResourcesMojo mojo, MavenProject project, File
resourceDir, String sourceEncoding)
throws Exception {
- File resourceDir = new File(project.getBasedir() +
"/src/main/resources");
- return lookupBundleMojoWithSettings(project, resourceDir, null);
- }
-
- protected BundleRemoteResourcesMojo lookupBundleMojoWithSettings(
- final MavenProject project, File resourceDir, String
sourceEncoding) throws Exception {
- final BundleRemoteResourcesMojo mojo = lookupBundleMojo();
-
setVariableValueToObject(mojo, "resourcesDirectory", resourceDir);
setVariableValueToObject(
mojo, "outputDirectory", new
File(project.getBuild().getOutputDirectory()));
setVariableValueToObject(mojo, "sourceEncoding", sourceEncoding);
- return mojo;
}
- protected ProcessRemoteResourcesMojo lookupProcessMojo() throws Exception {
- File pomFile = new File(getBasedir(), DEFAULT_PROCESS_POM_PATH);
- ProcessRemoteResourcesMojo mojo = (ProcessRemoteResourcesMojo)
lookupMojo("process", pomFile);
-
- assertNotNull(mojo);
-
- return mojo;
- }
-
- protected ProcessRemoteResourcesMojo lookupProcessMojoWithSettings(final
MavenProject project, String[] bundles)
+ private void configureProcessMojo(ProcessRemoteResourcesMojo mojo,
MavenProject project, ArrayList<String> bundles)
Review Comment:
This helper unnecessarily requires an `ArrayList<String>`; it only needs the
`List<String>` abstraction. Accepting `List<String>` makes the method easier to
call (e.g., with `Collections.emptyList()` or other `List` implementations) and
reduces coupling to a specific concrete type.
##########
src/test/java/org/apache/maven/plugin/resources/remote/RemoteResourcesMojoTest.java:
##########
@@ -292,12 +335,13 @@ public void testFilteredBundlesWithProjectProperties()
throws Exception {
assertTrue(data.contains("rules"));
}
- protected void buildResourceBundle(String id, String sourceEncoding,
String[] resourceNames, File jarName)
+ private void buildResourceBundle(
+ BundleRemoteResourcesMojo mojo, String id, String sourceEncoding,
String[] resourceNames, File jarName)
throws Exception {
final MavenProjectResourcesStub project = createTestProject(id);
final File resourceDir = new File(project.getBasedir() +
"/src/main/resources");
Review Comment:
Building paths via string concatenation is harder to read and less robust
than using `File`/`Path` joins. Consider using `new File(project.getBasedir(),
\"src/main/resources\")` (or `project.getBasedir().toPath().resolve(...)`) to
keep path construction consistent and avoid manual separators.
##########
src/test/java/org/apache/maven/plugin/resources/remote/RemoteResourcesMojoTest.java:
##########
@@ -44,54 +48,63 @@
import org.apache.maven.plugin.MojoExecutionException;
import org.apache.maven.plugin.resources.remote.stub.MavenProjectBuildStub;
import org.apache.maven.plugin.resources.remote.stub.MavenProjectResourcesStub;
-import org.apache.maven.plugin.testing.AbstractMojoTestCase;
import org.apache.maven.project.MavenProject;
import org.apache.maven.repository.internal.MavenRepositorySystemUtils;
+import org.codehaus.plexus.PlexusContainer;
import org.codehaus.plexus.util.FileUtils;
import org.codehaus.plexus.util.IOUtil;
import org.eclipse.aether.DefaultRepositorySystemSession;
import org.eclipse.aether.internal.impl.SimpleLocalRepositoryManagerFactory;
import org.eclipse.aether.repository.LocalRepository;
+import org.junit.jupiter.api.Test;
+
+import static
org.apache.maven.api.plugin.testing.MojoExtension.getVariableValueFromObject;
+import static
org.apache.maven.api.plugin.testing.MojoExtension.setVariableValueToObject;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+import static org.junit.jupiter.api.Assertions.fail;
/**
* RemoteResources plugin Test Case
*/
-public class RemoteResourcesMojoTest extends AbstractMojoTestCase {
+@MojoTest
+class RemoteResourcesMojoTest {
static final String DEFAULT_BUNDLE_POM_PATH =
"target/test-classes/unit/rrmojotest/bundle-plugin-config.xml";
static final String DEFAULT_PROCESS_POM_PATH =
"target/test-classes/unit/rrmojotest/process-plugin-config.xml";
private static final String LOCAL_REPO = "target/local-repo/";
Review Comment:
Using a fixed on-disk local repository path under `target/local-repo/` makes
the tests less isolated and can lead to cross-test contamination (and flakiness
on CI), especially if tests are ever re-ordered or run concurrently. Prefer
creating a per-test temp directory (e.g., via JUnit 5 `@TempDir`) and wiring
that path into the repository/session setup, or at least incorporate the test
id into the local repo path and ensure it is cleaned before use.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
