Copilot commented on code in PR #307:
URL:
https://github.com/apache/maven-remote-resources-plugin/pull/307#discussion_r4229725247
##########
src/main/java/org/apache/maven/plugin/resources/remote/AbstractProcessRemoteResourcesMojo.java:
##########
@@ -532,28 +532,30 @@ protected List<MavenProject> getProjects() {
getLog().debug("PROJECTS: " + artifacts);
for (Artifact artifact : artifacts) {
- if (artifact.isSnapshot()) {
- artifact.setVersion(artifact.getBaseVersion());
- }
+ MavenProject p = findReactorProject(artifact, mavenSession);
+ if (p == null) {
+ if (artifact.isSnapshot()) {
+ artifact.setVersion(artifact.getBaseVersion());
+ }
Review Comment:
This changes behavior for snapshot artifacts found in the reactor:
previously *all* snapshot artifacts had their version normalized to
`baseVersion` before downstream logic ran, but now that normalization only
happens when `p == null`. If later logic (e.g., supplement map key generation,
comparisons, or logging) assumes snapshots are normalized, reactor snapshots
may now use timestamped versions and fail to match expected keys/config.
Consider normalizing `artifact` to `baseVersion` regardless of reactor match
(e.g., normalize after `findReactorProject(...)` but before any key
generation/lookup), or ensure downstream keying uses `baseVersion` consistently.
##########
src/test/java/org/apache/maven/plugin/resources/remote/RemoteResourcesMojoTest.java:
##########
@@ -89,6 +89,36 @@ public void testNoBundles() throws Exception {
mojo.execute();
}
+ public void testReactorProjectIsReusedWithoutBuildingItsModel() throws
Exception {
+ MavenProject reactorProject = new MavenProject(new
org.apache.maven.model.Model());
+ reactorProject.setGroupId("com.example");
+ reactorProject.setArtifactId("reactor-project");
+ reactorProject.setVersion("1.0-SNAPSHOT");
Review Comment:
The test name asserts the model is not built, but the test only exercises
`findReactorProject(...)` directly and does not verify that
`projectBuilder.build(...)` is skipped in `getProjects()`. To cover the
performance regression this PR targets, add an assertion at the `getProjects()`
level (e.g., using a spy/mock `ProjectBuilder` and verifying no build occurs
when the artifact matches a reactor project), or rename the test to reflect
what it actually validates (reactor matching for timestamped snapshots).
--
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]
