Only apply scope overrides on test scope dependencies

Split original unit test into multiple test methods

2 files changed, 78 insertions, 52 deletions

diff --git a/bundle-plugin/src/main/java/com/yahoo/container/plugin/mojo/TestBundleDependencyScopeTranslator.java b/bundle-plugin/src/main/java/com/yahoo/container/plugin/mojo/TestBundleDependencyScopeTranslator.java
index 9ad0a0e34ba..92023df8b23 100644
--- a/bundle-plugin/src/main/java/com/yahoo/container/plugin/mojo/TestBundleDependencyScopeTranslator.java
+++ b/bundle-plugin/src/main/java/com/yahoo/container/plugin/mojo/TestBundleDependencyScopeTranslator.java
@@ -18,7 +18,7 @@ import static java.util.stream.Collectors.toList;
  *
  * Dependencies of scope 'test' are by default translated to 'compile'. Dependencies of other scopes are kept as is.
  *
- * Default scope translation can be overridden through a comma-separated configuration string.
+ * Default scope translation for 'test' scope dependencies can be overridden through a comma-separated configuration string.
  * Each substring is a triplet on the form [groupId]:[artifactId]:[scope].
  * Scope translation overrides affects all transitive dependencies.
  * The ordering of the triplets determines the priority - only the first matching override will affect a given dependency.
