Add test to properly verify all exported packages from jdisc_core

- The integration test generates exportPackages.properties in the
  same way as the maven execution, reading all the embedded bundles
  that we re-export packages for.
- The failure message shows the difference between expected and
  actual packages, and a command to reset the expected set in case
  the change was intentional.
- Remove the now obsolete ExportPackagesTestCase.

2 files changed, 99 insertions, 45 deletions

diff --git a/jdisc_core/src/test/java/com/yahoo/jdisc/core/ExportPackagesIT.java b/jdisc_core/src/test/java/com/yahoo/jdisc/core/ExportPackagesIT.java
index 395ddc889ae..1cd8db67476 100644
--- a/jdisc_core/src/test/java/com/yahoo/jdisc/core/ExportPackagesIT.java
+++ b/jdisc_core/src/test/java/com/yahoo/jdisc/core/ExportPackagesIT.java
@@ -1,15 +1,22 @@
 package com.yahoo.jdisc.core;
 
-import org.junit.Rule;
+import org.junit.ClassRule;
 import org.junit.Test;
 import org.junit.rules.TemporaryFolder;
 
 import java.io.File;
 import java.io.FileReader;
+import java.io.IOException;
+import java.util.Arrays;
+import java.util.List;
 import java.util.Properties;
+import java.util.Set;
+import java.util.TreeSet;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
 
 import static org.junit.Assert.assertNotNull;
