gnodet-bot commented on code in PR #13546:
URL: https://github.com/apache/maven/pull/13546#discussion_r4299636551
##########
api/maven-api-core/src/main/java/org/apache/maven/api/Constants.java:
##########
@@ -880,5 +880,22 @@ public final class Constants {
@Config(type = "java.lang.Boolean", defaultValue = "false", source =
Config.Source.SYSTEM_PROPERTIES)
public static final String MAVEN_TEMPFILE_KEEP = "maven.tempfile.keep";
+ /**
+ * System property to disable the {@code site} lifecycle.
+ * When set to {@code true} via {@code
-Dmaven.site.lifecycle.disabled=true}, the {@code site}
+ * lifecycle is excluded from the default lifecycles, so Maven will not
resolve or bind the
+ * {@code maven-site-plugin} unless it is explicitly configured in the POM.
+ * This is a JVM system property only (read via {@link
Boolean#getBoolean(String)});
+ * it is not resolved through Maven's property mechanism (pom.xml,
profiles, settings.xml).
+ *
+ * @since 4.0.0
Review Comment:
⚠️ **Wrong `@since` tag** — still `4.0.0`, not fixed from the previous
review. This constant is introduced in 4.1.0. `MAVEN_TEMPFILE_KEEP` immediately
above and every other recently added constant use `@since 4.1.0`.
```suggestion
* @since 4.1.0
```
##########
impl/maven-core/src/test/java/org/apache/maven/internal/impl/DefaultLifecycleRegistryTest.java:
##########
@@ -18,14 +18,41 @@
*/
package org.apache.maven.internal.impl;
+import java.util.List;
+
+import org.apache.maven.api.Constants;
+import org.apache.maven.api.Lifecycle;
import org.junit.jupiter.api.Test;
+import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertNull;
+import static org.junit.jupiter.api.Assertions.assertTrue;
class DefaultLifecycleRegistryTest {
@Test
void returnsNullImplementationVersionWhenClassHasNoPackage() {
assertNull(DefaultLifecycleRegistry.getImplementationVersion(int.class));
}
+
+ /**
+ * Verifies that the site-lifecycle filtering constant reads from the
correct system property.
+ * The actual site-disabled path requires a forked JVM with the property
set at startup
+ * (static final field), so we verify correctness of the property name
used.
+ */
+ @Test
+ void siteDisabledPropertyNameMatchesConstant() throws Exception {
+ java.lang.reflect.Field field =
DefaultLifecycleRegistry.class.getDeclaredField("SITE_DISABLED");
+ field.setAccessible(true);
+ // SITE_DISABLED reads Constants.MAVEN_SITE_LIFECYCLE_DISABLED at
class-load time.
+ // We verify: when the property is not set, SITE_DISABLED is false,
+ // and the site lifecycle IS present in the default stream.
+ boolean siteDisabled = (boolean) field.get(null);
+ assertNull(
+ System.getProperty(Constants.MAVEN_SITE_LIFECYCLE_DISABLED),
+ "System property must not be set in this test JVM");
+ assertFalse(siteDisabled, "SITE_DISABLED should be false when property
is not set");
+ DefaultLifecycleRegistry registry = new
DefaultLifecycleRegistry(List.of());
+ assertTrue(registry.lookup("site").isPresent(), "site lifecycle
present when SITE_DISABLED=false");
+ }
Review Comment:
💡 **Reflection on private field is fragile** —
`getDeclaredField("SITE_DISABLED")` pins the test to the private field name.
Renaming the field compiles fine but breaks this test at runtime with
`NoSuchFieldException`. The test already exercises the observable behavior
(`registry.lookup("site").isPresent()`) — the reflection adds no additional
coverage. Drop the reflection entirely and test only the behavior:
```suggestion
void siteDisabledPropertyNameMatchesConstant() {
// SITE_DISABLED is read at class-load time from
Constants.MAVEN_SITE_LIFECYCLE_DISABLED.
// When the property is not set, the site lifecycle must be present.
assertNull(
System.getProperty(Constants.MAVEN_SITE_LIFECYCLE_DISABLED),
"System property must not be set in this test JVM");
DefaultLifecycleRegistry registry = new
DefaultLifecycleRegistry(List.of());
assertTrue(registry.lookup("site").isPresent(), "site lifecycle
present when SITE_DISABLED=false");
}
```
--
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]