@@ -72,12 +72,10 @@ class TestBundleDependencyScopeTranslator implements Artifacts.ScopeTranslator {
     private static String getScopeForDependency(
             Artifact dependency, List<DependencyOverride> overrides, Map<String, Artifact> otherArtifacts) {
         String oldScope = dependency.getScope();
+        if (!oldScope.equals(Artifact.SCOPE_TEST)) return oldScope;
         for (DependencyOverride override : overrides) {
             for (Artifact dependent : dependencyTrailOf(dependency, otherArtifacts)) {
                 if (override.isForArtifact(dependent)) {
-                    // This translation is not always correct for artifacts having 'runtime' scope dependencies.
-                    // If such dependencies are overridden to 'compile' scope, its 'runtime' dependencies will get
-                    // scope 'compile' instead of 'runtime'.
                     log.fine(() -> String.format(
                             "Overriding scope of '%s'; scope '%s' overridden to '%s'",
                             dependency.getId(), oldScope, override.scope));
@@ -85,11 +83,10 @@ class TestBundleDependencyScopeTranslator implements Artifacts.ScopeTranslator {
                 }
             }
         }
-        String newScope = defaultScopeTranslationOf(oldScope);
         log.fine(() -> String.format(
-                "Using default scope translation for '%s'; scope '%s' translated to '%s'",
-                dependency.getId(), oldScope, newScope));
-        return newScope;
+                "Using default scope translation for '%s'; scope 'test' translated to 'compile'",
+                dependency.getId()));
+        return Artifact.SCOPE_COMPILE;
     }
 
     private static List<Artifact> dependencyTrailOf(Artifact artifact, Map<String, Artifact> otherArtifacts) {
@@ -100,10 +97,6 @@ class TestBundleDependencyScopeTranslator implements Artifacts.ScopeTranslator {
                 .collect(toList());
     }
 
-    private static String defaultScopeTranslationOf(String scope) {
-        return scope.equals(Artifact.SCOPE_TEST) ? Artifact.SCOPE_COMPILE : scope;
-    }
-
     private static class DependencyOverride {
         final String groupId;
         final String artifactId;
diff --git a/bundle-plugin/src/test/java/com/yahoo/container/plugin/mojo/TestBundleDependencyScopeTranslatorTest.java b/bundle-plugin/src/test/java/com/yahoo/container/plugin/mojo/TestBundleDependencyScopeTranslatorTest.java
index c0e1c9b4b9a..91ca157a3eb 100644
--- a/bundle-plugin/src/test/java/com/yahoo/container/plugin/mojo/TestBundleDependencyScopeTranslatorTest.java
+++ b/bundle-plugin/src/test/java/com/yahoo/container/plugin/mojo/TestBundleDependencyScopeTranslatorTest.java
@@ -22,54 +22,87 @@ public class TestBundleDependencyScopeTranslatorTest {
     private static final String GROUP_ID = "com.test";
 
     @Test
-    public void findsAllTestProvidedDependencies() {
+    public void test_dependencies_are_translated_to_compile_scope_by_default() {
         Map<String, Artifact> artifacts = new TreeMap<>();
-        Artifact a = createArtifact(artifacts, "a", "compile", List.of());
-        Artifact aa = createArtifact(artifacts, "a-a", "compile", List.of("a"));
-        Artifact ab = createArtifact(artifacts, "a-b", "runtime", List.of("a"));
-        Artifact aba = createArtifact(artifacts, "a-b-a", "runtime", List.of("a", "a-b"));
+        Artifact a = createArtifact(artifacts, "a", "test", List.of());
+        Artifact aa = createArtifact(artifacts, "a-a", "test", List.of("a"));
+        Artifact ab = createArtifact(artifacts, "a-b", "test", List.of("a"));
+        Artifact aba = createArtifact(artifacts, "a-b-a", "test", List.of("a", "a-b"));
+
+        TestBundleDependencyScopeTranslator translator = TestBundleDependencyScopeTranslator.from(artifacts, null);
+        assertScope(translator, a, "compile");
+        assertScope(translator, aa, "compile");
+        assertScope(translator, ab, "compile");
+        assertScope(translator, aba, "compile");
+
+    }
+
+    @Test
+    public void non_test_scope_dependencies_keep_original_scope() {
+        Map<String, Artifact> artifacts = new TreeMap<>();
+        Artifact a = createArtifact(artifacts, "a", "provided", List.of());
+        Artifact aa = createArtifact(artifacts, "a-a", "provided", List.of("a"));
+        Artifact ab = createArtifact(artifacts, "a-b", "provided", List.of("a"));
+        Artifact b = createArtifact(artifacts, "b", "runtime", List.of());
+        Artifact ba = createArtifact(artifacts, "b-a", "runtime", List.of("b"));
+        Artifact c = createArtifact(artifacts, "c", "test", List.of());
+        Artifact ca = createArtifact(artifacts, "c-a", "test", List.of("c"));
+
+        TestBundleDependencyScopeTranslator translator = TestBundleDependencyScopeTranslator.from(artifacts, null);
+        assertScope(translator, a, "provided");
+        assertScope(translator, aa, "provided");
+        assertScope(translator, ab, "provided");
+        assertScope(translator, b, "runtime");
+        assertScope(translator, ba, "runtime");
+        assertScope(translator, c, "compile");
+        assertScope(translator, ca, "compile");
+    }
+
+    @Test
+    public void ordering_in_config_string_determines_translation() {
+        Map<String, Artifact> artifacts = new TreeMap<>();
+        Artifact a = createArtifact(artifacts, "a", "test", List.of());
+        Artifact aa = createArtifact(artifacts, "a-a", "test", List.of("a"));
+        {
+            String configString =
+                    "com.test:a-a:runtime," +
+                    "com.test:a:test,";
+            TestBundleDependencyScopeTranslator translator = TestBundleDependencyScopeTranslator.from(artifacts, configString);
+            assertScope(translator, a, "test");
+            assertScope(translator, aa, "runtime");
+        }
+        {
+            String configString =
+                    "com.test:a:test," +
+                    "com.test:a-a:runtime";
+            TestBundleDependencyScopeTranslator translator = TestBundleDependencyScopeTranslator.from(artifacts, configString);
+            assertScope(translator, a, "test");
+            assertScope(translator, aa, "test");
+        }
+    }
+
+    @Test
+    public void transitive_non_test_dependencies_of_test_dependencies_keep_original_scope() {
+        Map<String, Artifact> artifacts = new TreeMap<>();
+        Artifact a = createArtifact(artifacts, "a", "test", List.of());
+        Artifact aa = createArtifact(artifacts, "a-a", "test", List.of("a"));
+        Artifact ab = createArtifact(artifacts, "a-b", "test", List.of("a"));
+        Artifact aba = createArtifact(artifacts, "a-b-a", "compile", List.of("a", "a-b"));
         Artifact ac = createArtifact(artifacts, "a-c", "runtime", List.of("a"));
-        Artifact ad = createArtifact(artifacts, "a-d", "compile", List.of("a"));
-        Artifact ada = createArtifact(artifacts, "a-d-a", "compile", List.of("a", "a-d"));
-        Artifact adb = createArtifact(artifacts, "a-d-b", "compile", List.of("a", "a-d"));
-        Artifact b = createArtifact(artifacts, "b", "provided", List.of());
-        Artifact ba = createArtifact(artifacts, "b-a", "provided", List.of("b"));
+        Artifact b = createArtifact(artifacts, "b", "test", List.of());
+        Artifact ba = createArtifact(artifacts, "b-a", "test", List.of("b"));
         Artifact bb = createArtifact(artifacts, "b-b", "provided", List.of("b"));
-        Artifact c = createArtifact(artifacts, "c", "runtime", List.of());
-        Artifact ca = createArtifact(artifacts, "c-a", "runtime", List.of("c"));
-        Artifact d = createArtifact(artifacts, "d", "test", List.of());
-        Artifact da = createArtifact(artifacts, "d-a", "test", List.of("d"));
-        Artifact daa = createArtifact(artifacts, "d-a-a", "test", List.of("d", "d-a"));
-        Artifact db = createArtifact(artifacts, "d-b", "test", List.of("d"));
-        Artifact dc = createArtifact(artifacts, "d-c", "test", List.of("d"));
-        Artifact dca = createArtifact(artifacts, "d-c-a", "test", List.of("d", "d-c"));
-
-        String configString =
-                "com.test:a-d:compile," +
-                "com.test:a:provided," +
-                "com.test:d-a:test," +
-                "com.test:d-c:compile," +
-                "com.test:d:runtime";
+
+        String configString = "com.test:a:provided";
         TestBundleDependencyScopeTranslator translator = TestBundleDependencyScopeTranslator.from(artifacts, configString);
         assertScope(translator, a, "provided");
         assertScope(translator, aa, "provided");
         assertScope(translator, ab, "provided");
-        assertScope(translator, aba, "provided");
-        assertScope(translator, ac, "provided");
-        assertScope(translator, ad, "compile");
-        assertScope(translator, ada, "compile");
-        assertScope(translator, adb, "compile");
-        assertScope(translator, b, "provided");
-        assertScope(translator, ba, "provided");
+        assertScope(translator, aba, "compile");
+        assertScope(translator, ac, "runtime");
+        assertScope(translator, b, "compile");
+        assertScope(translator, ba, "compile");
         assertScope(translator, bb, "provided");
-        assertScope(translator, c, "runtime");
-        assertScope(translator, ca, "runtime");
-        assertScope(translator, d, "runtime");
-        assertScope(translator, da, "test");
-        assertScope(translator, daa, "test");
-        assertScope(translator, db, "runtime");
-        assertScope(translator, dc, "compile");
-        assertScope(translator, dca, "compile");
     }
 
     private static Artifact createArtifact(