Re: [PR] build(deps): upgrade org.apache.maven.resolver from 1.3.1 to 2.0.23 (druid)

["FrankChen021 (via GitHub)" <gi...@apache.org>]

FrankChen021 commented on code in PR #20386:
URL: https://github.com/apache/druid/pull/20386#discussion_r4053179481


##########
services/src/test/java/org/apache/druid/cli/PullDependenciesTest.java:
##########
@@ -340,23 +335,17 @@ private static class RealRepositorySystemUtil
   {
     public static RepositorySystem newRepositorySystem()
     {
-      DefaultServiceLocator locator = MavenRepositorySystemUtils.newServiceLocator();
-      locator.addService(RepositoryConnectorFactory.class, BasicRepositoryConnectorFactory.class);
-      locator.addService(TransporterFactory.class, HttpTransporterFactory.class);
-      return locator.getService(RepositorySystem.class);
+      return new RepositorySystemSupplier().get();
     }
 
-    public static DefaultRepositorySystemSession newRepositorySystemSession(
+    public static RepositorySystemSession newRepositorySystemSession(
         RepositorySystem system,
         String localRepoPath
     )
     {
-      DefaultRepositorySystemSession session = MavenRepositorySystemUtils.newSession();
-
-      LocalRepository localRepo = new LocalRepository(localRepoPath);
-      session.setLocalRepositoryManager(system.newLocalRepositoryManager(session, localRepo));
-
-      return session;
+      return system.createSessionBuilder()

Review Comment:
   [P3] Test fixture bypasses the new session defaults
   
   **Finding:** The production migration relies on SessionBuilderSupplier to install Maven-equivalent scope selector, dependency manager, and traverser defaults, but this helper now builds the injected session with bare system.createSessionBuilder(). All run() tests stub resolveDependencies for that injected session, and the direct session tests only inspect the local repository and proxy, so a regression that drops those defaults could again traverse test/provided/optional dependencies and reach unreachable repositories without a test failure.
   
   **Suggestion:** Construct the fixture with the same SessionBuilderSupplier used in production, or add a narrow real-resolution fixture asserting that test/provided/optional transitive dependencies are excluded.



-- 
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: commits-unsubscribe@druid.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org