[#12288] Backport ITs for settings.xml profile -> LRM property propagation#428
[#12288] Backport ITs for settings.xml profile -> LRM property propagation#428ascheman wants to merge 2 commits into
Conversation
…ation
Ports the integration tests added on apache/maven master via #12297 and
#12298 to the apache/maven-integration-testing maven-3.10.x branch.
Covers three settings.xml-only activation channels:
* testActiveByDefaultProfile -- guards <activation><activeByDefault>
* testActiveProfilesList -- guards <activeProfiles><activeProfile>
(already worked on 3.x, regression guard)
* testActiveByDefaultDeactivatedViaCli -- guards that -P !profileId still
deactivates an activeByDefault profile
Adaptations from master:
* java.io.File API instead of java.nio.file.Path
* Verifier from org.apache.maven.shared.verifier (3.x package)
* ResourceExtractor.simpleExtractResources for fixture extraction
* JUnit 4-style assertions inherited from AbstractMavenIntegrationTestCase
* Resource directory uses 3.x gh-NNNN convention
* Version constraint super("[3.10.0-SNAPSHOT,)") so the IT only runs once
the backport (apache/maven PR #12333) lands.
Master origin commits:
* 7725eaf25ab2715b26848fa56d3f0416862e83cb (initial 2 ITs)
* 578523519a33a78a651c472bb07d312e01b1e521 (rename + activeByDefault fix)
* 22f9a0ee086a464f2bfa757ecd0a32053f3d1e8d (squashed merge of #12298,
adds testActiveByDefaultDeactivatedViaCli)
Co-Authored-By: Guillaume Nodet <gnodet@gmail.com>
gnodet
left a comment
There was a problem hiding this comment.
Fully automatic review from Claude Code
Nice work on the backport — the three tests are well-structured and correctly cover the activeByDefault, activeProfiles list, and CLI deactivation channels. Two items to address before merging:
1. Spotless formatting violations (CI failing)
All Java 11/17/21 CI builds fail with Spotless violations. Running mvn spotless:apply should fix them.
2. Missing TestSuiteOrdering registration
MavenITgh12288SettingsProfileAetherPropertiesTest is not registered in TestSuiteOrdering.java. Both existing gh- prefixed tests (MavenITgh10312…, MavenITgh10937…) are registered there, and the ordering enforcer will warn for every unregistered test.
This review was generated by an AI agent and may contain inaccuracies. Please verify all suggestions before applying.
|
|
||
| Verifier verifier = newVerifier(testDir.getAbsolutePath()); | ||
| verifier.setAutoclean(false); | ||
| verifier.deleteDirectory("target"); |
There was a problem hiding this comment.
Spotless expects different line wrapping here. CI shows the expected format is:
| verifier.deleteDirectory("target"); | |
| File testDir = | |
| ResourceExtractor.simpleExtractResources(getClass(), "/gh-12288-settings-profile-aether-properties"); |
Running mvn spotless:apply will fix this and the other violation automatically.
|
Claude Code on behalf of Guillaume Nodet Here's a patch for the two issues flagged in the review. You can apply it with Patch: Fix Spotless formatting and add TestSuiteOrdering registrationdiff --git a/core-it-suite/src/test/java/org/apache/maven/it/MavenITgh12288SettingsProfileAetherPropertiesTest.java b/core-it-suite/src/test/java/org/apache/maven/it/MavenITgh12288SettingsProfileAetherPropertiesTest.java
index c262829e5..50471617f 100644
--- a/core-it-suite/src/test/java/org/apache/maven/it/MavenITgh12288SettingsProfileAetherPropertiesTest.java
+++ b/core-it-suite/src/test/java/org/apache/maven/it/MavenITgh12288SettingsProfileAetherPropertiesTest.java
@@ -77,8 +77,8 @@ public class MavenITgh12288SettingsProfileAetherPropertiesTest extends AbstractM
@Test
public void testActiveByDefaultDeactivatedViaCli() throws Exception {
- File testDir = ResourceExtractor.simpleExtractResources(
- getClass(), "/gh-12288-settings-profile-aether-properties");
+ File testDir =
+ ResourceExtractor.simpleExtractResources(getClass(), "/gh-12288-settings-profile-aether-properties");
Verifier verifier = newVerifier(testDir.getAbsolutePath());
verifier.setAutoclean(false);
@@ -111,8 +111,8 @@ public class MavenITgh12288SettingsProfileAetherPropertiesTest extends AbstractM
}
private void runAndAssertCustomPrefix(String settingsFile) throws Exception {
- File testDir = ResourceExtractor.simpleExtractResources(
- getClass(), "/gh-12288-settings-profile-aether-properties");
+ File testDir =
+ ResourceExtractor.simpleExtractResources(getClass(), "/gh-12288-settings-profile-aether-properties");
Verifier verifier = newVerifier(testDir.getAbsolutePath());
verifier.setAutoclean(false);
diff --git a/core-it-suite/src/test/java/org/apache/maven/it/TestSuiteOrdering.java b/core-it-suite/src/test/java/org/apache/maven/it/TestSuiteOrdering.java
index c9bd8e395..8acd0b027 100644
--- a/core-it-suite/src/test/java/org/apache/maven/it/TestSuiteOrdering.java
+++ b/core-it-suite/src/test/java/org/apache/maven/it/TestSuiteOrdering.java
@@ -121,6 +121,7 @@ public class TestSuiteOrdering implements ClassOrderer {
* a fail fast technique as well.
*/
+ suite.addTestSuite(MavenITgh12288SettingsProfileAetherPropertiesTest.class);
suite.addTestSuite(MavenITgh10312TerminallyDeprecatedMethodInGuiceTest.class);
suite.addTestSuite(MavenITgh10937QuotedPipesInMavenOptsTest.class);
suite.addTestSuite(MavenITmng8106OverlappingDirectoryRolesTest.class);Alternatively, run |
…teOrdering Two CI/review items flagged by gnodet on apache#428: * Spotless formatting violations in MavenITgh12288SettingsProfileAetherPropertiesTest (line wrapping) and the fixture pom.xml (description block). * Missing TestSuiteOrdering.addTestSuite(...) registration so the ordering enforcer recognizes the new IT. Co-Authored-By: Guillaume Nodet <gnodet@gmail.com>
@gnodet — for review. Companion to apache/maven#12333 (which backports
the core fix from #12297 / #12298 to the maven-3.10.x line).
What this PR adds
Three integration tests in
MavenITgh12288SettingsProfileAetherPropertiesTest:testActiveByDefaultProfile— guards that a settings.xml profileactivated via
<activation><activeByDefault>true</activeByDefault>has its
<properties>(notablyaether.*) propagated to the resolversession config in time for LRM init.
testActiveProfilesList— guards the<activeProfiles><activeProfile>...</activeProfile></activeProfiles>channel that already worked on 3.x. Acts as a regression guard so the
channel keeps working after the activeByDefault fix.
testActiveByDefaultDeactivatedViaCli— guards that-P !profileIdstill deactivates an
<activeByDefault>profile after the fix, so its<properties>are NOT merged into the resolver session config.All three tests use the same fixture artifact
(
org.apache.maven.its.settings.profile.aether:test-artifact:1.0:pom)and assert the install location under
<localRepo>/it-custom-prefix/...— the placeaether.enhancedLocalRepository.localPrefixwould land it iff theproperties reach the session config.
Adaptations from master
Equivalent ITs on apache/maven master were merged via #12297 and #12298.
Ported with the following 3.x conventions:
java.io.FileAPI instead ofjava.nio.file.PathVerifierfromorg.apache.maven.shared.verifier(3.x package)ResourceExtractor.simpleExtractResourcesfor fixture extractionAbstractMavenIntegrationTestCasegh-NNNNnaming conventionsuper("[3.10.0-SNAPSHOT,)")so the IT only runs once the corebackport ([#12288] Backport: settings.xml activeByDefault profile props to LRM maven#12333) lands
CI behaviour
Until apache/maven#12333 is merged and a Maven 3.10.0 build with the fix
is what the IT-suite runs against, the IT will be skipped by the
version-range constraint — green-by-skip is the expected state for now.
Master origin commits
7725eaf25ab2715b26848fa56d3f0416862e83cb— initial 2 ITs578523519a33a78a651c472bb07d312e01b1e521— rename + review hardening22f9a0ee086a464f2bfa757ecd0a32053f3d1e8d— squashed merge of #12298 (addedtestActiveByDefaultDeactivatedViaCli)References apache/maven#12288, apache/maven#12333.