From d5df27f2715ea5fc3db4579c1832acf99c359de0 Mon Sep 17 00:00:00 2001 From: Ryan Ernst Date: Mon, 18 Jul 2022 14:36:54 -0700 Subject: [PATCH 1/2] Reduce boilerplate in plugins utils tests Tests for PluginsUtils need to create many different plugin descriptors. But the vast majority of these just need a plugin name and dependencies. This commit adds a utility method to simplify these tests. --- .../plugins/PluginsUtilsTests.java | 342 ++---------------- 1 file changed, 40 insertions(+), 302 deletions(-) diff --git a/server/src/test/java/org/elasticsearch/plugins/PluginsUtilsTests.java b/server/src/test/java/org/elasticsearch/plugins/PluginsUtilsTests.java index 956d522626026..99f8665fc86c7 100644 --- a/server/src/test/java/org/elasticsearch/plugins/PluginsUtilsTests.java +++ b/server/src/test/java/org/elasticsearch/plugins/PluginsUtilsTests.java @@ -21,7 +21,6 @@ import java.net.URL; import java.nio.file.Files; import java.nio.file.Path; -import java.util.Arrays; import java.util.Collections; import java.util.HashMap; import java.util.LinkedHashSet; @@ -38,6 +37,21 @@ @LuceneTestCase.SuppressFileSystems(value = "ExtrasFS") public class PluginsUtilsTests extends ESTestCase { + PluginDescriptor newTestDescriptor(String name, List deps) { + return new PluginDescriptor( + name, + "desc", + "1.0", + Version.CURRENT, + Runtime.version().toString(), + "MyPlugin", + null, + deps, + false, + false + ); + } + public void testExistingPluginMissingDescriptor() throws Exception { Path pluginsDir = createTempDir(); Files.createDirectory(pluginsDir.resolve("plugin-missing-descriptor")); @@ -47,18 +61,7 @@ public void testExistingPluginMissingDescriptor() throws Exception { public void testSortBundlesCycleSelfReference() throws Exception { Path pluginDir = createTempDir(); - PluginDescriptor info = new PluginDescriptor( - "foo", - "desc", - "1.0", - Version.CURRENT, - "1.8", - "MyPlugin", - null, - Collections.singletonList("foo"), - false, - false - ); + PluginDescriptor info = newTestDescriptor("foo", List.of("foo")); PluginBundle bundle = new PluginBundle(info, pluginDir); IllegalStateException e = expectThrows(IllegalStateException.class, () -> PluginsUtils.sortBundles(Collections.singleton(bundle))); assertEquals("Cycle found in plugin dependencies: foo -> foo", e.getMessage()); @@ -67,57 +70,13 @@ public void testSortBundlesCycleSelfReference() throws Exception { public void testSortBundlesCycle() throws Exception { Path pluginDir = createTempDir(); Set bundles = new LinkedHashSet<>(); // control iteration order, so we get know the beginning of the cycle - PluginDescriptor info = new PluginDescriptor( - "foo", - "desc", - "1.0", - Version.CURRENT, - "1.8", - "MyPlugin", - null, - Arrays.asList("bar", "other"), - false, - false - ); + PluginDescriptor info = newTestDescriptor("foo", List.of("bar", "other")); bundles.add(new PluginBundle(info, pluginDir)); - PluginDescriptor info2 = new PluginDescriptor( - "bar", - "desc", - "1.0", - Version.CURRENT, - "1.8", - "MyPlugin", - null, - Collections.singletonList("baz"), - false, - false - ); + PluginDescriptor info2 = newTestDescriptor("bar", List.of("baz")); bundles.add(new PluginBundle(info2, pluginDir)); - PluginDescriptor info3 = new PluginDescriptor( - "baz", - "desc", - "1.0", - Version.CURRENT, - "1.8", - "MyPlugin", - null, - Collections.singletonList("foo"), - false, - false - ); + PluginDescriptor info3 = newTestDescriptor("baz", List.of("foo")); bundles.add(new PluginBundle(info3, pluginDir)); - PluginDescriptor info4 = new PluginDescriptor( - "other", - "desc", - "1.0", - Version.CURRENT, - "1.8", - "MyPlugin", - null, - Collections.emptyList(), - false, - false - ); + PluginDescriptor info4 = newTestDescriptor("other", List.of()); bundles.add(new PluginBundle(info4, pluginDir)); IllegalStateException e = expectThrows(IllegalStateException.class, () -> PluginsUtils.sortBundles(bundles)); @@ -126,18 +85,7 @@ public void testSortBundlesCycle() throws Exception { public void testSortBundlesSingle() throws Exception { Path pluginDir = createTempDir(); - PluginDescriptor info = new PluginDescriptor( - "foo", - "desc", - "1.0", - Version.CURRENT, - "1.8", - "MyPlugin", - null, - Collections.emptyList(), - false, - false - ); + PluginDescriptor info = newTestDescriptor("foo", List.of()); PluginBundle bundle = new PluginBundle(info, pluginDir); List sortedBundles = PluginsUtils.sortBundles(Collections.singleton(bundle)); assertThat(sortedBundles, Matchers.contains(bundle)); @@ -146,46 +94,13 @@ public void testSortBundlesSingle() throws Exception { public void testSortBundlesNoDeps() throws Exception { Path pluginDir = createTempDir(); Set bundles = new LinkedHashSet<>(); // control iteration order - PluginDescriptor info1 = new PluginDescriptor( - "foo", - "desc", - "1.0", - Version.CURRENT, - "1.8", - "MyPlugin", - null, - Collections.emptyList(), - false, - false - ); + PluginDescriptor info1 = newTestDescriptor("foo", List.of()); PluginBundle bundle1 = new PluginBundle(info1, pluginDir); bundles.add(bundle1); - PluginDescriptor info2 = new PluginDescriptor( - "bar", - "desc", - "1.0", - Version.CURRENT, - "1.8", - "MyPlugin", - null, - Collections.emptyList(), - false, - false - ); + PluginDescriptor info2 = newTestDescriptor("bar", List.of()); PluginBundle bundle2 = new PluginBundle(info2, pluginDir); bundles.add(bundle2); - PluginDescriptor info3 = new PluginDescriptor( - "baz", - "desc", - "1.0", - Version.CURRENT, - "1.8", - "MyPlugin", - null, - Collections.emptyList(), - false, - false - ); + PluginDescriptor info3 = newTestDescriptor("baz", List.of()); PluginBundle bundle3 = new PluginBundle(info3, pluginDir); bundles.add(bundle3); List sortedBundles = PluginsUtils.sortBundles(bundles); @@ -194,18 +109,7 @@ public void testSortBundlesNoDeps() throws Exception { public void testSortBundlesMissingDep() throws Exception { Path pluginDir = createTempDir(); - PluginDescriptor info = new PluginDescriptor( - "foo", - "desc", - "1.0", - Version.CURRENT, - "1.8", - "MyPlugin", - null, - Collections.singletonList("dne"), - false, - false - ); + PluginDescriptor info = newTestDescriptor("foo", List.of("dne")); PluginBundle bundle = new PluginBundle(info, pluginDir); IllegalArgumentException e = expectThrows( IllegalArgumentException.class, @@ -217,60 +121,16 @@ public void testSortBundlesMissingDep() throws Exception { public void testSortBundlesCommonDep() throws Exception { Path pluginDir = createTempDir(); Set bundles = new LinkedHashSet<>(); // control iteration order - PluginDescriptor info1 = new PluginDescriptor( - "grandparent", - "desc", - "1.0", - Version.CURRENT, - "1.8", - "MyPlugin", - null, - Collections.emptyList(), - false, - false - ); + PluginDescriptor info1 = newTestDescriptor("grandparent", List.of()); PluginBundle bundle1 = new PluginBundle(info1, pluginDir); bundles.add(bundle1); - PluginDescriptor info2 = new PluginDescriptor( - "foo", - "desc", - "1.0", - Version.CURRENT, - "1.8", - "MyPlugin", - null, - Collections.singletonList("common"), - false, - false - ); + PluginDescriptor info2 = newTestDescriptor("foo", List.of("common")); PluginBundle bundle2 = new PluginBundle(info2, pluginDir); bundles.add(bundle2); - PluginDescriptor info3 = new PluginDescriptor( - "bar", - "desc", - "1.0", - Version.CURRENT, - "1.8", - "MyPlugin", - null, - Collections.singletonList("common"), - false, - false - ); + PluginDescriptor info3 = newTestDescriptor("bar", List.of("common")); PluginBundle bundle3 = new PluginBundle(info3, pluginDir); bundles.add(bundle3); - PluginDescriptor info4 = new PluginDescriptor( - "common", - "desc", - "1.0", - Version.CURRENT, - "1.8", - "MyPlugin", - null, - Collections.singletonList("grandparent"), - false, - false - ); + PluginDescriptor info4 = newTestDescriptor("common", List.of("grandparent")); PluginBundle bundle4 = new PluginBundle(info4, pluginDir); bundles.add(bundle4); List sortedBundles = PluginsUtils.sortBundles(bundles); @@ -280,32 +140,10 @@ public void testSortBundlesCommonDep() throws Exception { public void testSortBundlesAlreadyOrdered() throws Exception { Path pluginDir = createTempDir(); Set bundles = new LinkedHashSet<>(); // control iteration order - PluginDescriptor info1 = new PluginDescriptor( - "dep", - "desc", - "1.0", - Version.CURRENT, - "1.8", - "MyPlugin", - null, - Collections.emptyList(), - false, - false - ); + PluginDescriptor info1 = newTestDescriptor("dep", List.of()); PluginBundle bundle1 = new PluginBundle(info1, pluginDir); bundles.add(bundle1); - PluginDescriptor info2 = new PluginDescriptor( - "myplugin", - "desc", - "1.0", - Version.CURRENT, - "1.8", - "MyPlugin", - null, - Collections.singletonList("dep"), - false, - false - ); + PluginDescriptor info2 = newTestDescriptor("myplugin", List.of("dep")); PluginBundle bundle2 = new PluginBundle(info2, pluginDir); bundles.add(bundle2); List sortedBundles = PluginsUtils.sortBundles(bundles); @@ -363,18 +201,7 @@ public void testJarHellDuplicateCodebaseWithDep() throws Exception { makeJar(dupJar); Map> transitiveDeps = new HashMap<>(); transitiveDeps.put("dep", Collections.singleton(dupJar.toUri().toURL())); - PluginDescriptor info1 = new PluginDescriptor( - "myplugin", - "desc", - "1.0", - Version.CURRENT, - "1.8", - "MyPlugin", - null, - Collections.singletonList("dep"), - false, - false - ); + PluginDescriptor info1 = newTestDescriptor("myplugin", List.of("dep")); PluginBundle bundle = new PluginBundle(info1, pluginDir); IllegalStateException e = expectThrows( IllegalStateException.class, @@ -394,18 +221,7 @@ public void testJarHellDuplicateCodebaseAcrossDeps() throws Exception { Map> transitiveDeps = new HashMap<>(); transitiveDeps.put("dep1", Collections.singleton(dupJar.toUri().toURL())); transitiveDeps.put("dep2", Collections.singleton(dupJar.toUri().toURL())); - PluginDescriptor info1 = new PluginDescriptor( - "myplugin", - "desc", - "1.0", - Version.CURRENT, - "1.8", - "MyPlugin", - null, - Arrays.asList("dep1", "dep2"), - false, - false - ); + PluginDescriptor info1 = newTestDescriptor("myplugin", List.of("dep1", "dep2")); PluginBundle bundle = new PluginBundle(info1, pluginDir); IllegalStateException e = expectThrows( IllegalStateException.class, @@ -422,18 +238,7 @@ public void testJarHellDuplicateClassWithCore() throws Exception { Path pluginDir = createTempDir(); Path pluginJar = pluginDir.resolve("plugin.jar"); makeJar(pluginJar, Level.class); - PluginDescriptor info1 = new PluginDescriptor( - "myplugin", - "desc", - "1.0", - Version.CURRENT, - "1.8", - "MyPlugin", - null, - Collections.emptyList(), - false, - false - ); + PluginDescriptor info1 = newTestDescriptor("myplugin", List.of()); PluginBundle bundle = new PluginBundle(info1, pluginDir); IllegalStateException e = expectThrows( IllegalStateException.class, @@ -451,19 +256,7 @@ public void testJarHellWhenExtendedPluginJarNotFound() throws Exception { Path otherDir = createTempDir(); Path extendedPlugin = otherDir.resolve("extendedDep-not-present.jar"); - PluginDescriptor info = new PluginDescriptor( - "dummy", - "desc", - "1.0", - Version.CURRENT, - "1.8", - "Dummy", - null, - Arrays.asList("extendedPlugin"), - false, - false - ); - + PluginDescriptor info = newTestDescriptor("dummy", List.of("extendedPlugin")); PluginBundle bundle = new PluginBundle(info, pluginDir); Map> transitiveUrls = new HashMap<>(); transitiveUrls.put("extendedPlugin", Collections.singleton(extendedPlugin.toUri().toURL())); @@ -485,18 +278,7 @@ public void testJarHellDuplicateClassWithDep() throws Exception { makeJar(depJar, DummyClass1.class); Map> transitiveDeps = new HashMap<>(); transitiveDeps.put("dep", Collections.singleton(depJar.toUri().toURL())); - PluginDescriptor info1 = new PluginDescriptor( - "myplugin", - "desc", - "1.0", - Version.CURRENT, - "1.8", - "MyPlugin", - null, - Collections.singletonList("dep"), - false, - false - ); + PluginDescriptor info1 = newTestDescriptor("myplugin", List.of("dep")); PluginBundle bundle = new PluginBundle(info1, pluginDir); IllegalStateException e = expectThrows( IllegalStateException.class, @@ -520,18 +302,7 @@ public void testJarHellDuplicateClassAcrossDeps() throws Exception { Map> transitiveDeps = new HashMap<>(); transitiveDeps.put("dep1", Collections.singleton(dep1Jar.toUri().toURL())); transitiveDeps.put("dep2", Collections.singleton(dep2Jar.toUri().toURL())); - PluginDescriptor info1 = new PluginDescriptor( - "myplugin", - "desc", - "1.0", - Version.CURRENT, - "1.8", - "MyPlugin", - null, - Arrays.asList("dep1", "dep2"), - false, - false - ); + PluginDescriptor info1 = newTestDescriptor("myplugin", List.of("dep1", "dep2")); PluginBundle bundle = new PluginBundle(info1, pluginDir); IllegalStateException e = expectThrows( IllegalStateException.class, @@ -555,18 +326,7 @@ public void testJarHellTransitiveMap() throws Exception { Map> transitiveDeps = new HashMap<>(); transitiveDeps.put("dep1", Collections.singleton(dep1Jar.toUri().toURL())); transitiveDeps.put("dep2", Collections.singleton(dep2Jar.toUri().toURL())); - PluginDescriptor info1 = new PluginDescriptor( - "myplugin", - "desc", - "1.0", - Version.CURRENT, - "1.8", - "MyPlugin", - null, - Arrays.asList("dep1", "dep2"), - false, - false - ); + PluginDescriptor info1 = newTestDescriptor("myplugin", List.of("dep1", "dep2")); PluginBundle bundle = new PluginBundle(info1, pluginDir); PluginsUtils.checkBundleJarHell(JarHell.parseModulesAndClassPath(), bundle, transitiveDeps); Set deps = transitiveDeps.get("myplugin"); @@ -587,18 +347,7 @@ public void testJarHellSpiAddedToTransitiveDeps() throws Exception { makeJar(depJar, DummyClass1.class); Map> transitiveDeps = new HashMap<>(); transitiveDeps.put("dep", Collections.singleton(depJar.toUri().toURL())); - PluginDescriptor info1 = new PluginDescriptor( - "myplugin", - "desc", - "1.0", - Version.CURRENT, - "1.8", - "MyPlugin", - null, - Collections.singletonList("dep"), - false, - false - ); + PluginDescriptor info1 = newTestDescriptor("myplugin", List.of("dep")); PluginBundle bundle = new PluginBundle(info1, pluginDir); PluginsUtils.checkBundleJarHell(JarHell.parseModulesAndClassPath(), bundle, transitiveDeps); Set transitive = transitiveDeps.get("myplugin"); @@ -618,18 +367,7 @@ public void testJarHellSpiConflict() throws Exception { makeJar(depJar, DummyClass1.class); Map> transitiveDeps = new HashMap<>(); transitiveDeps.put("dep", Collections.singleton(depJar.toUri().toURL())); - PluginDescriptor info1 = new PluginDescriptor( - "myplugin", - "desc", - "1.0", - Version.CURRENT, - "1.8", - "MyPlugin", - null, - Collections.singletonList("dep"), - false, - false - ); + PluginDescriptor info1 = newTestDescriptor("myplugin", List.of("dep")); PluginBundle bundle = new PluginBundle(info1, pluginDir); IllegalStateException e = expectThrows( IllegalStateException.class, From f94ffe3acbd52887dfc0384e749d0c87481289d3 Mon Sep 17 00:00:00 2001 From: Ryan Ernst Date: Mon, 18 Jul 2022 14:42:56 -0700 Subject: [PATCH 2/2] more --- .../elasticsearch/plugins/PluginsUtilsTests.java | 14 ++------------ 1 file changed, 2 insertions(+), 12 deletions(-) diff --git a/server/src/test/java/org/elasticsearch/plugins/PluginsUtilsTests.java b/server/src/test/java/org/elasticsearch/plugins/PluginsUtilsTests.java index 99f8665fc86c7..86611b3d5beed 100644 --- a/server/src/test/java/org/elasticsearch/plugins/PluginsUtilsTests.java +++ b/server/src/test/java/org/elasticsearch/plugins/PluginsUtilsTests.java @@ -38,18 +38,8 @@ public class PluginsUtilsTests extends ESTestCase { PluginDescriptor newTestDescriptor(String name, List deps) { - return new PluginDescriptor( - name, - "desc", - "1.0", - Version.CURRENT, - Runtime.version().toString(), - "MyPlugin", - null, - deps, - false, - false - ); + String javaVersion = Runtime.version().toString(); + return new PluginDescriptor(name, "desc", "1.0", Version.CURRENT, javaVersion, "MyPlugin", null, deps, false, false); } public void testExistingPluginMissingDescriptor() throws Exception {