Skip to content

Commit

Permalink
Fix incremental build support for copying REST api and tests (elastic…
Browse files Browse the repository at this point in the history
…#52860)

A recent PR elastic#52114 introduced two new tasks to copy the REST api and tests.
A couple bugs were found in that initial PR that prevents the incremental
build from working as expected.

The pattern match of empty string is equivalent to match all and it was coded
as match none. Fixed with explicit checks against empty patterns.

The fileCollection.plus return value was ignored. Fixed by changing how the
input's fileTree is constructed.

If a project has an src/test/resources directory, and tests are being copied
without a rest-api-spec/test directory could result no-op. Masked by the other
bugs and fixed by minor changes to logic to determine if a project has tests.
  • Loading branch information
jakelandis committed Feb 27, 2020
1 parent 5e1ea07 commit 4738c39
Show file tree
Hide file tree
Showing 2 changed files with 38 additions and 23 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -84,16 +84,25 @@ public ListProperty<String> getIncludeXpack() {
@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);
FileTree coreFileTree = null;
FileTree xpackFileTree = null;
if (includeXpack.get().isEmpty() == false) {
xpackPatternSet.setIncludes(includeXpack.get().stream().map(prefix -> prefix + "*/**").collect(Collectors.toList()));
xpackFileTree = xpackConfig.getAsFileTree().matching(xpackPatternSet);
}
boolean projectHasYamlRestTests = projectHasYamlRestTests();
if (includeCore.get().isEmpty() == false || projectHasYamlRestTests) {
if (BuildParams.isInternal()) {
corePatternSet.setIncludes(includeCore.get().stream().map(prefix -> prefix + "*/**").collect(Collectors.toList()));
coreFileTree = coreConfig.getAsFileTree().matching(corePatternSet); // directory on disk
} else {
coreFileTree = coreConfig.getAsFileTree(); // jar file
}
}
ConfigurableFileCollection fileCollection = getProject().files(coreFileTree, xpackFileTree);

// if project has rest tests or the includes are explicitly configured execute the task, else NO-SOURCE due to the null input
return projectHasYamlRestTests() || includeCore.get().isEmpty() == false || includeXpack.get().isEmpty() == false
return projectHasYamlRestTests || includeCore.get().isEmpty() == false || includeXpack.get().isEmpty() == false
? fileCollection.getAsFileTree()
: null;
}
Expand Down Expand Up @@ -148,15 +157,13 @@ private boolean projectHasYamlRestTests() {
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 (testSourceResourceDir != null && new File(testSourceResourceDir, "rest-api-spec/test").exists()) {
return 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"));
if (testOutputResourceDir != null && new File(testOutputResourceDir, "rest-api-spec/test").exists()) {
return 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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -81,14 +81,22 @@ public ListProperty<String> getIncludeXpack() {
@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);
FileTree coreFileTree = null;
FileTree xpackFileTree = null;
if (includeXpack.get().isEmpty() == false) {
xpackPatternSet.setIncludes(includeXpack.get().stream().map(prefix -> prefix + "*/**").collect(Collectors.toList()));
xpackFileTree = xpackConfig.getAsFileTree().matching(xpackPatternSet);
}
if (includeCore.get().isEmpty() == false) {
if (BuildParams.isInternal()) {
corePatternSet.setIncludes(includeCore.get().stream().map(prefix -> prefix + "*/**").collect(Collectors.toList()));
coreFileTree = coreConfig.getAsFileTree().matching(corePatternSet); // directory on disk
} else {
coreFileTree = coreConfig.getAsFileTree(); // jar file
}
}
ConfigurableFileCollection fileCollection = getProject().files(coreFileTree, xpackFileTree);

// copy tests only if explicitly requested
return includeCore.get().isEmpty() == false || includeXpack.get().isEmpty() == false ? fileCollection.getAsFileTree() : null;
}
Expand Down

0 comments on commit 4738c39

Please sign in to comment.