-import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
 
 /**
  * Integration tests for {@link ExportPackages}.
@@ -18,25 +25,102 @@ import static org.junit.Assert.assertTrue;
  */
 public class ExportPackagesIT {
 
-    @Rule
-    public TemporaryFolder tempFolder= new TemporaryFolder();
+
+    private static final File expectedExportPackages = new File("src/test/resources" + ExportPackages.PROPERTIES_FILE);
+
+    private static final String JAR_PATH = "target/dependency/";
+
+    // This list must be kept in sync with the list in the export-packages execution in pom.xml.
+    private static final List<String> RE_EXPORTED_BUNDLES = Stream.of(
+            "guava.jar",
+            "guice-no_aop.jar",
+            "slf4j-api.jar",
+            "slf4j-jdk14.jar",
+            "jcl-over-slf4j.jar",
+            "log4j-over-slf4j.jar",
+            "annotations.jar",
+            "config-lib.jar",
+            "defaults.jar",
+            "vespajlib.jar",
+            "vespalog.jar",
+            "jaxb-api.jar",
+            "jaxb-core.jar",
+            "jaxb-impl.jar",
+            "javax.activation.jar"
+    ).map(f -> JAR_PATH + f).toList();
+
+    @ClassRule
+    public static TemporaryFolder tempFolder = new TemporaryFolder();
 
     @Test
-    public void export_packages_are_added_from_dependency_jars() throws Exception {
-        File file = tempFolder.newFile(ExportPackages.PROPERTIES_FILE);
+    public void exported_packages_are_not_changed_unintentionally() throws Exception {
+        Properties actualProperties = getPropertiesFromFile(createPropertiesFile());
+        String actualValue = actualProperties.getProperty(ExportPackages.EXPORT_PACKAGES);
+        assertNotNull("Missing exportPackages property in file.", actualValue);
 
-        ExportPackages.main(new String[] { file.getAbsolutePath(), "target/dependency/guice-no_aop.jar" });
-        assertTrue(file.exists());
-        Properties props = new Properties();
-        String exportPackages;
-        try (FileReader reader = new FileReader(file)) {
-            props.load(reader);
-            exportPackages = props.getProperty(ExportPackages.EXPORT_PACKAGES);
+        Properties expectedProperties = getPropertiesFromFile(expectedExportPackages);
+        String expectedValue = expectedProperties.getProperty(ExportPackages.EXPORT_PACKAGES);
+        assertNotNull("Missing exportPackages property in file.", expectedValue);
+
+        Set<String> actualPackages = getPackages(actualValue);
+        Set<String> expectedPackages = getPackages(expectedValue);
+        if (! actualPackages.equals(expectedPackages)) {
+            StringBuilder message = getDiff(actualPackages, expectedPackages);
+            message.append("\n\nIf this test fails due to an intentional change in exported packages, run the following command:\n")
+                    .append("$ cp jdisc_core/target/classes/exportPackages.properties jdisc_core/src/test/resources/")
+                    .append("\n\nNote that removing exported packages usually requires a new major version of Vespa.\n");
+            fail(message.toString());
         }
-        assertNotNull(exportPackages);
+    }
 
-        assertTrue(exportPackages.contains("com.google.inject"));
+    private static StringBuilder getDiff(Set<String> actual, Set<String> expected) {
+        StringBuilder sb = new StringBuilder();
+        Set<String> onlyInActual = onlyInSet1(actual, expected);
+        if (! onlyInActual.isEmpty()) {
+            sb.append("\nexportPackages.properties contained ")
+                    .append(onlyInActual.size())
+                    .append(" unexpected packages:\n")
+                    .append(onlyInActual.stream().collect(Collectors.joining(",\n  ", " [", "]")));
+        }
 
+        Set<String> onlyInExpected = onlyInSet1(expected, actual);
+        if (! onlyInExpected.isEmpty()) {
+            sb.append("\nexportPackages.properties did not contain ")
+                    .append(onlyInExpected.size())
+                    .append(" expected packages:\n")
+                    .append(onlyInExpected.stream().collect(Collectors.joining(",\n  ", " [", "]")));
+        }
+        return sb;
+    }
+
+    // Returns a sorted set for readability.
+    private static Set<String> onlyInSet1(Set<String> set1, Set<String> set2) {
+        return set1.stream()
+                .filter(s -> ! set2.contains(s))
+                .collect(Collectors.toCollection(TreeSet::new));
+    }
+
+    private static Set<String> getPackages(String propertyValue) {
+        return Arrays.stream(propertyValue.split(","))
+                .map(String::trim)
+                .filter(s -> ! s.isEmpty())
+                .collect(Collectors.toSet());
+    }
+
+    private static Properties getPropertiesFromFile(File file) throws IOException {
+        Properties properties = new Properties();
+        try (FileReader reader = new FileReader(file)) {
+            properties.load(reader);
+        }
+        return properties;
+    }
+
+    private static File createPropertiesFile() throws IOException {
+        File file = tempFolder.newFile(ExportPackages.PROPERTIES_FILE);
+        String[] args = Stream.concat(Stream.of(file.getAbsolutePath()),
+                                      RE_EXPORTED_BUNDLES.stream()).toArray(String[]::new);
+        ExportPackages.main(args);
+        return file;
     }
 
 }
diff --git a/jdisc_core/src/test/java/com/yahoo/jdisc/core/ExportPackagesTestCase.java b/jdisc_core/src/test/java/com/yahoo/jdisc/core/ExportPackagesTestCase.java
deleted file mode 100644
index 225ecd4a242..00000000000
--- a/jdisc_core/src/test/java/com/yahoo/jdisc/core/ExportPackagesTestCase.java
+++ /dev/null
@@ -1,30 +0,0 @@
-// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
-package com.yahoo.jdisc.core;
-
-import org.junit.Test;
-
-import java.io.File;
-import java.io.FileReader;
-import java.util.Properties;
-
-import static org.junit.Assert.assertNotNull;
-import static org.junit.Assert.assertTrue;
-
-/**
- * @author Simon Thoresen Hult
- */
-public class ExportPackagesTestCase {
-
-    @Test
-    public void requireThatPropertiesAreWritten() throws Exception {
-        File file = new File("target", ExportPackages.PROPERTIES_FILE);
-        file.deleteOnExit();
-        ExportPackages.main(new String[] { file.getAbsolutePath() });
-        assertTrue(file.exists());
-        Properties props = new Properties();
-        try (FileReader reader = new FileReader(file)) {
-            props.load(reader);
-            assertNotNull(props.getProperty(ExportPackages.EXPORT_PACKAGES));
-        }
-    }
-}