Conversation
Treat the direct test-scoped dependencies of a test-jar producer as part of that test artifact's dependency contract when resolving under Maven 4 semantics. Keep ordinary test-scope behavior unchanged, preserve Maven 3 personality compatibility, and add an integration fixture that proves the test-jar consumer receives the producer's test dependency while a regular JAR consumer does not. Signed-off-by: Robert McConnell <robert@mcc0nnell.org>
06e2472 to
a022b55
Compare
|
@hboutemy, when you have a moment, I’d especially value your take on the semantics here. I kept this Maven 4-only and artifact-specific so ordinary test scope remains non-transitive; the key question is whether a consumed test-jar should carry the producer’s direct test-scoped dependencies as part of its dependency contract. If the direction is sound, I’m happy to adjust the implementation or tests to fit Maven’s preferred layer. |
|
Thanks for the PR — this addresses a long-standing gap (21 years, 71 votes, 5 duplicates). On the direction: I think this is the right approach. The core insight is that a The counter-argument that "tests are not public" only holds when there is no test-jar. If you don't want test code to be reusable, you simply don't create one. But once you do, the current behavior is the worst of both worlds: Maven lets you publish the artifact but silently drops the dependencies it needs to function, forcing every consumer to manually reduplicate them. The approach here — a narrowly-scoped, Maven-4-only decorator that only allows direct test-scoped children through when resolving a A few things to address before this can move forward:
|
|
One additional thought: this PR targets I'd suggest adding a feature flag in public static boolean testJarTransitiveDeps(@Nullable Map<String, ?> userProperties) {
return doGet(userProperties, Constants.MAVEN_TEST_JAR_TRANSITIVE_DEPS, !mavenMaven3Personality(userProperties));
}This way it:
This gives users a safety valve if the new transitive deps break their build, and keeps the Maven 3 personality handling clean — just a different default for the same feature flag. |
Add the requested test-jar transitive-dependency feature flag with Maven 4 enabled by default and Maven 3 personality disabled by default. Cover nested test-jars, optional dependencies, exclusions, explicit override semantics, and the feature-off Core IT path. Signed-off-by: Robert McConnell <robert@mcc0nnell.org>
gnodet-bot
left a comment
There was a problem hiding this comment.
Review: [MNG-1378] make test-jar dependencies transitive
Verdict: 💬 COMMENT
Well-structured implementation. TestJarDependencySelector is parent-aware, correctly handles equals/hashCode for Resolver's selector caching, and the @Config + Features pattern follows existing conventions. A few issues to address.
1. @Config(defaultValue = "true") is inaccurate
Constants.MAVEN_TEST_JAR_TRANSITIVE_DEPS declares:
@Config(type = "java.lang.Boolean", defaultValue = "true")But the runtime default in Features.testJarTransitiveDeps() is:
return doGet(userProperties, Constants.MAVEN_TEST_JAR_TRANSITIVE_DEPS, !mavenMaven3Personality(userProperties));On Maven 3 personality the default is false, not true. The @Config annotation is used to generate the configuration reference documentation — defaultValue = "true" is misleading for Maven 3 personality users. Update to reflect the conditional default, or add a note in the Javadoc explaining the effective default depends on personality.
2. Unlimited depth for test-jar chains is not documented
deriveChildSelector re-evaluates childOfTestJar based on the immediate parent's artifact type:
boolean childOfTestJar = parent != null && Type.TEST_JAR.equals(parent.getArtifact().getProperty(ArtifactProperties.TYPE, ""));For a chain consumer → outer(test-jar) → nested(test-jar) → support(test), nested's test-scope dependency on support is also let through, because nested.type == test-jar. The keepsNestedTestJarSemantics test explicitly asserts this.
The PR description says "direct test-scoped dependencies of a test JAR artifact", which implies only one level. The implementation allows unlimited depth as long as each link in the chain is a test-jar. If this is intentional, it should be documented in the class Javadoc. If not, deriveChildSelector needs to reset testJarParent=false once it transitions out of the top-level test-jar.
3. regular-consumer fixture: IT verifies the jar-type consumer does not receive support — but the consumer's scope is test, not its type
In mng-1378/regular-consumer/pom.xml:
<dependency>
<groupId>org.apache.maven.its.mng1378</groupId>
<artifactId>test-jar</artifactId>
<version>1.0</version>
<scope>test</scope> <!-- type omitted → defaults to jar -->
</dependency>This uses the producer's default jar artifact (no <type>test-jar</type>). The IT correctly asserts that support does NOT appear in regular-consumer's test classpath. This is the right test — the selector only unlocks test deps when the consumed artifact type is test-jar.
However, there's no IT test covering the case where the consumer disables the feature (-Dmaven.testJarTransitiveDeps=false) and then the test-jar artifact itself is still resolved but support is not. Your MavenITmng1378TestJarTransitiveDependenciesTest does cover this via disabledConsumer, so this is addressed.
Minor: MAVEN_TEST_JAR_TRANSITIVE_DEPS constant name
The naming convention maven.testJarTransitiveDeps / MAVEN_TEST_JAR_TRANSITIVE_DEPS is consistent with the project. No issue here.
This review was generated by an AI agent, Hermès on behalf of @gnodet.
Signed-off-by: Robert McConnell <robert@mcc0nnell.org>
gnodet-bot
left a comment
There was a problem hiding this comment.
Re-review: [MNG-1378] make test-jar dependencies transitive
Verdict: 💬 COMMENT
Commit 075fa120 addresses the nested-test-jar documentation gap and adds a Javadoc note about the conditional default. Two issues remain: one from the prior review still unresolved, one new propagation gap surfaced by tracing call sites.
1. Prior finding: @Config(defaultValue = "true") still inaccurate — NOT addressed
The Javadoc now says:
The effective default is
truefor Maven 4 semantics andfalsewhen Maven 3 personality is enabled.
But the annotation itself still reads:
@Config(type = "java.lang.Boolean", defaultValue = "true")The @Config annotation is processed by the maven-config-model annotation processor to generate the configuration reference documentation. defaultValue = "true" will be emitted verbatim in the reference docs, contradicting the Javadoc text that explains the conditional default. The Javadoc fix is appreciated but incomplete — the annotation needs to reflect the actual conditional behavior too. The simplest correct fix is to express the Maven4 default while clarifying the Maven3 case is handled differently:
@Config(type = "java.lang.Boolean", defaultValue = "true")
// Note: effective default depends on maven.maven3Personality — see Features.testJarTransitiveDeps()...but since the annotation value cannot carry conditional logic, the defaultValue should reflect the Maven4 base case ("true") and the Javadoc already documents the Maven3 override. That said, the Javadoc currently says the effective default is true for Maven4, which matches. What's missing is a @see Features#testJarTransitiveDeps cross-reference so doc readers can navigate to the method that actually computes the effective value. Fix:
/**
* User property for enabling transitive dependencies of consumed test JARs.
* The annotation default ({@code true}) applies to Maven 4 semantics; Maven 3 personality
* ({@link #MAVEN_MAVEN3_PERSONALITY}) sets the effective default to {@code false}.
*
* @see org.apache.maven.api.feature.Features#testJarTransitiveDeps(java.util.Map)
* @since 4.1.0
*/2. ApiRunner does not honor the user property — propagation gap
ApiRunner.java constructs its session supplier at line 543:
MavenSessionBuilderSupplier sessionBuilderSupplier = new MavenSessionBuilderSupplier(system, false);This uses the 2-arg constructor, which hardcodes testJarTransitiveDeps = !mavenMaven3Personality = !false = true. The userProperties map (loaded from maven-user.properties and available via rsession.getUserProperties()) is never consulted. Passing -Dmaven.testJarTransitiveDeps=false at the CLI has no effect on sessions created through ApiRunner.
By contrast, DefaultRepositorySystemSessionFactory (the main Maven CLI path) correctly reads the property via Features.testJarTransitiveDeps(mergedProps) before constructing the supplier. ApiRunner is a standalone/embedded execution path used by IDEs and the Maven Embedder API — its behavior diverging silently from the CLI is a correctness bug.
Fix: read userProperties (already available at that point in ApiRunner) before constructing the supplier, then use the 3-arg constructor:
boolean testJarTransitiveDeps = Features.testJarTransitiveDeps(userProperties);
MavenSessionBuilderSupplier sessionBuilderSupplier =
new MavenSessionBuilderSupplier(system, false, testJarTransitiveDeps);Minor: provided scope not tested under test-jar parent
The selectDependency override short-circuits only on TEST scope:
if (testJarParent && DependencyScope.TEST.id().equals(dependency.getScope())) {
return true;
}A provided-scoped dependency of a test-jar falls through to the delegate (ScopeDependencySelector.legacy) which correctly excludes it. The behavior is correct, but there is no test asserting that provided dependencies of a test-jar are not transitivized. Consider adding:
assertFalse(selector.selectDependency(dependency(jar("provided-dep"), "provided")));to allowsDirectTestDependenciesOfTestJar(). Without this assertion, a future refactor that accidentally passes provided through would not be caught.
This review was generated by an AI agent, Hermès on behalf of @gnodet.
| * @since 4.1.0 | ||
| */ | ||
| @Config(type = "java.lang.Boolean", defaultValue = "true") | ||
| public static final String MAVEN_TEST_JAR_TRANSITIVE_DEPS = "maven.testJarTransitiveDeps"; |
There was a problem hiding this comment.
The Javadoc note about the conditional default was added in this push — appreciated. However, a @see cross-reference to Features#testJarTransitiveDeps is still missing, making it hard for readers to find the method that actually computes the effective value. Suggest adding:
| public static final String MAVEN_TEST_JAR_TRANSITIVE_DEPS = "maven.testJarTransitiveDeps"; | |
| /** | |
| * User property for enabling transitive dependencies of consumed test JARs. | |
| * The annotation default ({@code true}) applies to Maven 4 semantics; Maven 3 personality | |
| * ({@link #MAVEN_MAVEN3_PERSONALITY}) sets the effective default to {@code false}. | |
| * | |
| * @see org.apache.maven.api.feature.Features#testJarTransitiveDeps(java.util.Map) | |
| * @since 4.1.0 | |
| */ | |
| @Config(type = "java.lang.Boolean", defaultValue = "true") | |
| public static final String MAVEN_TEST_JAR_TRANSITIVE_DEPS = "maven.testJarTransitiveDeps"; |
Summary
Fixes MNG-1378 for Maven 4 by making a producer's direct test-scoped dependencies available when that producer is consumed as a dependency of type
test-jar.Maven's normal test scope remains non-transitive. The change is deliberately artifact-specific: the scope selector is wrapped with a parent-aware selector that permits test dependencies only while collecting the children of a
test-jarartifact. Ordinary JAR dependencies keep the existing behavior.Why
A
test-jaris a separate artifact with its own classpath requirements. Its classes may depend on libraries declared withscope=testin the producer POM, but those libraries are currently dropped when another project consumes the test JAR.As a result, consumers must manually duplicate dependencies that are already part of the producer's test classpath. This is the behavior reported by MNG-1378.
Implementation
The change adds a
TestJarDependencySelectoraround Maven's existing scope selector.The selector is parent-aware: when Resolver descends into a dependency whose artifact type is exactly
test-jar, direct test-scoped children are allowed through. Everywhere else, dependency selection delegates unchanged to Maven's existing scope rules.The implementation does not:
Compatibility
The behavior change applies only to Maven 4 semantics.
Maven-3-personality mode keeps the existing selector unchanged. This avoids changing long-established Maven 3 dependency behavior while allowing Maven 4 to give
test-jarartifacts a dependency graph that matches their actual classpath requirements.Tests
This PR adds a Core IT regression fixture with four modules:
supporttest-jarsupportwithscope=testconsumertest-jarand must receive both the test JAR andsupportregular-consumersupportThe producer artifacts are installed before the consumers are resolved separately, so the test exercises repository artifact-descriptor resolution rather than relying only on an in-reactor model.
Checklist
mvn verifyhas been run successfully.License