Skip to content

[MNG-1378] make test-jar dependencies transitive - #13066

Open
mcc0nnell wants to merge 3 commits into
apache:masterfrom
mcc0nnell:fix/MNG-1378-test-jar-transitives
Open

mcc0nnell wants to merge 3 commits into
apache:masterfrom
mcc0nnell:fix/MNG-1378-test-jar-transitives

Conversation

@mcc0nnell

@mcc0nnell mcc0nnell commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

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-jar artifact. Ordinary JAR dependencies keep the existing behavior.

Why

A test-jar is a separate artifact with its own classpath requirements. Its classes may depend on libraries declared with scope=test in 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 TestJarDependencySelector around 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:

  • make Maven's test scope generally transitive;
  • rewrite producer POMs;
  • change ordinary JAR dependency behavior;
  • change Maven-3-personality behavior.

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-jar artifacts a dependency graph that matches their actual classpath requirements.

Tests

This PR adds a Core IT regression fixture with four modules:

Module Role
support An ordinary dependency
test-jar Produces a test JAR and depends on support with scope=test
consumer Depends only on the producer's test-jar and must receive both the test JAR and support
regular-consumer Depends on the producer's ordinary JAR and must not receive support

The 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

  • This pull request addresses one issue: MNG-1378.
  • The pull request description explains what changes, how, and why.
  • The branch contains one focused commit with a meaningful subject and body.
  • A behavioral Core IT regression fixture is included and is expected to fail without the runtime change.
  • mvn verify has been run successfully.
  • Core IT has been run successfully.

License

  • I hereby declare this contribution to be licenced under the Apache License Version 2.0, January 2004.

Draft until Maven CI and Core IT validation are complete.

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>
@mcc0nnell
mcc0nnell force-pushed the fix/MNG-1378-test-jar-transitives branch from 06e2472 to a022b55 Compare September 7, 2026 17:30
@mcc0nnell

Copy link
Copy Markdown
Contributor Author

@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.

@gnodet

gnodet commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

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 test-jar is not "private tests" — it's an explicitly published artifact with its own classpath contract. The moment you configure maven-jar-plugin with <goal>test-jar</goal>, you're making a deliberate decision to package test classes as a reusable, deployable artifact with its own GAV coordinates. At that point, its test-scoped dependencies are part of that contract, and stripping them produces a broken classpath for consumers.

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 test-jar node — is appropriately conservative. It doesn't make test scope generally transitive, it doesn't affect regular JAR resolution, and it preserves Maven 3 personality behavior.

A few things to address before this can move forward:

  1. Unit tests for TestJarDependencySelector — the IT coverage is solid, but the selector itself should have direct unit tests covering edge cases (nested test-jars, test-jar with transitive test-jar dependencies, interaction with exclusions/optional).
  2. Type detection — the selector keys on ArtifactProperties.TYPE being exactly "test-jar". Worth verifying this property is reliably set on artifacts resolved from the repository, not just in-reactor.

@gnodet

gnodet commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

One additional thought: this PR targets master (4.1.0), but the feature did not exist in 4.0.0. A test-jar consumer upgrading from 4.0.0 to 4.1.0 would suddenly get extra transitive dependencies they didn't have before — that could break builds (dependency conflicts, convergence enforcer rules, etc.). Even if the new behavior is correct, a silent behavioral change in a minor version can be painful.

I'd suggest adding a feature flag in o.a.m.api.feature.Features (with a corresponding constant in Constants), following the existing pattern (e.g. consumerPom). Something like:

public static boolean testJarTransitiveDeps(@Nullable Map<String, ?> userProperties) {
    return doGet(userProperties, Constants.MAVEN_TEST_JAR_TRANSITIVE_DEPS, !mavenMaven3Personality(userProperties));
}

This way it:

  • defaults to true in Maven 4 mode
  • defaults to false under Maven 3 personality
  • can be explicitly overridden via -Dmaven.testJarTransitiveDeps=false (or in .mvn/maven.config)

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.

@gnodet gnodet added the enhancement New feature or request label Sep 7, 2026
@gnodet gnodet added this to the 4.1.0 milestone Sep 7, 2026
@elharo elharo changed the title MNG-1378: make test-jar dependencies transitive [MNG-1378] make test-jar dependencies transitive Sep 9, 2026
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 gnodet-bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 gnodet-bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 true for Maven 4 semantics and false when 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";

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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:

Suggested change
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";

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants