Skip to content

Commit

Permalink
7.x - Create plugin for yamlTest task (#56841) (#59090)
Browse files Browse the repository at this point in the history
This commit creates a new Gradle plugin to provide a separate task name
and source set for running YAML based REST tests. The only project
converted to use the new plugin in this PR is distribution/archives/integ-test-zip.
For which the testing has been moved to :rest-api-spec since it makes the most
sense and it avoids a small but awkward change to the distribution plugin.

The remaining cases in modules, plugins, and x-pack will be handled in followups.

This plugin is distinctly different from the plugin introduced in #55896 since
the YAML REST tests are intended to be black box tests over HTTP. As such they
should not (by default) have access to the classpath for that which they are testing.

The YAML based REST tests will be moved to separate source sets (yamlRestTest).
The which source is the target for the test resources is dependent on if this
new plugin is applied. If it is not applied, it will default to the test source
set.

Further, this introduces a breaking change for plugin developers that
use the YAML testing framework. They will now need to either use the new source set
and matching task, or configure the rest resources to use the old "test" source set that
matches the old integTest task. (The former should be preferred).

As part of this change (which is also breaking for plugin developers) the
rest resources plugin has been removed from the build plugin and now requires
either explicit application or application via the new YAML REST test plugin.

Plugin developers should be able to fix the breaking changes to the YAML tests
by adding apply plugin: 'elasticsearch.yaml-rest-test' and moving the YAML tests
under a yamlRestTest folder (instead of test)
  • Loading branch information
jakelandis authored Jul 6, 2020
1 parent eff5f4d commit 604c6dd
Show file tree
Hide file tree
Showing 97 changed files with 351 additions and 57 deletions.
11 changes: 5 additions & 6 deletions TESTING.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -301,22 +301,21 @@ restResources {
}
---------------------------------------------------------------------------

YAML tests that include x-pack specific APIs need to explicitly declare
YAML REST tests that include x-pack specific APIs need to explicitly declare
which APIs are required through a similar `includeXpack` configuration.

The REST tests are run automatically when executing the "./gradlew check" command. To run only the
REST tests use the following command:
YAML REST tests use the following command (modules and plugins may also include YAML REST tests):

---------------------------------------------------------------------------
./gradlew :distribution:archives:integ-test-zip:integTestRunner \
--tests "org.elasticsearch.test.rest.IntegTestZipClientYamlTestSuiteIT"
./gradlew :rest-api-spec:yamlRestTest
---------------------------------------------------------------------------

A specific test case can be run with the following command:

---------------------------------------------------------------------------
./gradlew ':distribution:archives:integ-test-zip:integTestRunner' \
--tests "org.elasticsearch.test.rest.IntegTestZipClientYamlTestSuiteIT" \
./gradlew ':rest-api-spec:yamlRestTestRunner' \
--tests "org.elasticsearch.test.rest.ClientYamlTestSuiteIT" \
-Dtests.method="test {p0=cat.segments/10_basic/Help}"
---------------------------------------------------------------------------

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ import org.elasticsearch.gradle.NoticeTask
import org.elasticsearch.gradle.Version
import org.elasticsearch.gradle.VersionProperties
import org.elasticsearch.gradle.info.BuildParams
import org.elasticsearch.gradle.test.rest.RestResourcesPlugin
import org.elasticsearch.gradle.test.RestIntegTestTask
import org.elasticsearch.gradle.testclusters.RunTask
import org.elasticsearch.gradle.testclusters.TestClustersPlugin
Expand Down Expand Up @@ -56,7 +55,6 @@ class PluginBuildPlugin implements Plugin<Project> {
void apply(Project project) {
project.pluginManager.apply(BuildPlugin)
project.pluginManager.apply(TestClustersPlugin)
project.pluginManager.apply(RestResourcesPlugin)

PluginPropertiesExtension extension = project.extensions.create(PLUGIN_EXTENSION_NAME, PluginPropertiesExtension, project)
configureDependencies(project)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ import org.elasticsearch.gradle.ExportElasticsearchBuildResourcesTask
import org.elasticsearch.gradle.info.BuildParams
import org.elasticsearch.gradle.info.GlobalBuildInfoPlugin
import org.elasticsearch.gradle.precommit.PrecommitTasks
import org.elasticsearch.gradle.test.rest.RestResourcesPlugin
import org.elasticsearch.gradle.testclusters.TestClustersPlugin
import org.gradle.api.InvalidUserDataException
import org.gradle.api.Plugin
Expand Down Expand Up @@ -73,8 +72,6 @@ class StandaloneRestTestPlugin implements Plugin<Project> {
// only setup tests to build
SourceSetContainer sourceSets = project.extensions.getByType(SourceSetContainer)
SourceSet testSourceSet = sourceSets.create('test')
// need to apply plugin after test source sets are created
project.pluginManager.apply(RestResourcesPlugin)

project.tasks.withType(Test).configureEach { Test test ->
test.testClassesDirs = testSourceSet.output.classesDirs
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
import org.gradle.api.NamedDomainObjectContainer;
import org.gradle.api.Project;
import org.gradle.api.Task;
import org.gradle.api.tasks.Internal;

public class RestIntegTestTask extends DefaultTask {

Expand All @@ -36,6 +37,9 @@ public class RestIntegTestTask extends DefaultTask {
private static final String TESTS_CLUSTER = "tests.cluster";
private static final String TESTS_CLUSTER_NAME = "tests.clustername";

// TODO: refactor this so that work is not done in constructor and find usages and register them, not create them
// See: https://docs.gradle.org/current/userguide/task_configuration_avoidance.html
// See: https://github.com/elastic/elasticsearch/issues/47804
public RestIntegTestTask() {
Project project = getProject();
String name = getName();
Expand Down Expand Up @@ -101,4 +105,9 @@ public void setDependsOn(Iterable<?> dependencies) {
public void runner(Action<? super RestTestRunnerTask> configure) {
configure.execute(runner);
}

@Internal
public RestTestRunnerTask getRunner() {
return runner;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
import org.gradle.api.artifacts.Configuration;
import org.gradle.api.file.ConfigurableFileCollection;
import org.gradle.api.file.FileTree;
import org.gradle.api.plugins.JavaPluginConvention;
import org.gradle.api.provider.ListProperty;
import org.gradle.api.tasks.Input;
import org.gradle.api.tasks.InputFiles;
Expand All @@ -41,6 +42,8 @@
import java.io.File;
import java.io.IOException;
import java.nio.file.Files;
import java.util.Objects;
import java.util.Optional;
import java.util.Set;
import java.util.stream.Collectors;

Expand All @@ -54,7 +57,7 @@ public class CopyRestApiTask extends DefaultTask {
private static final String REST_API_PREFIX = "rest-api-spec/api";
final ListProperty<String> includeCore = getProject().getObjects().listProperty(String.class);
final ListProperty<String> includeXpack = getProject().getObjects().listProperty(String.class);

String sourceSetName;
Configuration coreConfig;
Configuration xpackConfig;

Expand All @@ -81,6 +84,11 @@ public ListProperty<String> getIncludeXpack() {
return includeXpack;
}

@Input
String getSourceSetName() {
return sourceSetName;
}

@SkipWhenEmpty
@InputFiles
public FileTree getInputDir() {
Expand Down Expand Up @@ -109,7 +117,12 @@ public FileTree getInputDir() {

@OutputDirectory
public File getOutputDir() {
return new File(getTestSourceSet().getOutput().getResourcesDir(), REST_API_PREFIX);
return new File(
getSourceSet().orElseThrow(() -> new IllegalArgumentException("could not find source set [" + sourceSetName + "]"))
.getOutput()
.getResourcesDir(),
REST_API_PREFIX
);
}

@TaskAction
Expand All @@ -131,7 +144,8 @@ void copy() {
);
project.copy(c -> {
c.from(project.zipTree(coreConfig.getSingleFile()));
c.into(getTestSourceSet().getOutput().getResourcesDir()); // this ends up as the same dir as outputDir
// this ends up as the same dir as outputDir
c.into(Objects.requireNonNull(getSourceSet().orElseThrow().getOutput().getResourcesDir()));
if (includeCore.get().isEmpty()) {
c.include(REST_API_PREFIX + "/**");
} else {
Expand Down Expand Up @@ -178,31 +192,33 @@ private boolean projectHasYamlRestTests() {
}

private File getTestSourceResourceDir() {
SourceSet testSources = getTestSourceSet();
if (testSources == null) {
return null;
}
Set<File> resourceDir = testSources.getResources()
.getSrcDirs()
.stream()
.filter(f -> f.isDirectory() && f.getParentFile().getName().equals("test") && f.getName().equals("resources"))
.collect(Collectors.toSet());
assert resourceDir.size() <= 1;
if (resourceDir.size() == 0) {
Optional<SourceSet> testSourceSet = getSourceSet();
if (testSourceSet.isPresent()) {
SourceSet testSources = testSourceSet.get();
Set<File> resourceDir = testSources.getResources()
.getSrcDirs()
.stream()
.filter(f -> f.isDirectory() && f.getParentFile().getName().equals("test") && f.getName().equals("resources"))
.collect(Collectors.toSet());
assert resourceDir.size() <= 1;
if (resourceDir.size() == 0) {
return null;
}
return resourceDir.iterator().next();
} else {
return null;
}
return resourceDir.iterator().next();
}

private File getTestOutputResourceDir() {
SourceSet testSources = getTestSourceSet();
if (testSources == null) {
return null;
}
return testSources.getOutput().getResourcesDir();
Optional<SourceSet> testSourceSet = getSourceSet();
return testSourceSet.map(sourceSet -> sourceSet.getOutput().getResourcesDir()).orElse(null);
}

private SourceSet getTestSourceSet() {
return GradleUtils.getJavaSourceSets(getProject()).findByName("test");
private Optional<SourceSet> getSourceSet() {
Project project = getProject();
return project.getConvention().findPlugin(JavaPluginConvention.class) == null
? Optional.empty()
: Optional.ofNullable(GradleUtils.getJavaSourceSets(project).findByName(getSourceSetName()));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
import org.gradle.api.artifacts.Configuration;
import org.gradle.api.file.ConfigurableFileCollection;
import org.gradle.api.file.FileTree;
import org.gradle.api.plugins.JavaPluginConvention;
import org.gradle.api.provider.ListProperty;
import org.gradle.api.tasks.Input;
import org.gradle.api.tasks.InputFiles;
Expand All @@ -39,6 +40,8 @@

import javax.inject.Inject;
import java.io.File;
import java.util.Objects;
import java.util.Optional;
import java.util.stream.Collectors;

/**
Expand All @@ -52,6 +55,7 @@ public class CopyRestTestsTask extends DefaultTask {
final ListProperty<String> includeCore = getProject().getObjects().listProperty(String.class);
final ListProperty<String> includeXpack = getProject().getObjects().listProperty(String.class);

String sourceSetName;
Configuration coreConfig;
Configuration xpackConfig;

Expand All @@ -78,6 +82,11 @@ public ListProperty<String> getIncludeXpack() {
return includeXpack;
}

@Input
String getSourceSetName() {
return sourceSetName;
}

@SkipWhenEmpty
@InputFiles
public FileTree getInputDir() {
Expand All @@ -103,7 +112,12 @@ public FileTree getInputDir() {

@OutputDirectory
public File getOutputDir() {
return new File(getTestSourceSet().getOutput().getResourcesDir(), REST_TEST_PREFIX);
return new File(
getSourceSet().orElseThrow(() -> new IllegalArgumentException("could not find source set [" + sourceSetName + "]"))
.getOutput()
.getResourcesDir(),
REST_TEST_PREFIX
);
}

@TaskAction
Expand All @@ -127,7 +141,8 @@ void copy() {
);
project.copy(c -> {
c.from(project.zipTree(coreConfig.getSingleFile()));
c.into(getTestSourceSet().getOutput().getResourcesDir()); // this ends up as the same dir as outputDir
// this ends up as the same dir as outputDir
c.into(Objects.requireNonNull(getSourceSet().orElseThrow().getOutput().getResourcesDir()));
c.include(
includeCore.get().stream().map(prefix -> REST_TEST_PREFIX + "/" + prefix + "*/**").collect(Collectors.toList())
);
Expand All @@ -145,7 +160,10 @@ void copy() {
}
}

private SourceSet getTestSourceSet() {
return GradleUtils.getJavaSourceSets(getProject()).findByName("test");
private Optional<SourceSet> getSourceSet() {
Project project = getProject();
return project.getConvention().findPlugin(JavaPluginConvention.class) == null
? Optional.empty()
: Optional.ofNullable(GradleUtils.getJavaSourceSets(project).findByName(getSourceSetName()));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@
import org.gradle.api.artifacts.Configuration;
import org.gradle.api.artifacts.Dependency;
import org.gradle.api.provider.Provider;
import org.gradle.api.tasks.SourceSet;
import org.gradle.api.tasks.SourceSetContainer;

import java.util.Map;

Expand Down Expand Up @@ -76,6 +78,7 @@
* Will copy any of the the x-pack tests that start with graph, and will copy the X-pack graph specification, as well as the full core
* Rest API specification.
*
* Additionally you can specify which sourceSetName resources should be copied to. The default is the yamlRestTest source set.
* @see CopyRestApiTask
* @see CopyRestTestsTask
*/
Expand All @@ -97,6 +100,7 @@ public void apply(Project project) {
task.includeCore.set(extension.restTests.getIncludeCore());
task.includeXpack.set(extension.restTests.getIncludeXpack());
task.coreConfig = testConfig;
task.sourceSetName = SourceSet.TEST_SOURCE_SET_NAME;
if (BuildParams.isInternal()) {
// core
Dependency restTestdependency = project.getDependencies()
Expand Down Expand Up @@ -127,6 +131,7 @@ public void apply(Project project) {
task.includeXpack.set(extension.restApi.getIncludeXpack());
task.dependsOn(copyRestYamlTestTask);
task.coreConfig = specConfig;
task.sourceSetName = SourceSet.TEST_SOURCE_SET_NAME;
if (BuildParams.isInternal()) {
Dependency restSpecDependency = project.getDependencies()
.project(Map.of("path", ":rest-api-spec", "configuration", "restSpecs"));
Expand All @@ -144,6 +149,12 @@ public void apply(Project project) {
task.dependsOn(task.coreConfig);
});

project.getTasks().named("processTestResources").configure(t -> t.dependsOn(copyRestYamlSpecTask));
project.afterEvaluate(p -> {
SourceSetContainer sourceSets = project.getExtensions().getByType(SourceSetContainer.class);
SourceSet testSourceSet = sourceSets.findByName(SourceSet.TEST_SOURCE_SET_NAME);
if (testSourceSet != null) {
project.getTasks().named(testSourceSet.getProcessResourcesTaskName()).configure(t -> t.dependsOn(copyRestYamlSpecTask));
}
});
}
}
Loading

0 comments on commit 604c6dd

Please sign in to comment.