Separate distro tests to be per distribution

buildSrc/src/main/groovy/org/elasticsearch/gradle/test/DistroTestPlugin.java

@@ -36,6 +36,7 @@
 import org.gradle.api.NamedDomainObjectContainer;
 import org.gradle.api.Plugin;
 import org.gradle.api.Project;
+import org.gradle.api.Task;
 import org.gradle.api.artifacts.Configuration;
 import org.gradle.api.file.Directory;
 import org.gradle.api.plugins.ExtraPropertiesExtension;
@@ -75,6 +76,7 @@ public class DistroTestPlugin implements Plugin<Project> {
     private static final String COPY_UPGRADE_TASK = "copyUpgradePackages";
     private static final String COPY_PLUGINS_TASK = "copyPlugins";
     private static final String IN_VM_SYSPROP = "tests.inVM";
+    private static final String DISTRIBUTION_SYSPROP = "tests.distribution";
 
     @Override
     public void apply(Project project) {
@@ -89,14 +91,15 @@ public void apply(Project project) {
         Provider<Directory> upgradeDir = project.getLayout().getBuildDirectory().dir("packaging/upgrade");
         Provider<Directory> pluginsDir = project.getLayout().getBuildDirectory().dir("packaging/plugins");
 
-        configureDistributions(project, upgradeVersion);
+        List<ElasticsearchDistribution> distributions = configureDistributions(project, upgradeVersion);
         TaskProvider<Copy> copyDistributionsTask = configureCopyDistributionsTask(project, distributionsDir);
         TaskProvider<Copy> copyUpgradeTask = configureCopyUpgradeTask(project, upgradeVersion, upgradeDir);
         TaskProvider<Copy> copyPluginsTask = configureCopyPluginsTask(project, pluginsDir);
 
-        Map<String, TaskProvider<?>> distroTests = new HashMap<>();
         Map<String, TaskProvider<?>> batsTests = new HashMap<>();
-        distroTests.put("distribution", configureDistroTest(project, distributionsDir, copyDistributionsTask));
+        for (ElasticsearchDistribution distribution : distributions) {
+            configureDistroTest(project, distribution);
+        }
         batsTests.put("bats oss", configureBatsTest(project, "oss", distributionsDir, copyDistributionsTask));
         batsTests.put("bats default", configureBatsTest(project, "default", distributionsDir, copyDistributionsTask));
         configureBatsTest(project, "plugins",distributionsDir, copyDistributionsTask, copyPluginsTask).configure(t ->
@@ -109,17 +112,27 @@ public void apply(Project project) {
             vmProject.getPluginManager().apply(VagrantBasePlugin.class);
             vmProject.getPluginManager().apply(JdkDownloadPlugin.class);
             List<Object> vmDependencies = new ArrayList<>(configureVM(vmProject));
-            // a hack to ensure the parent task has already been run. this will not be necessary once tests are per distribution
-            // which will eliminate the copy distributions task altogether
-            vmDependencies.add(copyDistributionsTask);
             vmDependencies.add(project.getConfigurations().getByName("testRuntimeClasspath"));
 
-            distroTests.forEach((desc, task) -> configureVMWrapperTask(vmProject, desc, task.getName(), vmDependencies));
-            VagrantExtension vagrant = vmProject.getExtensions().getByType(VagrantExtension.class);
+            TaskProvider<Task> distroTest = vmProject.getTasks().register("distroTest");
+            for (ElasticsearchDistribution distribution : distributions) {
+                String destructiveTaskName = destructiveDistroTestTaskName(distribution);
+                Platform platform = distribution.getPlatform();
+                // this condition ensures windows boxes get windows distributions, and linux boxes get linux distributions
+                if (isWindows(vmProject) == (platform == Platform.WINDOWS)) {
+                    TaskProvider<GradleDistroTestTask> vmTask =
+                        configureVMWrapperTask(vmProject, distribution.getName() + " distribution", destructiveTaskName, vmDependencies);
+                    vmTask.configure(t -> t.dependsOn(distribution));
+                    distroTest.configure(t -> t.dependsOn(vmTask));
+                }
+            }
+
+
             batsTests.forEach((desc, task) -> {
                 configureVMWrapperTask(vmProject, desc, task.getName(), vmDependencies).configure(t -> {
                     t.setProgressHandler(new BatsProgressLogger(project.getLogger()));
-                    t.onlyIf(spec -> vagrant.isWindowsVM() == false); // bats doesn't run on windows
+                    t.onlyIf(spec -> isWindows(vmProject) == false); // bats doesn't run on windows
+                    t.dependsOn(copyDistributionsTask);
                 });
             });
         });
@@ -169,7 +182,7 @@ private static List<Object> configureVM(Project project) {
         vagrant.setBox(box);
         vagrant.vmEnv("SYSTEM_JAVA_HOME", convertPath(project, vagrant, systemJdk, "", ""));
         vagrant.vmEnv("PATH", convertPath(project, vagrant, gradleJdk, "/bin:$PATH", "\\bin;$Env:PATH"));
-        vagrant.setIsWindowsVM(box.contains("windows"));
+        vagrant.setIsWindowsVM(isWindows(project));
 
         return Arrays.asList(systemJdk, gradleJdk);
     }
@@ -269,15 +282,14 @@ private static TaskProvider<GradleDistroTestTask> configureVMWrapperTask(Project
             });
     }
 
-    private static TaskProvider<?> configureDistroTest(Project project, Provider<Directory> distributionsDir,
-                                                       TaskProvider<Copy> copyPackagingArchives) {
-        // TODO: don't run with security manager...
-        return project.getTasks().register("destructiveDistroTest", Test.class,
+    private static TaskProvider<?> configureDistroTest(Project project, ElasticsearchDistribution distribution) {
+        return project.getTasks().register(destructiveDistroTestTaskName(distribution), Test.class,
             t -> {
                 t.setMaxParallelForks(1);
-                t.setWorkingDir(distributionsDir);
+                t.setWorkingDir(project.getProjectDir());
+                t.systemProperty(DISTRIBUTION_SYSPROP, distribution.toString());
                 if (System.getProperty(IN_VM_SYSPROP) == null) {
-                    t.dependsOn(copyPackagingArchives);
+                    t.dependsOn(distribution);
                 }
             });
     }
@@ -297,7 +309,7 @@ private static TaskProvider<BatsTestTask> configureBatsTest(Project project, Str
             });
     }
 
-    private void configureDistributions(Project project, Version upgradeVersion) {
+    private List<ElasticsearchDistribution> configureDistributions(Project project, Version upgradeVersion) {
         NamedDomainObjectContainer<ElasticsearchDistribution> distributions = DistributionDownloadPlugin.getContainer(project);
         List<ElasticsearchDistribution> currentDistros = new ArrayList<>();
         List<ElasticsearchDistribution> upgradeDistros = new ArrayList<>();
@@ -335,13 +347,15 @@ private void configureDistributions(Project project, Version upgradeVersion) {
         List<Configuration> distroUpgradeConfigs = upgradeDistros.stream().map(ElasticsearchDistribution::getConfiguration)
             .collect(Collectors.toList());
         packagingUpgradeConfig.setExtendsFrom(distroUpgradeConfigs);
+
+        return currentDistros;
     }
 
     private static void addDistro(NamedDomainObjectContainer<ElasticsearchDistribution> distributions,
                                   Type type, Platform platform, Flavor flavor, boolean bundledJdk, String version,
                                   List<ElasticsearchDistribution> container) {
 
-        String name = flavor + "-" + (type == Type.ARCHIVE ? platform + "-" : "") + type + (bundledJdk ? "" : "-no-jdk") + "-" + version;
+        String name = distroId(type, platform, flavor, bundledJdk) + "-" + version;
         if (distributions.findByName(name) != null) {
             return;
         }
@@ -356,4 +370,17 @@ private static void addDistro(NamedDomainObjectContainer<ElasticsearchDistributi
         });
         container.add(distro);
     }
+
+    // return true if the project is for a windows VM, false otherwise
+    private static boolean isWindows(Project project) {
+        return project.getName().contains("windows");
+    }
+
+    private static String distroId(Type type, Platform platform, Flavor flavor, boolean bundledJdk) {
+        return flavor + "-" + (type == Type.ARCHIVE ? platform + "-" : "") + type + (bundledJdk ? "" : "-no-jdk");
+    }
+
+    private static String destructiveDistroTestTaskName(ElasticsearchDistribution distro) {
+        return "destructiveDistroTest." + distroId(distro.getType(), distro.getPlatform(), distro.getFlavor(), distro.getBundledJdk());
+    }
 }

buildSrc/src/main/java/org/elasticsearch/gradle/vagrant/VagrantShellTask.java

@@ -104,15 +104,15 @@ public void runScript() {
                     spec.setProgressHandler(progressHandler);
                 });
             } catch (Exception e) {
-                getLogger().error("Failed command, dumping dmesg", e);
+                /*getLogger().error("Failed command, dumping dmesg", e);
                 service.execute(spec -> {
                     spec.setCommand("ssh");
                     spec.setArgs("--command", "dmesg");
                     spec.setProgressHandler(line -> {
                         getLogger().error(line);
                         return null;
                     });
-                });
+                });*/
                 throw e;
             }
         }

