gnodet-bot commented on code in PR #13249: URL: https://github.com/apache/maven/pull/13249#discussion_r4296193174
########## its/core-it-suite/src/test/java/org/apache/maven/it/MavenITmng13249ReactorXmlPhaseInjectionTest.java: ########## @@ -0,0 +1,129 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.maven.it; + +import java.nio.file.Path; +import java.util.List; + +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.assertTrue; + +/** + * Verifies that custom lifecycle phases defined in {@code .mvn/reactor.xml} are injected + * into the lifecycle DAG and that plugin goals bound to those phases execute correctly. + * + * @see <a href="https://github.com/apache/maven/pull/13249";>#13249</a> + */ +class MavenITmng13249ReactorXmlPhaseInjectionTest extends AbstractMavenIntegrationTestCase { + + /** + * A plugin goal bound to a reactor.xml-injected phase must execute when running + * through that phase (via {@code verify}). + */ + @Test + void pluginBoundToInjectedPhaseExecutesDuringVerify() throws Exception { + Path testDir = extractResources("/mng-13249-reactor-xml-phase-injection"); + + Verifier verifier = newVerifier(testDir); + verifier.setAutoclean(false); + verifier.deleteDirectory("target"); + verifier.addCliArgument("verify"); + verifier.execute(); + verifier.verifyErrorFreeLog(); + + verifier.verifyFilePresent("target/pre-integration-marker.txt"); + } + + /** + * The injected phase can be used as a direct CLI target. + */ + @Test + void injectedPhaseCanBeUsedAsCliTarget() throws Exception { + Path testDir = extractResources("/mng-13249-reactor-xml-phase-injection"); + + Verifier verifier = newVerifier(testDir); + verifier.setAutoclean(false); + verifier.deleteDirectory("target"); + verifier.addCliArgument("pre-integration"); + verifier.execute(); + verifier.verifyErrorFreeLog(); + + verifier.verifyFilePresent("target/pre-integration-marker.txt"); + } + + /** + * The injected phase respects ordering: running only up to {@code compile} + * (which comes before {@code pre-integration}) must NOT trigger the plugin. + */ + @Test + void pluginDoesNotRunBeforeInjectedPhase() throws Exception { + Path testDir = extractResources("/mng-13249-reactor-xml-phase-injection"); + + Verifier verifier = newVerifier(testDir); + verifier.setAutoclean(false); + verifier.deleteDirectory("target"); + verifier.addCliArgument("compile"); + verifier.execute(); + verifier.verifyErrorFreeLog(); + + verifier.verifyFileNotPresent("target/pre-integration-marker.txt"); + } + + /** + * Verify the injected phase runs in the correct position: after unit-test + * related phases but before integration-test phases. We check log ordering + * by looking for the execution banner lines ({@code [INFO] ---}) which only + * appear at actual phase-execution time, not at plan-calculation time. + */ + @Test + void injectedPhaseRunsInCorrectOrder() throws Exception { + Path testDir = extractResources("/mng-13249-reactor-xml-phase-injection"); + + Verifier verifier = newVerifier(testDir); + verifier.setAutoclean(false); + verifier.deleteDirectory("target"); + verifier.addCliArgument("verify"); + verifier.execute(); + verifier.verifyErrorFreeLog(); + + List<String> logLines = verifier.loadLogLines(); + + int touchIdx = -1; + int compileIdx = -1; + for (int i = 0; i < logLines.size(); i++) { + String line = logLines.get(i); + if (line.contains("compiler") && line.contains(":compile")) { Review Comment: **Javadoc/code mismatch — and the pattern is still overly broad** The method Javadoc (line 92) says _"looking for the execution banner lines `[INFO] ---` which only appear at actual phase-execution time, not at plan-calculation time"_ — but the implementation dropped `[INFO] ---` from the check. The Javadoc describes the original design intent (anchored to the execution banner) while the code reverted to a broad substring scan. The new pattern `line.contains("compiler") && line.contains(":compile")` will match the Maven 4 execution banner `[INFO] --- compiler:3.16.0:compile (default-compile) @ project ---`, which is correct. But it will also match any DEBUG or INFO planning line that includes the goal coordinates (e.g. `[DEBUG] Resolving plugin compiler:3.16.0:compile`), potentially capturing a line earlier in the log than the actual execution — which is the exact fragility the original review flagged. Fix: add `[INFO] ---` back as an anchor while keeping the Maven 4 format: ```suggestion if (line.contains("[INFO] ---") && line.contains("compiler") && line.contains(":compile")) { ``` Then update the Javadoc on line 92 to describe what the code actually does — it already describes this correctly, so no Javadoc change is needed once the code matches. -- 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]
