Testing conventions task part 2 (#36107)

Closes #35435

- make it easier to add additional testing tasks with the proper configuration and add some where they were missing.
- mute or fix failing tests
- add a check as part of testing conventions to find classes not included in any testing task.
This commit is contained in:
Alpar Torok 2018-12-05 14:20:01 +02:00 committed by GitHub
parent 786697a4b2
commit 60e45cd81d
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
24 changed files with 188 additions and 80 deletions

View File

@ -147,10 +147,8 @@ if (project == rootProject) {
mavenLocal()
}
}
test {
include "**/*Tests.class"
exclude "**/*IT.class"
}
// only run tests as build-tools
test.enabled = false
}
/*****************************************************************************
@ -180,9 +178,12 @@ if (project != rootProject) {
jarHell.enabled = false
thirdPartyAudit.enabled = false
// tests can't be run with randomized test runner
// it's fine as we run them as part of :buildSrc
test.enabled = false
test {
include "**/*Tests.class"
exclude "**/*IT.class"
}
// This can't be an RandomizedTestingTask because we can't yet reference it
task integTest(type: Test) {
// integration test requires the local testing repo for example plugin builds
dependsOn project.rootProject.allprojects.collect {

View File

@ -770,13 +770,26 @@ class BuildPlugin implements Plugin<Project> {
}
static void applyCommonTestConfig(Project project) {
project.tasks.withType(RandomizedTestingTask) {
project.tasks.withType(RandomizedTestingTask) {task ->
jvm "${project.runtimeJavaHome}/bin/java"
parallelism System.getProperty('tests.jvms', project.rootProject.ext.defaultParallel)
ifNoTests System.getProperty('tests.ifNoTests', 'fail')
onNonEmptyWorkDirectory 'wipe'
leaveTemporary true
// Make sure all test tasks are configured properly
if (name != "test") {
project.tasks.matching { it.name == "test"}.all { testTask ->
task.testClassesDirs = testTask.testClassesDirs
task.classpath = testTask.classpath
task.shouldRunAfter testTask
}
}
// no loose ends: check has to depend on all test tasks
project.tasks.matching {it.name == "check"}.all {
dependsOn(task)
}
// TODO: why are we not passing maxmemory to junit4?
jvmArg '-Xmx' + System.getProperty('tests.heap.size', '512m')
jvmArg '-Xms' + System.getProperty('tests.heap.size', '512m')

View File

@ -129,7 +129,6 @@ public class PluginBuildPlugin extends BuildPlugin {
RestIntegTestTask integTest = project.tasks.create('integTest', RestIntegTestTask.class)
integTest.mustRunAfter(project.precommit, project.test)
project.integTestCluster.distribution = System.getProperty('tests.distribution', 'integ-test-zip')
project.check.dependsOn(integTest)
}
/**

View File

@ -20,15 +20,20 @@ package org.elasticsearch.gradle.precommit;
import org.elasticsearch.gradle.tool.Boilerplate;
import org.gradle.api.DefaultTask;
import org.gradle.api.Task;
import org.gradle.api.file.FileCollection;
import org.gradle.api.file.FileTree;
import org.gradle.api.tasks.Input;
import org.gradle.api.tasks.OutputFile;
import org.gradle.api.tasks.SkipWhenEmpty;
import org.gradle.api.tasks.TaskAction;
import org.gradle.api.tasks.testing.Test;
import org.gradle.api.tasks.util.PatternFilterable;
import java.io.File;
import java.io.IOException;
import java.lang.annotation.Annotation;
import java.lang.reflect.InvocationTargetException;
import java.lang.reflect.Method;
import java.lang.reflect.Modifier;
import java.net.MalformedURLException;
@ -40,8 +45,9 @@ import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.StandardOpenOption;
import java.nio.file.attribute.BasicFileAttributes;
import java.util.ArrayList;
import java.util.List;
import java.util.HashMap;
import java.util.Map;
import java.util.Set;
import java.util.function.Predicate;
import java.util.stream.Collectors;
import java.util.stream.Stream;
@ -57,7 +63,7 @@ public class TestingConventionsTasks extends DefaultTask {
*/
private Boolean activeTestsExists;
private List<String> testClassNames;
private Map<String, File> testClassNames;
public TestingConventionsTasks() {
setDescription("Tests various testing conventions");
@ -68,56 +74,155 @@ public class TestingConventionsTasks extends DefaultTask {
@TaskAction
public void doCheck() throws IOException {
activeTestsExists = false;
final List<String> problems;
final String problems;
try (URLClassLoader isolatedClassLoader = new URLClassLoader(
getTestsClassPath().getFiles().stream().map(this::fileToUrl).toArray(URL[]::new)
)) {
List<? extends Class<?>> classes = getTestClassNames().stream()
.map(name -> loadClassWithoutInitializing(name, isolatedClassLoader))
.collect(Collectors.toList());
Predicate<Class<?>> isStaticClass = clazz -> Modifier.isStatic(clazz.getModifiers());
Predicate<Class<?>> isPublicClass = clazz -> Modifier.isPublic(clazz.getModifiers());
Predicate<Class<?>> implementsNamingConvention = clazz -> clazz.getName().endsWith(TEST_CLASS_SUFIX) ||
clazz.getName().endsWith(INTEG_TEST_CLASS_SUFIX);
Predicate<Class<?>> implementsNamingConvention = clazz ->
clazz.getName().endsWith(TEST_CLASS_SUFIX) ||
clazz.getName().endsWith(INTEG_TEST_CLASS_SUFIX);
problems = Stream.concat(
Map<File, ? extends Class<?>> classes = getTestClassNames().entrySet().stream()
.collect(Collectors.toMap(
Map.Entry::getValue,
entry -> loadClassWithoutInitializing(entry.getKey(), isolatedClassLoader))
);
FileTree allTestClassFiles = getProject().files(
classes.values().stream()
.filter(isStaticClass.negate())
.filter(isPublicClass)
.filter(implementsNamingConvention)
.map(clazz -> testClassNames.get(clazz.getName()))
.collect(Collectors.toList())
).getAsFileTree();
final Map<String, Set<File>> classFilesPerRandomizedTestingTask = classFilesPerRandomizedTestingTask(allTestClassFiles);
final Map<String, Set<File>> classFilesPerGradleTestTask = classFilesPerGradleTestTask();
problems = collectProblems(
checkNoneExists(
"Test classes implemented by inner classes will not run",
classes.stream()
classes.values().stream()
.filter(isStaticClass)
.filter(implementsNamingConvention.or(this::seemsLikeATest))
).stream(),
),
checkNoneExists(
"Seem like test classes but don't match naming convention",
classes.stream()
classes.values().stream()
.filter(isStaticClass.negate())
.filter(isPublicClass)
.filter(this::seemsLikeATest)
.filter(implementsNamingConvention.negate())
).stream()
).collect(Collectors.toList());
),
checkNoneExists(
"Test classes are not included in any enabled task (" +
Stream.concat(
classFilesPerRandomizedTestingTask.keySet().stream(),
classFilesPerGradleTestTask.keySet().stream()
).collect(Collectors.joining(",")) + ")",
allTestClassFiles.getFiles().stream()
.filter(testFile ->
classFilesPerRandomizedTestingTask.values().stream()
.anyMatch(fileSet -> fileSet.contains(testFile)) == false &&
classFilesPerGradleTestTask.values().stream()
.anyMatch(fileSet -> fileSet.contains(testFile)) == false
)
.map(classes::get)
)
);
}
if (problems.isEmpty()) {
getLogger().error(problems);
throw new IllegalStateException("Testing conventions are not honored");
} else {
getSuccessMarker().getParentFile().mkdirs();
Files.write(getSuccessMarker().toPath(), new byte[]{}, StandardOpenOption.CREATE);
} else {
problems.forEach(getProject().getLogger()::error);
throw new IllegalStateException("Testing conventions are not honored");
}
}
private String collectProblems(String... problems) {
return Stream.of(problems)
.map(String::trim)
.filter(String::isEmpty)
.map(each -> each + "\n")
.collect(Collectors.joining());
}
@Input
public Map<String, Set<File>> classFilesPerRandomizedTestingTask(FileTree testClassFiles) {
return
Stream.concat(
getProject().getTasks().withType(getRandomizedTestingTask()).stream(),
// Look at sub-projects too. As sometimes tests are implemented in parent but ran in sub-projects against
// different configurations
getProject().getSubprojects().stream().flatMap(subproject ->
subproject.getTasks().withType(getRandomizedTestingTask()).stream()
)
)
.filter(Task::getEnabled)
.collect(Collectors.toMap(
Task::getPath,
task -> testClassFiles.matching(getRandomizedTestingPatternSet(task)).getFiles()
));
}
@Input
public Map<String, Set<File>> classFilesPerGradleTestTask() {
return Stream.concat(
getProject().getTasks().withType(Test.class).stream(),
getProject().getSubprojects().stream().flatMap(subproject ->
subproject.getTasks().withType(Test.class).stream()
)
)
.filter(Task::getEnabled)
.collect(Collectors.toMap(
Task::getPath,
task -> task.getCandidateClassFiles().getFiles()
));
}
@SuppressWarnings("unchecked")
private PatternFilterable getRandomizedTestingPatternSet(Task task) {
try {
if (
getRandomizedTestingTask().isAssignableFrom(task.getClass()) == false
) {
throw new IllegalStateException("Expected " + task + " to be RandomizedTestingTask or Test but it was " + task.getClass());
}
Method getPatternSet = task.getClass().getMethod("getPatternSet");
return (PatternFilterable) getPatternSet.invoke(task);
} catch (NoSuchMethodException e) {
throw new IllegalStateException("Expecte task to have a `patternSet` " + task, e);
} catch (IllegalAccessException | InvocationTargetException e) {
throw new IllegalStateException("Failed to get pattern set from task" + task, e);
}
}
@SuppressWarnings("unchecked")
private Class<? extends Task> getRandomizedTestingTask() {
try {
return (Class<? extends Task>) Class.forName("com.carrotsearch.gradle.junit4.RandomizedTestingTask");
} catch (ClassNotFoundException | ClassCastException e) {
throw new IllegalStateException("Failed to load randomized testing class", e);
}
}
@Input
@SkipWhenEmpty
public List<String> getTestClassNames() {
public Map<String, File> getTestClassNames() {
if (testClassNames == null) {
testClassNames = Boilerplate.getJavaSourceSets(getProject()).getByName("test").getOutput().getClassesDirs()
.getFiles().stream()
.filter(File::exists)
.flatMap(testRoot -> walkPathAndLoadClasses(testRoot).stream())
.collect(Collectors.toList());
.flatMap(testRoot -> walkPathAndLoadClasses(testRoot).entrySet().stream())
.collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue));
}
return testClassNames;
}
@ -127,16 +232,15 @@ public class TestingConventionsTasks extends DefaultTask {
return new File(getProject().getBuildDir(), "markers/" + getName());
}
private List<String> checkNoneExists(String message, Stream<? extends Class<?>> stream) {
List<String> problems = new ArrayList<>();
List<Class<?>> entries = stream.collect(Collectors.toList());
if (entries.isEmpty() == false) {
problems.add(message + ":");
entries.stream()
.map(each -> " * " + each.getName())
.forEach(problems::add);
private String checkNoneExists(String message, Stream<? extends Class<?>> stream) {
String problem = stream
.map(each -> " * " + each.getName())
.collect(Collectors.joining("\n"));
if (problem.isEmpty() == false) {
return message + ":\n" + problem;
} else{
return "";
}
return problems;
}
private boolean seemsLikeATest(Class<?> clazz) {
@ -197,8 +301,8 @@ public class TestingConventionsTasks extends DefaultTask {
);
}
private List<String> walkPathAndLoadClasses(File testRoot) {
List<String> classes = new ArrayList<>();
private Map<String, File> walkPathAndLoadClasses(File testRoot) {
Map<String, File> classes = new HashMap<>();
try {
Files.walkFileTree(testRoot.toPath(), new FileVisitor<Path>() {
private String packageName;
@ -227,7 +331,7 @@ public class TestingConventionsTasks extends DefaultTask {
String filename = file.getFileName().toString();
if (filename.endsWith(".class")) {
String className = filename.substring(0, filename.length() - ".class".length());
classes.add(packageName + className);
classes.put(packageName + className, file.toFile());
}
return FileVisitResult.CONTINUE;
}

View File

@ -19,7 +19,6 @@
package org.elasticsearch.gradle.precommit;
import java.io.File;
import java.io.IOException;
import java.nio.charset.Charset;
import java.nio.file.Files;
import java.util.List;
@ -32,12 +31,8 @@ import org.gradle.api.Project;
import org.gradle.api.plugins.JavaPlugin;
import org.gradle.testfixtures.ProjectBuilder;
import org.junit.Assert;
import org.junit.Rule;
import org.junit.rules.TemporaryFolder;
public class FilePermissionsTaskTests extends GradleUnitTestCase {
@Rule
public TemporaryFolder temporaryFolder = new TemporaryFolder();
public void testCheckPermissionsWhenAnExecutableFileExists() throws Exception {
RandomizedTest.assumeFalse("Functionality is Unix specific", Os.isFamily(Os.FAMILY_WINDOWS));
@ -93,11 +88,10 @@ public class FilePermissionsTaskTests extends GradleUnitTestCase {
assertEquals("done", result.get(0));
file.delete();
}
private Project createProject() throws IOException {
Project project = ProjectBuilder.builder().withProjectDir(temporaryFolder.newFolder()).build();
private Project createProject() {
Project project = ProjectBuilder.builder().build();
project.getPlugins().apply(JavaPlugin.class);
return project;
}
@ -105,4 +99,5 @@ public class FilePermissionsTaskTests extends GradleUnitTestCase {
private FilePermissionsTask createTask(Project project) {
return project.getTasks().create("filePermissionsTask", FilePermissionsTask.class);
}
}

View File

@ -301,6 +301,7 @@ if (secureFixtureSupported) {
// Security tests unsupported. Don't run these tests.
integTestSecure.enabled = false
integTestSecureHa.enabled = false
testingConventions.enabled = false
}
thirdPartyAudit.excludes = [

View File

@ -73,10 +73,7 @@ task testRepositoryCreds(type: RandomizedTestingTask) {
include '**/RepositoryCredentialsTests.class'
include '**/S3BlobStoreRepositoryTests.class'
systemProperty 'es.allow_insecure_settings', 'true'
classpath = tasks.test.classpath
testClassesDirs = tasks.test.testClassesDirs
}
project.check.dependsOn(testRepositoryCreds)
test {
// these are tested explicitly in separate test tasks

View File

@ -320,10 +320,6 @@ if (isEclipse == false || project.path == ":server-tests") {
group: JavaBasePlugin.VERIFICATION_GROUP,
description: 'Multi-node tests',
dependsOn: test.dependsOn) {
classpath = project.test.classpath
testClassesDirs = project.test.testClassesDirs
include '**/*IT.class'
}
check.dependsOn integTest
integTest.mustRunAfter test
}

View File

@ -16,6 +16,8 @@
* specific language governing permissions and limitations
* under the License.
*/
import com.carrotsearch.gradle.junit4.RandomizedTestingTask;
dependencies {
compile "org.elasticsearch.client:elasticsearch-rest-client:${version}"
compile "org.elasticsearch.client:elasticsearch-rest-client-sniffer:${version}"
@ -73,3 +75,7 @@ test.configure {
systemProperty 'tests.gradle_wire_compat_versions', bwcVersions.wireCompatible.join(',')
systemProperty 'tests.gradle_unreleased_versions', bwcVersions.unreleased.join(',')
}
task integTest(type: RandomizedTestingTask) {
include "**/*IT.class"
}

View File

@ -19,6 +19,7 @@
package org.elasticsearch.test.disruption;
import org.apache.lucene.util.LuceneTestCase.AwaitsFix;
import org.elasticsearch.plugins.Plugin;
import org.elasticsearch.test.ESIntegTestCase;
import org.elasticsearch.test.InternalTestCluster;
@ -34,6 +35,7 @@ import java.util.Set;
import static org.hamcrest.Matchers.greaterThanOrEqualTo;
@AwaitsFix(bugUrl = "https://github.com/elastic/elasticsearch/issues/36205")
public class NetworkDisruptionIT extends ESIntegTestCase {
@Override
protected Collection<Class<? extends Plugin>> nodePlugins() {

View File

@ -41,6 +41,7 @@ public class SuiteScopeClusterIT extends ESIntegTestCase {
@Test
@SuppressForbidden(reason = "repeat is a feature here")
@Repeat(iterations = 10, useConstantSeed = true)
@AwaitsFix(bugUrl = "https://github.com/elastic/elasticsearch/issues/36202")
public void testReproducible() throws IOException {
if (ITER++ == 0) {
CLUSTER_SEED = cluster().seed();

View File

@ -24,15 +24,10 @@ task internalClusterTest(type: RandomizedTestingTask,
group: JavaBasePlugin.VERIFICATION_GROUP,
description: 'Java fantasy integration tests',
dependsOn: test.dependsOn) {
classpath = project.test.classpath
testClassesDirs = project.test.testClassesDirs
include '**/*IT.class'
systemProperty 'es.set.netty.runtime.available.processors', 'false'
}
check.dependsOn internalClusterTest
internalClusterTest.mustRunAfter test
// add all sub-projects of the qa sub-project
gradle.projectsEvaluated {
project.subprojects

View File

@ -4,6 +4,7 @@ import java.nio.file.Files
import java.nio.file.Path
import java.nio.file.Paths
import java.nio.file.StandardCopyOption
import com.carrotsearch.gradle.junit4.RandomizedTestingTask;
apply plugin: 'elasticsearch.esplugin'
apply plugin: 'nebula.maven-base-publish'
@ -136,5 +137,10 @@ thirdPartyAudit.excludes = [
// xpack modules are installed in real clusters as the meta plugin, so
// installing them as individual plugins for integ tests doesn't make sense,
// so we disable integ tests and there are no integ tests in xpack core module
// so we disable integ tests
integTest.enabled = false
// There are some integ tests that don't require a cluster, we still want to run those
task internalClusterTest(type: RandomizedTestingTask) {
include "**/*IT.class"
}

View File

@ -98,15 +98,10 @@ task internalClusterTest(type: RandomizedTestingTask,
group: JavaBasePlugin.VERIFICATION_GROUP,
description: 'Multi-node tests',
dependsOn: test.dependsOn) {
classpath = project.test.classpath
testClassesDirs = project.test.testClassesDirs
include '**/*IT.class'
systemProperty 'es.set.netty.runtime.available.processors', 'false'
}
check.dependsOn internalClusterTest
internalClusterTest.mustRunAfter test
// add all sub-projects of the qa sub-project
gradle.projectsEvaluated {
project.subprojects

View File

@ -60,13 +60,9 @@ task internalClusterTest(type: RandomizedTestingTask,
group: JavaBasePlugin.VERIFICATION_GROUP,
description: 'Multi-node tests',
dependsOn: test.dependsOn) {
classpath = project.test.classpath
testClassesDirs = project.test.testClassesDirs
include '**/*IT.class'
systemProperty 'es.set.netty.runtime.available.processors', 'false'
}
check.dependsOn internalClusterTest
internalClusterTest.mustRunAfter test
// also add an "alias" task to make typing on the command line easier task icTest {
task icTest {

View File

@ -33,13 +33,9 @@ task internalClusterTest(type: RandomizedTestingTask,
group: JavaBasePlugin.VERIFICATION_GROUP,
description: 'Multi-node tests',
dependsOn: test.dependsOn) {
classpath = project.test.classpath
testClassesDirs = project.test.testClassesDirs
include '**/*IT.class'
systemProperty 'es.set.netty.runtime.available.processors', 'false'
}
check.dependsOn internalClusterTest
internalClusterTest.mustRunAfter test
// also add an "alias" task to make typing on the command line easier
task icTest {

View File

@ -133,6 +133,7 @@ integTestRunner {
if (project.rootProject.vagrantSupported == false) {
integTest.enabled = false
testingConventions.enabled = false
} else {
project.sourceSets.test.output.dir(generatedResources)
integTestCluster.dependsOn krb5AddPrincipals, krb5kdcFixture, copyKeytabToGeneratedResources

View File

@ -27,6 +27,7 @@ if (project.rootProject.vagrantSupported) {
test.finalizedBy idpFixtureProject.halt
} else {
test.enabled = false
testingConventions.enabled = false
}
namingConventions {

View File

@ -28,6 +28,7 @@ if (project.rootProject.vagrantSupported) {
integTest.finalizedBy idpFixtureProject.halt
} else {
integTest.enabled = false
testingConventions.enabled = false
}
integTestCluster {

View File

@ -29,5 +29,3 @@ test {
include '**/*Tests.class'
}
// these are just tests, no need to audit
thirdPartyAudit.enabled = false

View File

@ -29,4 +29,5 @@ integTestCluster {
if (!integrationAccount && !userAccount && !v1Account) {
integTest.enabled = false
testingConventions.enabled = false
}

View File

@ -47,6 +47,7 @@ task cleanJira(type: DefaultTask) {
// require network access for this one, exit early instead of starting up the cluster if we dont have network
if (!jiraUrl && !jiraUser && !jiraPassword && !jiraProject) {
integTest.enabled = false
testingConventions.enabled = false
} else {
integTestRunner.finalizedBy cleanJira
}

View File

@ -18,5 +18,6 @@ integTestCluster {
}
if (!pagerDutyServiceKey) {
integTest.enabled = false
integTest.enabled = false
testingConventions.enabled = false
}

View File

@ -22,4 +22,5 @@ integTestCluster {
if (!slackUrl) {
integTest.enabled = false
testingConventions.enabled = false
}