qa/os/src/test/java/org/elasticsearch/packaging/test/ArchiveTestCase.java → qa/os/src/test/java/org/elasticsearch/packaging/test/ArchiveTests.java

@@ -19,7 +19,6 @@
 
 package org.elasticsearch.packaging.test;
 
-import com.carrotsearch.randomizedtesting.annotations.TestCaseOrdering;
 import org.apache.http.client.fluent.Request;
 import org.elasticsearch.packaging.util.Archives;
 import org.elasticsearch.packaging.util.Distribution;
@@ -29,6 +28,7 @@
 import org.elasticsearch.packaging.util.ServerUtils;
 import org.elasticsearch.packaging.util.Shell;
 import org.elasticsearch.packaging.util.Shell.Result;
+import org.junit.BeforeClass;
 
 import java.nio.file.Files;
 import java.nio.file.Path;
@@ -55,13 +55,15 @@
 import static org.hamcrest.CoreMatchers.notNullValue;
 import static org.hamcrest.Matchers.isEmptyString;
 import static org.junit.Assume.assumeThat;
+import static org.junit.Assume.assumeTrue;
 
-/**
- * Tests that apply to the archive distributions (tar, zip). To add a case for a distribution, subclass and
- * override {@link ArchiveTestCase#distribution()}. These tests should be the same across all archive distributions
- */
-@TestCaseOrdering(TestCaseOrdering.AlphabeticOrder.class)
-public abstract class ArchiveTestCase extends PackagingTestCase {
+public class ArchiveTests extends PackagingTestCase {
+
+    @BeforeClass
+    public static void assumptions() {
+        assumeTrue("only archive distributions",
+            distribution().packaging == Distribution.Packaging.TAR || distribution().packaging == Distribution.Packaging.ZIP);
+    }
 
     public void test10Install() throws Exception {
         installation = installArchive(distribution());
@@ -362,7 +364,7 @@ public void test90SecurityCliPackaging() throws Exception {
         final Installation.Executables bin = installation.executables();
         final Shell sh = newShell();
 
-        if (distribution().equals(Distribution.DEFAULT_LINUX) || distribution().equals(Distribution.DEFAULT_WINDOWS)) {
+        if (distribution().isDefault()) {
             assertTrue(Files.exists(installation.lib.resolve("tools").resolve("security-cli")));
             final Platforms.PlatformAction action = () -> {
                 Result result = sh.run(bin.elasticsearchCertutil + " --help");
@@ -375,7 +377,7 @@ public void test90SecurityCliPackaging() throws Exception {
             };
             Platforms.onLinux(action);
             Platforms.onWindows(action);
-        } else if (distribution().equals(Distribution.OSS_LINUX) || distribution().equals(Distribution.OSS_WINDOWS)) {
+        } else {
             assertFalse(Files.exists(installation.lib.resolve("tools").resolve("security-cli")));
         }
     }
@@ -391,7 +393,8 @@ public void test91ElasticsearchShardCliPackaging() throws Exception {
             assertThat(result.stdout, containsString("A CLI tool to remove corrupted parts of unrecoverable shards"));
         };
 
-        if (distribution().equals(Distribution.DEFAULT_LINUX) || distribution().equals(Distribution.DEFAULT_WINDOWS)) {
+        // TODO: this should be checked on all distributions
+        if (distribution().isDefault()) {
             Platforms.onLinux(action);
             Platforms.onWindows(action);
         }
@@ -409,7 +412,8 @@ public void test92ElasticsearchNodeCliPackaging() throws Exception {
                     containsString("A CLI tool to do unsafe cluster and index manipulations on current node"));
         };
 
-        if (distribution().equals(Distribution.DEFAULT_LINUX) || distribution().equals(Distribution.DEFAULT_WINDOWS)) {
+        // TODO: this should be checked on all distributions
+        if (distribution().isDefault()) {
             Platforms.onLinux(action);
             Platforms.onWindows(action);
         }
@@ -454,7 +458,8 @@ public void test94ElasticsearchNodeExecuteCliNotEsHomeWorkDir() throws Exception
                 containsString("Manages elasticsearch file users"));
         };
 
-        if (distribution().equals(Distribution.DEFAULT_LINUX) || distribution().equals(Distribution.DEFAULT_WINDOWS)) {
+        // TODO: this should be checked on all distributions
+        if (distribution().isDefault()) {
             Platforms.onLinux(action);
             Platforms.onWindows(action);
         }

qa/os/src/test/java/org/elasticsearch/packaging/test/DebPreservationTestCase.java → qa/os/src/test/java/org/elasticsearch/packaging/test/DebPreservationTests.java

@@ -21,15 +21,12 @@
 
 import com.carrotsearch.randomizedtesting.annotations.TestCaseOrdering;
 import org.elasticsearch.packaging.util.Distribution;
-import org.elasticsearch.packaging.util.Installation;
 import org.elasticsearch.packaging.util.Shell;
 import org.junit.Before;
-import org.junit.BeforeClass;
 
 import java.nio.file.Files;
 import java.nio.file.Paths;
 
-import static org.elasticsearch.packaging.util.Cleanup.cleanEverything;
 import static org.elasticsearch.packaging.util.FileUtils.assertPathsDontExist;
 import static org.elasticsearch.packaging.util.FileUtils.assertPathsExist;
 import static org.elasticsearch.packaging.util.Packages.SYSVINIT_SCRIPT;
@@ -46,21 +43,13 @@
 import static org.junit.Assume.assumeTrue;
 
 @TestCaseOrdering(TestCaseOrdering.AlphabeticOrder.class)
-public abstract class DebPreservationTestCase extends PackagingTestCase {
-
-    private static Installation installation;
-
-    protected abstract Distribution distribution();
-
-    @BeforeClass
-    public static void cleanup() throws Exception {
-        installation = null;
-        cleanEverything();
-    }
+public class DebPreservationTests extends PackagingTestCase {
 
     @Before
     public void onlyCompatibleDistributions() {
         assumeTrue("only dpkg platforms", isDPKG());
+        assumeTrue("deb distributions", distribution().packaging == Distribution.Packaging.DEB);
+        assumeTrue("only bundled jdk", distribution().hasJdk);
         assumeTrue("only compatible distributions", distribution().packaging.compatible);
     }