gnodet-bot commented on code in PR #13584:
URL: https://github.com/apache/maven/pull/13584#discussion_r4301495654
##########
maven-embedder/src/test/java/org/apache/maven/cli/MavenCliTest.java:
##########
@@ -607,4 +617,63 @@ void skipsCheckWhenAlternatePomFileSpecified() throws
Exception {
assertNull(ex, "Expected no ExitException when -f/--file is
specified");
}
}
+
+ @Test
+ public void testCoreExtensionComponentVisibility() throws Exception {
+ Field f = MavenCli.class.getDeclaredField("slf4jLogger");
+ f.setAccessible(true);
+ f.set(cli, LoggerFactory.getLogger(MavenCli.class));
+
+ ClassWorld classWorld = new ClassWorld();
+ ClassRealm coreRealm =
+ classWorld.newRealm("plexus.core",
Thread.currentThread().getContextClassLoader());
+ ClassRealm extRealm1 =
+ classWorld.newRealm("coreExtension>ext1",
Thread.currentThread().getContextClassLoader());
+
+ CoreExtensionEntry entry = new CoreExtensionEntry(extRealm1,
Collections.emptyList(), Collections.emptyList());
+ ClassRealm containerRealm = cli.setupContainerRealm(
+ classWorld, coreRealm, Collections.emptyList(),
Collections.singletonList(entry));
+
+ assertTrue(
+ extRealm1.getImportRealms().contains(containerRealm),
+ "Extension realm must import container realm for Sisu
component visibility");
+
+ ContainerConfiguration cc = new DefaultContainerConfiguration()
+ .setClassWorld(classWorld)
+ .setRealm(containerRealm)
+ .setClassPathScanning(PlexusConstants.SCANNING_INDEX)
+ .setAutoWiring(true)
+ .setJSR250Lifecycle(true)
+ .setName("maven");
+
+ DefaultPlexusContainer container = new DefaultPlexusContainer(cc);
+ try {
+ container.setLookupRealm(null);
+
+ ComponentDescriptor<Runnable> descriptor = new
ComponentDescriptor<>();
+ descriptor.setRole(Runnable.class.getName());
+ descriptor.setRoleHint("test");
+ descriptor.setImplementationClass(MyLazyComponent.class);
+ container.addComponentDescriptor(descriptor);
Review Comment:
💡 **Test design note:** `addComponentDescriptor` registers the component via
the Plexus classic path, which bypasses Sisu's JSR330 index-based discovery.
This means the `container.lookup()` call below tests Plexus resolution rather
than Sisu visibility filtering.
The real validation of the fix is the structural assertion above —
`extRealm1.getImportRealms().contains(containerRealm)` directly verifies the
new `realm.importFrom(extRealm, extRealm.getId())` line. You might consider
noting that distinction, or simplifying the lookup portion since it doesn't
exercise the Sisu path.
##########
maven-embedder/src/main/java/org/apache/maven/cli/MavenCli.java:
##########
@@ -48,6 +48,7 @@
import java.util.regex.Pattern;
import java.util.stream.Stream;
+import com.google.common.annotations.VisibleForTesting;
Review Comment:
💡 **Style:** `@VisibleForTesting` from Guava is used here solely to document
that `setupContainerRealm` is package-private for test access. Consider
replacing the annotation with a simple comment — it conveys the same intent
without introducing a Guava annotation into production code:
```suggestion
```
(and inline on the method: `/* package-private for testing */`)
--
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]
