gnodet-bot commented on code in PR #13112:
URL: https://github.com/apache/maven/pull/13112#discussion_r4026714463
##########
compat/maven-model-builder/src/test/java/org/apache/maven/model/building/ExternalModelProfileActivationTest.java:
##########
@@ -85,39 +151,148 @@ class ExternalModelProfileActivationTest {
+ " </profiles>\n"
+ "</project>\n";
- private Model build(int validationLevel) throws Exception {
+ private Model build(int validationLevel, Properties systemProperties,
Properties userProperties) throws Exception {
ModelBuilder builder = new DefaultModelBuilderFactory().newInstance();
- Properties systemProperties = new Properties();
- systemProperties.putAll(System.getProperties());
- systemProperties.setProperty("some.dir",
System.getProperty("java.io.tmpdir"));
- systemProperties.setProperty("some.gating.property", "true");
-
DefaultModelBuildingRequest request = new
DefaultModelBuildingRequest();
request.setModelSource(new StringModelSource(POM));
request.setValidationLevel(validationLevel);
request.setSystemProperties(systemProperties);
+ request.setUserProperties(userProperties);
+ request.setModelResolver(new
DefaultModelBuilderTest.BaseModelResolver());
return builder.build(request).getEffectiveModel();
}
+ private Properties systemPropertiesWithTestValues() {
+ Properties sp = new Properties();
+ sp.putAll(System.getProperties());
+ sp.setProperty("some.dir", System.getProperty("java.io.tmpdir"));
+ sp.setProperty("sys.gating.prop", "true");
+ return sp;
+ }
+
+ private Properties userPropertiesWithTestValues() {
+ Properties up = new Properties();
+ up.setProperty("user.gating.prop", "true");
+ return up;
+ }
+
@Test
void testProjectBuildEvaluatesAllActivators() throws Exception {
- Model model = build(ModelBuildingRequest.VALIDATION_LEVEL_STRICT);
+ Model model = build(
+ ModelBuildingRequest.VALIDATION_LEVEL_STRICT,
+ systemPropertiesWithTestValues(),
+ userPropertiesWithTestValues());
+
+ assertEquals("activated", model.getProperties().get("profile.file"),
"file profile must fire in project build");
+ assertEquals(
+ "activated",
+ model.getProperties().get("profile.user.property"),
+ "user-property profile must fire in project build");
+ assertEquals(
+ "activated",
+ model.getProperties().get("profile.sys.property"),
+ "system-property profile must fire in project build");
+ // POM-declared property activation is only supported in external
(sandbox) builds.
+ // In project builds, the PropertyProfileActivator does not check
model properties
+ // because doing so would cause unintended profile activation when a
POM declares
+ // a property that also matches a profile's activation condition (see
IT proxy profile).
+ assertNull(
+ model.getProperties().get("profile.pom.property"),
+ "POM-declared-property profile must NOT fire in project build
(only in external)");
+ assertEquals(
+ "activated",
+ model.getProperties().get("profile.negated.property"),
+ "negated-property profile must fire in project build");
+ assertEquals("activated", model.getProperties().get("profile.jdk"),
"JDK profile must fire in project build");
+ assertTrue(
+ model.getRepositories().stream().anyMatch(r ->
"profile-repo".equals(r.getId())),
+ "profile repository must be present in project build");
+ }
+
+ @Test
+ void testExternalModelFileActivationIsSuppressed() throws Exception {
+ // File activation must be suppressed in external model builds
regardless of whether the
+ // path exists: publisher-local paths don't exist on the consumer's
machine.
+ Model model = build(
+ ModelBuildingRequest.VALIDATION_LEVEL_MINIMAL,
+ systemPropertiesWithTestValues(),
+ userPropertiesWithTestValues());
+
+ assertNull(model.getProperties().get("profile.file"), "file profile
must NOT fire in external model build");
+ }
+
+ @Test
+ void testExternalModelUserPropertyActivationIsSuppressed() throws
Exception {
+ // Consumer -D flags must not activate dependency profiles.
+ Model model = build(
+ ModelBuildingRequest.VALIDATION_LEVEL_MINIMAL,
+ systemPropertiesWithTestValues(),
+ userPropertiesWithTestValues());
+
+ assertNull(
+ model.getProperties().get("profile.user.property"),
+ "user-property profile must NOT fire in external model build
(consumer -D suppressed)");
+ }
+
+ @Test
+ void testExternalModelSystemPropertyActivationIsPreserved() throws
Exception {
+ // System properties are platform facts and must drive activation even
in external builds.
+ Model model = build(
+ ModelBuildingRequest.VALIDATION_LEVEL_MINIMAL,
systemPropertiesWithTestValues(), new Properties());
+
+ assertEquals(
+ "activated",
+ model.getProperties().get("profile.sys.property"),
+ "system-property profile must fire in external model build");
+ }
+
+ @Test
+ void testExternalModelPomDeclaredPropertyActivationIsPreserved() throws
Exception {
+ // A profile conditioned on the POM's own <properties> must fire in
external builds:
+ // those properties are part of the artifact's published identity, not
the consumer's env.
+ Model model = build(
+ ModelBuildingRequest.VALIDATION_LEVEL_MINIMAL,
systemPropertiesWithTestValues(), new Properties());
+
+ assertEquals(
+ "activated",
+ model.getProperties().get("profile.pom.property"),
+ "POM-declared-property profile must fire in external model
build");
+ }
+
+ @Test
+ void testExternalModelNegatedPropertyDefaultOnIsPreserved() throws
Exception {
+ // A negated-property profile (!foo) fires when the property is absent.
+ // In the sandbox, user properties are suppressed so 'skip.defaults'
is absent → fires.
+ // This is the common "opt-out flag" pattern (e.g. resteasy-default in
JBoss projects).
+ Model model = build(
+ ModelBuildingRequest.VALIDATION_LEVEL_MINIMAL,
systemPropertiesWithTestValues(), new Properties());
+
+ assertEquals(
+ "activated",
+ model.getProperties().get("profile.negated.property"),
+ "negated-property (default-on) profile must fire in external
model build");
+ }
Review Comment:
⚠️ **Missing symmetric test (parallel-path gap with impl stack).**
`testExternalModelSystemPropertySuppressesDefaultOnProfile` was just added
to `impl/maven-impl/DefaultModelBuilderTest` (lines 204–225 of that file) to
prove that a consumer setting `skip.defaults` as a **system** property
suppresses the `!skip.defaults` default-on profile even in external builds. The
compat stack has the same sandbox behavior and the same `!skip.defaults`
profile in its POM fixture, but `ExternalModelProfileActivationTest` only
covers the "fires when absent" direction — the complementary case is not tested
here.
Add the symmetric test after this method:
```java
@Test
void testExternalModelSystemPropertySuppressesDefaultOnProfile() throws
Exception {
// A system property named skip.defaults suppresses the default-on
negated-property
// profile (!skip.defaults) even in external model builds. System
properties are platform
// facts that pass through the sandbox unchanged, so the publisher's
opt-out mechanism
// still works when the consumer sets it at the JVM level.
Properties sp = systemPropertiesWithTestValues();
sp.setProperty("skip.defaults", "true"); // system-level opt-out
Model model = build(ModelBuildingRequest.VALIDATION_LEVEL_MINIMAL,
sp, new Properties());
assertNull(
model.getProperties().get("profile.negated.property"),
"negated-property (default-on) profile must be suppressed
when system property 'skip.defaults' is set");
}
```
--
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]
