Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Smarter copying of the rest specs and tests #52114

Merged
merged 27 commits into from
Feb 26, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
c7d36e0
Smarter copying of the rest specs and tests
jakelandis Feb 5, 2020
1e02fbe
fix autoscaling project (merge clash)
jakelandis Feb 9, 2020
02f95b8
fix graph tests
jakelandis Feb 9, 2020
7b81e65
fix xpack docs build
jakelandis Feb 10, 2020
f334d40
fix ml tests
jakelandis Feb 10, 2020
c1ee4ff
Merge branch 'master' into rest_spec_from_common_dir3
elasticmachine Feb 10, 2020
f43429c
Use lazy config, task, and review comments
jakelandis Feb 10, 2020
818e761
Merge branch 'master' into rest_spec_from_common_dir3
jakelandis Feb 12, 2020
bddf05b
change name of plugin
jakelandis Feb 12, 2020
0c1d164
fix hlrc test
jakelandis Feb 12, 2020
bbcde3a
fix some missed includes
jakelandis Feb 12, 2020
2dc1a3a
move to 1 extension, minor clean up
jakelandis Feb 12, 2020
8942ac2
split to two tasks
jakelandis Feb 13, 2020
2f5739c
fix smoke-test-security-with-mustache test
jakelandis Feb 13, 2020
907c0dd
fix some of the x-pack:qa tests
jakelandis Feb 13, 2020
173b494
fix another x-pack qa test
jakelandis Feb 13, 2020
6d0aec5
fix spotless
jakelandis Feb 13, 2020
6a74f0b
another qa xpack test fix
jakelandis Feb 13, 2020
a391dd7
fix another x-pack test (the prior iteration auto added all of x-pac…
jakelandis Feb 14, 2020
9583d15
clean up
jakelandis Feb 24, 2020
1617f6f
Merge remote-tracking branch 'upstream/master' into rest_spec_from_co…
jakelandis Feb 24, 2020
d86fc58
move logger to debug and remove incidental change
jakelandis Feb 24, 2020
852b30e
fix 3rd party config and remove uneeded copy
jakelandis Feb 25, 2020
10700bc
address review comments
jakelandis Feb 25, 2020
bb84161
fix checkStyle
jakelandis Feb 25, 2020
2e4d1d6
Merge remote-tracking branch 'upstream/master' into rest_spec_from_co…
jakelandis Feb 25, 2020
9e6e2aa
fix plugin name
jakelandis Feb 25, 2020
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ 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 @@ -51,6 +52,7 @@ 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 @@ -18,29 +18,19 @@
*/
package org.elasticsearch.gradle.test

import org.elasticsearch.gradle.VersionProperties
import org.elasticsearch.gradle.info.BuildParams
import org.elasticsearch.gradle.testclusters.ElasticsearchCluster
import org.elasticsearch.gradle.testclusters.RestTestRunnerTask
import org.elasticsearch.gradle.tool.Boilerplate
import org.gradle.api.DefaultTask
import org.gradle.api.Task
import org.gradle.api.file.FileCopyDetails
import org.gradle.api.tasks.Copy
import org.gradle.api.tasks.Input
import org.gradle.api.tasks.testing.Test
import org.gradle.plugins.ide.idea.IdeaPlugin

/**
* A wrapper task around setting up a cluster and running rest tests.
*/
class RestIntegTestTask extends DefaultTask {

protected Test runner

/** Flag indicating whether the rest tests in the rest spec should be run. */
@Input
Boolean includePackaged = false

RestIntegTestTask() {
runner = project.tasks.create("${name}Runner", RestTestRunnerTask.class)
super.dependsOn(runner)
Expand Down Expand Up @@ -69,10 +59,6 @@ class RestIntegTestTask extends DefaultTask {
runner.systemProperty('test.clustername', System.getProperty("tests.clustername"))
}

// copy the rest spec/tests onto the test classpath
Copy copyRestSpec = createCopyRestSpecTask()
project.sourceSets.test.output.builtBy(copyRestSpec)

// this must run after all projects have been configured, so we know any project
// references can be accessed as a fully configured
project.gradle.projectsEvaluated {
Expand All @@ -83,12 +69,6 @@ class RestIntegTestTask extends DefaultTask {
}
}

/** Sets the includePackaged property */
public void includePackaged(boolean include) {
includePackaged = include
}


@Override
public Task dependsOn(Object... dependencies) {
runner.dependsOn(dependencies)
Expand All @@ -114,37 +94,4 @@ class RestIntegTestTask extends DefaultTask {
project.tasks.getByName("${name}Runner").configure(configure)
}

Copy createCopyRestSpecTask() {
Boilerplate.maybeCreate(project.configurations, 'restSpec') {
project.dependencies.add(
'restSpec',
BuildParams.internal ? project.project(':rest-api-spec') :
"org.elasticsearch:rest-api-spec:${VersionProperties.elasticsearch}"
)
}

return Boilerplate.maybeCreate(project.tasks, 'copyRestSpec', Copy) { Copy copy ->
copy.dependsOn project.configurations.restSpec
copy.into(project.sourceSets.test.output.resourcesDir)
copy.from({ project.zipTree(project.configurations.restSpec.singleFile) }) {
includeEmptyDirs = false
include 'rest-api-spec/**'
filesMatching('rest-api-spec/test/**') { FileCopyDetails details ->
if (includePackaged == false) {
details.exclude()
}
}
}

if (project.plugins.hasPlugin(IdeaPlugin)) {
project.idea {
module {
if (scopes.TEST != null) {
scopes.TEST.plus.add(project.configurations.restSpec)
}
}
}
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,9 @@ 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.JavaVersion
import org.gradle.api.Plugin
import org.gradle.api.Project
import org.gradle.api.artifacts.Configuration
Expand All @@ -42,6 +42,7 @@ import org.gradle.api.tasks.compile.JavaCompile
import org.gradle.api.tasks.testing.Test
import org.gradle.plugins.ide.eclipse.model.EclipseModel
import org.gradle.plugins.ide.idea.model.IdeaModel

/**
* Configures the build to compile tests against Elasticsearch's test framework
* and run REST tests. Use BuildPlugin if you want to build main code as well
Expand Down Expand Up @@ -74,6 +75,8 @@ 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) { Test test ->
test.testClassesDirs = testSourceSet.output.classesDirs
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -250,7 +250,7 @@ public void doCheck() throws IOException {
Files.write(getSuccessMarker().toPath(), new byte[] {}, StandardOpenOption.CREATE);
} else {
getLogger().error(problems);
throw new IllegalStateException("Testing conventions are not honored");
throw new IllegalStateException(String.format("Testing conventions [%s] are not honored", problems));
}
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,195 @@
/*
* Licensed to Elasticsearch under one or more contributor
* license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright
* ownership. Elasticsearch licenses this file to you under
* the Apache License, Version 2.0 (the "License"); you may
* not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/
package org.elasticsearch.gradle.test.rest;

import org.elasticsearch.gradle.VersionProperties;
import org.elasticsearch.gradle.info.BuildParams;
import org.elasticsearch.gradle.tool.Boilerplate;
import org.gradle.api.DefaultTask;
import org.gradle.api.Project;
import org.gradle.api.artifacts.Configuration;
import org.gradle.api.file.ConfigurableFileCollection;
import org.gradle.api.file.FileTree;
import org.gradle.api.provider.ListProperty;
import org.gradle.api.tasks.Input;
import org.gradle.api.tasks.InputFiles;
import org.gradle.api.tasks.OutputDirectory;
import org.gradle.api.tasks.SkipWhenEmpty;
import org.gradle.api.tasks.SourceSet;
import org.gradle.api.tasks.TaskAction;
import org.gradle.api.tasks.util.PatternFilterable;
import org.gradle.api.tasks.util.PatternSet;
import org.gradle.internal.Factory;

import javax.inject.Inject;
import java.io.File;
import java.io.IOException;
import java.nio.file.Files;
import java.util.Set;
import java.util.stream.Collectors;

/**
* Copies the files needed for the Rest YAML specs to the current projects test resources output directory.
* This is intended to be be used from {@link RestResourcesPlugin} since the plugin wires up the needed
* configurations and custom extensions.
* @see RestResourcesPlugin
*/
public class CopyRestApiTask extends DefaultTask {
private static final String COPY_TO = "rest-api-spec/api";
final ListProperty<String> includeCore = getProject().getObjects().listProperty(String.class);
final ListProperty<String> includeXpack = getProject().getObjects().listProperty(String.class);

Configuration coreConfig;
Configuration xpackConfig;

private final PatternFilterable corePatternSet;
private final PatternFilterable xpackPatternSet;

public CopyRestApiTask() {
corePatternSet = getPatternSetFactory().create();
xpackPatternSet = getPatternSetFactory().create();
}

@Inject
protected Factory<PatternSet> getPatternSetFactory() {
throw new UnsupportedOperationException();
}

@Input
public ListProperty<String> getIncludeCore() {
return includeCore;
}

@Input
public ListProperty<String> getIncludeXpack() {
return includeXpack;
}

@SkipWhenEmpty
@InputFiles
public FileTree getInputDir() {
xpackPatternSet.setIncludes(includeXpack.get().stream().map(prefix -> prefix + "*/**").collect(Collectors.toList()));
ConfigurableFileCollection fileCollection = getProject().files(xpackConfig.getAsFileTree().matching(xpackPatternSet));
if (BuildParams.isInternal()) {
corePatternSet.setIncludes(includeCore.get().stream().map(prefix -> prefix + "*/**").collect(Collectors.toList()));
fileCollection.plus(coreConfig.getAsFileTree().matching(corePatternSet));
} else {
fileCollection.plus(coreConfig);
}
// if project has rest tests or the includes are explicitly configured execute the task, else NO-SOURCE due to the null input
jakelandis marked this conversation as resolved.
Show resolved Hide resolved
return projectHasYamlRestTests() || includeCore.get().isEmpty() == false || includeXpack.get().isEmpty() == false
? fileCollection.getAsFileTree()
: null;
}

@OutputDirectory
public File getOutputDir() {
jakelandis marked this conversation as resolved.
Show resolved Hide resolved
return new File(getTestSourceSet().getOutput().getResourcesDir(), COPY_TO);
}

@TaskAction
void copy() {
Project project = getProject();
// always copy the core specs if the task executes
if (BuildParams.isInternal()) {
getLogger().debug("Rest specs for project [{}] will be copied to the test resources.", project.getPath());
project.copy(c -> {
c.from(coreConfig.getSingleFile());
c.into(getOutputDir());
c.include(corePatternSet.getIncludes());
});
} else {
getLogger().debug(
"Rest specs for project [{}] will be copied to the test resources from the published jar (version: [{}]).",
project.getPath(),
VersionProperties.getElasticsearch()
);
project.copy(c -> {
c.from(project.zipTree(coreConfig.getSingleFile()));
c.into(getTestSourceSet().getOutput().getResourcesDir()); // this ends up as the same dir as outputDir
c.include(includeCore.get().stream().map(prefix -> COPY_TO + "/" + prefix + "*/**").collect(Collectors.toList()));
});
}
// only copy x-pack specs if explicitly instructed
if (includeXpack.get().isEmpty() == false) {
getLogger().debug("X-pack rest specs for project [{}] will be copied to the test resources.", project.getPath());
project.copy(c -> {
c.from(xpackConfig.getSingleFile());
c.into(getOutputDir());
c.include(xpackPatternSet.getIncludes());
});
}
}

/**
* Returns true if any files with a .yml extension exist the test resources rest-api-spec/test directory (from source or output dir)
*/
private boolean projectHasYamlRestTests() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should live in the plugin not in the task.

Copy link
Contributor Author

@jakelandis jakelandis Feb 13, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, thinking about this a bit more, I think this needs to stay in the task for a couple reasons:

  1. This method checks against the source dir for tests committed to GH, but also checks the output directory for tests copied over by prior gradle tasks. This is why the spec task depends on the test task. Since this evaluation is done in the task at execution time, I believe this is a reliable way to ensure that any source or copied tests exist.

  2. We can not actually skip the creation of the task based on the output of this. The spec copy task needs to execute if the project has no rest tests, but also you requested to include some tests, and the IIUC the extension property isn't guaranteed to be resolved at configuration time. e.g. since we can not read the property at configuration time we will always need to create the task in case there are no tests at all, but you need the spec (docs and HLRC are examples of this).

File testSourceResourceDir = getTestSourceResourceDir();
File testOutputResourceDir = getTestOutputResourceDir(); // check output for cases where tests are copied programmatically

if (testSourceResourceDir == null && testOutputResourceDir == null) {
return false;
}
try {
if (testSourceResourceDir != null) {
return new File(testSourceResourceDir, "rest-api-spec/test").exists() == false
|| Files.walk(testSourceResourceDir.toPath().resolve("rest-api-spec/test"))
.anyMatch(p -> p.getFileName().toString().endsWith("yml"));
}
if (testOutputResourceDir != null) {
return new File(testOutputResourceDir, "rest-api-spec/test").exists() == false
|| Files.walk(testOutputResourceDir.toPath().resolve("rest-api-spec/test"))
.anyMatch(p -> p.getFileName().toString().endsWith("yml"));
}
} catch (IOException e) {
throw new IllegalStateException(String.format("Error determining if this project [%s] has rest tests.", getProject()), e);
}
return false;
}

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) {
return null;
}
return resourceDir.iterator().next();
}

private File getTestOutputResourceDir() {
SourceSet testSources = getTestSourceSet();
if (testSources == null) {
return null;
}
return testSources.getOutput().getResourcesDir();
}

private SourceSet getTestSourceSet() {
return Boilerplate.getJavaSourceSets(getProject()).findByName("test");
}
}
Loading