Skip to content

Commit 810dc9f

Browse files
authored
Smarter copying of the rest specs and tests (#52114)
This PR addresses the unnecessary copying of the rest specs and allows for better semantics for which specs and tests are copied. By default the rest specs will get copied if the project applies `elasticsearch.standalone-rest-test` or `esplugin` and the project has rest tests or you configure the custom extension `restResources`. This PR also removes the need for dozens of places where the x-pack specs were copied by supporting copying of the x-pack rest specs too. The plugin/task introduced here can also copy the rest tests to the local project through a similar configuration. The new plugin/task allows a user to minimize the surface area of which rest specs are copied. Per project can be configured to include only a subset of the specs (or tests). Configuring a project to only copy the specs when actually needed should help with build cache hit rates since we can better define what is actually in use. However, project level optimizations for build cache hit rates are not included with this PR. Also, with this PR you can no longer use the includePackaged flag on integTest task. The following items are included in this PR: * new plugin: `elasticsearch.rest-resources` * new tasks: CopyRestApiTask and CopyRestTestsTask - performs the copy * new extension 'restResources' ``` restResources { restApi { includeCore 'foo' , 'bar' //will include the core specs that start with foo and bar includeXpack 'baz' //will include x-pack specs that start with baz } restTests { includeCore 'foo', 'bar' //will include the core tests that start with foo and bar includeXpack 'baz' //will include the x-pack tests that start with baz } } ```
1 parent 18f5e5a commit 810dc9f

File tree

49 files changed

+743
-346
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

49 files changed

+743
-346
lines changed

buildSrc/src/main/groovy/org/elasticsearch/gradle/plugin/PluginBuildPlugin.groovy

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ import org.elasticsearch.gradle.NoticeTask
2424
import org.elasticsearch.gradle.Version
2525
import org.elasticsearch.gradle.VersionProperties
2626
import org.elasticsearch.gradle.info.BuildParams
27+
import org.elasticsearch.gradle.test.rest.RestResourcesPlugin
2728
import org.elasticsearch.gradle.test.RestIntegTestTask
2829
import org.elasticsearch.gradle.testclusters.RunTask
2930
import org.elasticsearch.gradle.testclusters.TestClustersPlugin
@@ -51,6 +52,7 @@ class PluginBuildPlugin implements Plugin<Project> {
5152
void apply(Project project) {
5253
project.pluginManager.apply(BuildPlugin)
5354
project.pluginManager.apply(TestClustersPlugin)
55+
project.pluginManager.apply(RestResourcesPlugin)
5456

5557
PluginPropertiesExtension extension = project.extensions.create(PLUGIN_EXTENSION_NAME, PluginPropertiesExtension, project)
5658
configureDependencies(project)

buildSrc/src/main/groovy/org/elasticsearch/gradle/test/RestIntegTestTask.groovy

Lines changed: 1 addition & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -18,29 +18,19 @@
1818
*/
1919
package org.elasticsearch.gradle.test
2020

21-
import org.elasticsearch.gradle.VersionProperties
22-
import org.elasticsearch.gradle.info.BuildParams
2321
import org.elasticsearch.gradle.testclusters.ElasticsearchCluster
2422
import org.elasticsearch.gradle.testclusters.RestTestRunnerTask
25-
import org.elasticsearch.gradle.tool.Boilerplate
2623
import org.gradle.api.DefaultTask
2724
import org.gradle.api.Task
28-
import org.gradle.api.file.FileCopyDetails
29-
import org.gradle.api.tasks.Copy
30-
import org.gradle.api.tasks.Input
3125
import org.gradle.api.tasks.testing.Test
32-
import org.gradle.plugins.ide.idea.IdeaPlugin
26+
3327
/**
3428
* A wrapper task around setting up a cluster and running rest tests.
3529
*/
3630
class RestIntegTestTask extends DefaultTask {
3731

3832
protected Test runner
3933

40-
/** Flag indicating whether the rest tests in the rest spec should be run. */
41-
@Input
42-
Boolean includePackaged = false
43-
4434
RestIntegTestTask() {
4535
runner = project.tasks.create("${name}Runner", RestTestRunnerTask.class)
4636
super.dependsOn(runner)
@@ -69,10 +59,6 @@ class RestIntegTestTask extends DefaultTask {
6959
runner.systemProperty('test.clustername', System.getProperty("tests.clustername"))
7060
}
7161

72-
// copy the rest spec/tests onto the test classpath
73-
Copy copyRestSpec = createCopyRestSpecTask()
74-
project.sourceSets.test.output.builtBy(copyRestSpec)
75-
7662
// this must run after all projects have been configured, so we know any project
7763
// references can be accessed as a fully configured
7864
project.gradle.projectsEvaluated {
@@ -83,12 +69,6 @@ class RestIntegTestTask extends DefaultTask {
8369
}
8470
}
8571

86-
/** Sets the includePackaged property */
87-
public void includePackaged(boolean include) {
88-
includePackaged = include
89-
}
90-
91-
9272
@Override
9373
public Task dependsOn(Object... dependencies) {
9474
runner.dependsOn(dependencies)
@@ -114,37 +94,4 @@ class RestIntegTestTask extends DefaultTask {
11494
project.tasks.getByName("${name}Runner").configure(configure)
11595
}
11696

117-
Copy createCopyRestSpecTask() {
118-
Boilerplate.maybeCreate(project.configurations, 'restSpec') {
119-
project.dependencies.add(
120-
'restSpec',
121-
BuildParams.internal ? project.project(':rest-api-spec') :
122-
"org.elasticsearch:rest-api-spec:${VersionProperties.elasticsearch}"
123-
)
124-
}
125-
126-
return Boilerplate.maybeCreate(project.tasks, 'copyRestSpec', Copy) { Copy copy ->
127-
copy.dependsOn project.configurations.restSpec
128-
copy.into(project.sourceSets.test.output.resourcesDir)
129-
copy.from({ project.zipTree(project.configurations.restSpec.singleFile) }) {
130-
includeEmptyDirs = false
131-
include 'rest-api-spec/**'
132-
filesMatching('rest-api-spec/test/**') { FileCopyDetails details ->
133-
if (includePackaged == false) {
134-
details.exclude()
135-
}
136-
}
137-
}
138-
139-
if (project.plugins.hasPlugin(IdeaPlugin)) {
140-
project.idea {
141-
module {
142-
if (scopes.TEST != null) {
143-
scopes.TEST.plus.add(project.configurations.restSpec)
144-
}
145-
}
146-
}
147-
}
148-
}
149-
}
15097
}

buildSrc/src/main/groovy/org/elasticsearch/gradle/test/StandaloneRestTestPlugin.groovy

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,9 +26,9 @@ import org.elasticsearch.gradle.ExportElasticsearchBuildResourcesTask
2626
import org.elasticsearch.gradle.info.BuildParams
2727
import org.elasticsearch.gradle.info.GlobalBuildInfoPlugin
2828
import org.elasticsearch.gradle.precommit.PrecommitTasks
29+
import org.elasticsearch.gradle.test.rest.RestResourcesPlugin
2930
import org.elasticsearch.gradle.testclusters.TestClustersPlugin
3031
import org.gradle.api.InvalidUserDataException
31-
import org.gradle.api.JavaVersion
3232
import org.gradle.api.Plugin
3333
import org.gradle.api.Project
3434
import org.gradle.api.artifacts.Configuration
@@ -42,6 +42,7 @@ import org.gradle.api.tasks.compile.JavaCompile
4242
import org.gradle.api.tasks.testing.Test
4343
import org.gradle.plugins.ide.eclipse.model.EclipseModel
4444
import org.gradle.plugins.ide.idea.model.IdeaModel
45+
4546
/**
4647
* Configures the build to compile tests against Elasticsearch's test framework
4748
* and run REST tests. Use BuildPlugin if you want to build main code as well
@@ -74,6 +75,8 @@ class StandaloneRestTestPlugin implements Plugin<Project> {
7475
// only setup tests to build
7576
SourceSetContainer sourceSets = project.extensions.getByType(SourceSetContainer)
7677
SourceSet testSourceSet = sourceSets.create('test')
78+
// need to apply plugin after test source sets are created
79+
project.pluginManager.apply(RestResourcesPlugin)
7780

7881
project.tasks.withType(Test) { Test test ->
7982
test.testClassesDirs = testSourceSet.output.classesDirs

buildSrc/src/main/java/org/elasticsearch/gradle/precommit/TestingConventionsTasks.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -250,7 +250,7 @@ public void doCheck() throws IOException {
250250
Files.write(getSuccessMarker().toPath(), new byte[] {}, StandardOpenOption.CREATE);
251251
} else {
252252
getLogger().error(problems);
253-
throw new IllegalStateException("Testing conventions are not honored");
253+
throw new IllegalStateException(String.format("Testing conventions [%s] are not honored", problems));
254254
}
255255
}
256256

Lines changed: 195 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,195 @@
1+
/*
2+
* Licensed to Elasticsearch under one or more contributor
3+
* license agreements. See the NOTICE file distributed with
4+
* this work for additional information regarding copyright
5+
* ownership. Elasticsearch licenses this file to you under
6+
* the Apache License, Version 2.0 (the "License"); you may
7+
* not use this file except in compliance with the License.
8+
* You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing,
13+
* software distributed under the License is distributed on an
14+
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
15+
* KIND, either express or implied. See the License for the
16+
* specific language governing permissions and limitations
17+
* under the License.
18+
*/
19+
package org.elasticsearch.gradle.test.rest;
20+
21+
import org.elasticsearch.gradle.VersionProperties;
22+
import org.elasticsearch.gradle.info.BuildParams;
23+
import org.elasticsearch.gradle.tool.Boilerplate;
24+
import org.gradle.api.DefaultTask;
25+
import org.gradle.api.Project;
26+
import org.gradle.api.artifacts.Configuration;
27+
import org.gradle.api.file.ConfigurableFileCollection;
28+
import org.gradle.api.file.FileTree;
29+
import org.gradle.api.provider.ListProperty;
30+
import org.gradle.api.tasks.Input;
31+
import org.gradle.api.tasks.InputFiles;
32+
import org.gradle.api.tasks.OutputDirectory;
33+
import org.gradle.api.tasks.SkipWhenEmpty;
34+
import org.gradle.api.tasks.SourceSet;
35+
import org.gradle.api.tasks.TaskAction;
36+
import org.gradle.api.tasks.util.PatternFilterable;
37+
import org.gradle.api.tasks.util.PatternSet;
38+
import org.gradle.internal.Factory;
39+
40+
import javax.inject.Inject;
41+
import java.io.File;
42+
import java.io.IOException;
43+
import java.nio.file.Files;
44+
import java.util.Set;
45+
import java.util.stream.Collectors;
46+
47+
/**
48+
* Copies the files needed for the Rest YAML specs to the current projects test resources output directory.
49+
* This is intended to be be used from {@link RestResourcesPlugin} since the plugin wires up the needed
50+
* configurations and custom extensions.
51+
* @see RestResourcesPlugin
52+
*/
53+
public class CopyRestApiTask extends DefaultTask {
54+
private static final String COPY_TO = "rest-api-spec/api";
55+
final ListProperty<String> includeCore = getProject().getObjects().listProperty(String.class);
56+
final ListProperty<String> includeXpack = getProject().getObjects().listProperty(String.class);
57+
58+
Configuration coreConfig;
59+
Configuration xpackConfig;
60+
61+
private final PatternFilterable corePatternSet;
62+
private final PatternFilterable xpackPatternSet;
63+
64+
public CopyRestApiTask() {
65+
corePatternSet = getPatternSetFactory().create();
66+
xpackPatternSet = getPatternSetFactory().create();
67+
}
68+
69+
@Inject
70+
protected Factory<PatternSet> getPatternSetFactory() {
71+
throw new UnsupportedOperationException();
72+
}
73+
74+
@Input
75+
public ListProperty<String> getIncludeCore() {
76+
return includeCore;
77+
}
78+
79+
@Input
80+
public ListProperty<String> getIncludeXpack() {
81+
return includeXpack;
82+
}
83+
84+
@SkipWhenEmpty
85+
@InputFiles
86+
public FileTree getInputDir() {
87+
xpackPatternSet.setIncludes(includeXpack.get().stream().map(prefix -> prefix + "*/**").collect(Collectors.toList()));
88+
ConfigurableFileCollection fileCollection = getProject().files(xpackConfig.getAsFileTree().matching(xpackPatternSet));
89+
if (BuildParams.isInternal()) {
90+
corePatternSet.setIncludes(includeCore.get().stream().map(prefix -> prefix + "*/**").collect(Collectors.toList()));
91+
fileCollection.plus(coreConfig.getAsFileTree().matching(corePatternSet));
92+
} else {
93+
fileCollection.plus(coreConfig);
94+
}
95+
// if project has rest tests or the includes are explicitly configured execute the task, else NO-SOURCE due to the null input
96+
return projectHasYamlRestTests() || includeCore.get().isEmpty() == false || includeXpack.get().isEmpty() == false
97+
? fileCollection.getAsFileTree()
98+
: null;
99+
}
100+
101+
@OutputDirectory
102+
public File getOutputDir() {
103+
return new File(getTestSourceSet().getOutput().getResourcesDir(), COPY_TO);
104+
}
105+
106+
@TaskAction
107+
void copy() {
108+
Project project = getProject();
109+
// always copy the core specs if the task executes
110+
if (BuildParams.isInternal()) {
111+
getLogger().debug("Rest specs for project [{}] will be copied to the test resources.", project.getPath());
112+
project.copy(c -> {
113+
c.from(coreConfig.getSingleFile());
114+
c.into(getOutputDir());
115+
c.include(corePatternSet.getIncludes());
116+
});
117+
} else {
118+
getLogger().debug(
119+
"Rest specs for project [{}] will be copied to the test resources from the published jar (version: [{}]).",
120+
project.getPath(),
121+
VersionProperties.getElasticsearch()
122+
);
123+
project.copy(c -> {
124+
c.from(project.zipTree(coreConfig.getSingleFile()));
125+
c.into(getTestSourceSet().getOutput().getResourcesDir()); // this ends up as the same dir as outputDir
126+
c.include(includeCore.get().stream().map(prefix -> COPY_TO + "/" + prefix + "*/**").collect(Collectors.toList()));
127+
});
128+
}
129+
// only copy x-pack specs if explicitly instructed
130+
if (includeXpack.get().isEmpty() == false) {
131+
getLogger().debug("X-pack rest specs for project [{}] will be copied to the test resources.", project.getPath());
132+
project.copy(c -> {
133+
c.from(xpackConfig.getSingleFile());
134+
c.into(getOutputDir());
135+
c.include(xpackPatternSet.getIncludes());
136+
});
137+
}
138+
}
139+
140+
/**
141+
* Returns true if any files with a .yml extension exist the test resources rest-api-spec/test directory (from source or output dir)
142+
*/
143+
private boolean projectHasYamlRestTests() {
144+
File testSourceResourceDir = getTestSourceResourceDir();
145+
File testOutputResourceDir = getTestOutputResourceDir(); // check output for cases where tests are copied programmatically
146+
147+
if (testSourceResourceDir == null && testOutputResourceDir == null) {
148+
return false;
149+
}
150+
try {
151+
if (testSourceResourceDir != null) {
152+
return new File(testSourceResourceDir, "rest-api-spec/test").exists() == false
153+
|| Files.walk(testSourceResourceDir.toPath().resolve("rest-api-spec/test"))
154+
.anyMatch(p -> p.getFileName().toString().endsWith("yml"));
155+
}
156+
if (testOutputResourceDir != null) {
157+
return new File(testOutputResourceDir, "rest-api-spec/test").exists() == false
158+
|| Files.walk(testOutputResourceDir.toPath().resolve("rest-api-spec/test"))
159+
.anyMatch(p -> p.getFileName().toString().endsWith("yml"));
160+
}
161+
} catch (IOException e) {
162+
throw new IllegalStateException(String.format("Error determining if this project [%s] has rest tests.", getProject()), e);
163+
}
164+
return false;
165+
}
166+
167+
private File getTestSourceResourceDir() {
168+
SourceSet testSources = getTestSourceSet();
169+
if (testSources == null) {
170+
return null;
171+
}
172+
Set<File> resourceDir = testSources.getResources()
173+
.getSrcDirs()
174+
.stream()
175+
.filter(f -> f.isDirectory() && f.getParentFile().getName().equals("test") && f.getName().equals("resources"))
176+
.collect(Collectors.toSet());
177+
assert resourceDir.size() <= 1;
178+
if (resourceDir.size() == 0) {
179+
return null;
180+
}
181+
return resourceDir.iterator().next();
182+
}
183+
184+
private File getTestOutputResourceDir() {
185+
SourceSet testSources = getTestSourceSet();
186+
if (testSources == null) {
187+
return null;
188+
}
189+
return testSources.getOutput().getResourcesDir();
190+
}
191+
192+
private SourceSet getTestSourceSet() {
193+
return Boilerplate.getJavaSourceSets(getProject()).findByName("test");
194+
}
195+
}

0 commit comments

Comments
 (0)