From 4738c39e3d1b4db44c4e46f091a8015eeac18374 Mon Sep 17 00:00:00 2001 From: Jake Landis Date: Thu, 27 Feb 2020 11:00:13 -0600 Subject: [PATCH] Fix incremental build support for copying REST api and tests (#52860) A recent PR #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. --- .../gradle/test/rest/CopyRestApiTask.java | 39 +++++++++++-------- .../gradle/test/rest/CopyRestTestsTask.java | 22 +++++++---- 2 files changed, 38 insertions(+), 23 deletions(-) diff --git a/buildSrc/src/main/java/org/elasticsearch/gradle/test/rest/CopyRestApiTask.java b/buildSrc/src/main/java/org/elasticsearch/gradle/test/rest/CopyRestApiTask.java index 6e66df54dc48d..810762a08e87e 100644 --- a/buildSrc/src/main/java/org/elasticsearch/gradle/test/rest/CopyRestApiTask.java +++ b/buildSrc/src/main/java/org/elasticsearch/gradle/test/rest/CopyRestApiTask.java @@ -84,16 +84,25 @@ public ListProperty 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; } @@ -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); diff --git a/buildSrc/src/main/java/org/elasticsearch/gradle/test/rest/CopyRestTestsTask.java b/buildSrc/src/main/java/org/elasticsearch/gradle/test/rest/CopyRestTestsTask.java index 2fd5a207482a4..046bb5a6d1a8a 100644 --- a/buildSrc/src/main/java/org/elasticsearch/gradle/test/rest/CopyRestTestsTask.java +++ b/buildSrc/src/main/java/org/elasticsearch/gradle/test/rest/CopyRestTestsTask.java @@ -81,14 +81,22 @@ public ListProperty 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; }