From fc73540aacc0e73a2fe878dbf4f25ae59bcb485e Mon Sep 17 00:00:00 2001 From: Robert Muir Date: Fri, 15 May 2015 16:15:23 -0400 Subject: [PATCH] include rest tests in test-framework.jar Closes #11192 which I accidentally already closed. Squashed commit of the following: commit f23faccddc2a77a880841da4c89c494edaa2aa46 Author: Robert Muir Date: Fri May 15 16:04:55 2015 -0400 Simplify this FileUtils even more: its either from the filesystem, or the classpath, not both. Its already trying 4 different combinations of crazy paths for either of these anyway. commit c7016c8a2b5a6043e2ded4b48b160821ba196974 Author: Robert Muir Date: Fri May 15 14:21:37 2015 -0400 include rest tests in test-framework jar --- pom.xml | 4 +- .../test/rest/ElasticsearchRestTestCase.java | 81 +++++++++++++++---- .../test/rest/spec/RestSpec.java | 6 +- .../test/rest/support/FileUtils.java | 79 ++++++++++-------- .../test/rest/test/FileUtilsTests.java | 26 +++--- 5 files changed, 130 insertions(+), 66 deletions(-) diff --git a/pom.xml b/pom.xml index 4b36f6c44ef..61966eb58a5 100644 --- a/pom.xml +++ b/pom.xml @@ -787,8 +787,6 @@ org/elasticsearch/test/test/**/* - - true @@ -1519,6 +1517,7 @@ dev-tools/forbidden/all-signatures.txt ${forbidden.test.signatures} + **.SuppressForbidden test-compile @@ -1539,6 +1538,7 @@ + rest-api-spec/**/* org/elasticsearch/test/**/* org/elasticsearch/bootstrap/BootstrapForTesting.class org/elasticsearch/common/cli/CliToolTestCase.class diff --git a/src/test/java/org/elasticsearch/test/rest/ElasticsearchRestTestCase.java b/src/test/java/org/elasticsearch/test/rest/ElasticsearchRestTestCase.java index b7b207a6b11..9661e3950c6 100644 --- a/src/test/java/org/elasticsearch/test/rest/ElasticsearchRestTestCase.java +++ b/src/test/java/org/elasticsearch/test/rest/ElasticsearchRestTestCase.java @@ -23,11 +23,14 @@ import com.carrotsearch.randomizedtesting.RandomizedTest; import com.carrotsearch.randomizedtesting.annotations.TestGroup; import com.carrotsearch.randomizedtesting.annotations.TimeoutSuite; import com.google.common.collect.Lists; + import org.apache.lucene.util.LuceneTestCase.Slow; import org.apache.lucene.util.LuceneTestCase.SuppressCodecs; import org.apache.lucene.util.LuceneTestCase.SuppressFsync; +import org.apache.lucene.util.IOUtils; import org.apache.lucene.util.TimeUnits; import org.elasticsearch.common.Strings; +import org.elasticsearch.common.SuppressForbidden; import org.elasticsearch.common.io.PathUtils; import org.elasticsearch.common.settings.ImmutableSettings; import org.elasticsearch.common.settings.Settings; @@ -48,9 +51,17 @@ import org.junit.BeforeClass; import org.junit.Test; import java.io.IOException; +import java.io.InputStream; import java.lang.annotation.*; +import java.net.URI; +import java.net.URISyntaxException; +import java.net.URL; +import java.nio.file.FileSystem; +import java.nio.file.FileSystems; +import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.PathMatcher; +import java.nio.file.StandardCopyOption; import java.util.*; /** @@ -158,23 +169,29 @@ public abstract class ElasticsearchRestTestCase extends ElasticsearchIntegration } private static List collectTestCandidates(int id, int count) throws RestTestParseException, IOException { - String[] paths = resolvePathsProperty(REST_TESTS_SUITE, DEFAULT_TESTS_PATH); - Map> yamlSuites = FileUtils.findYamlSuites(DEFAULT_TESTS_PATH, paths); - List testCandidates = Lists.newArrayList(); - RestTestSuiteParser restTestSuiteParser = new RestTestSuiteParser(); - //yaml suites are grouped by directory (effectively by api) - for (String api : yamlSuites.keySet()) { - List yamlFiles = Lists.newArrayList(yamlSuites.get(api)); - for (Path yamlFile : yamlFiles) { - String key = api + yamlFile.getFileName().toString(); - if (mustExecute(key, id, count)) { - RestTestSuite restTestSuite = restTestSuiteParser.parse(api, yamlFile); - for (TestSection testSection : restTestSuite.getTestSections()) { - testCandidates.add(new RestTestCandidate(restTestSuite, testSection)); + FileSystem fileSystem = getFileSystem(); + // don't make a try-with, getFileSystem returns null + // ... and you can't close() the default filesystem + try { + String[] paths = resolvePathsProperty(REST_TESTS_SUITE, DEFAULT_TESTS_PATH); + Map> yamlSuites = FileUtils.findYamlSuites(fileSystem, DEFAULT_TESTS_PATH, paths); + RestTestSuiteParser restTestSuiteParser = new RestTestSuiteParser(); + //yaml suites are grouped by directory (effectively by api) + for (String api : yamlSuites.keySet()) { + List yamlFiles = Lists.newArrayList(yamlSuites.get(api)); + for (Path yamlFile : yamlFiles) { + String key = api + yamlFile.getFileName().toString(); + if (mustExecute(key, id, count)) { + RestTestSuite restTestSuite = restTestSuiteParser.parse(api, yamlFile); + for (TestSection testSection : restTestSuite.getTestSections()) { + testCandidates.add(new RestTestCandidate(restTestSuite, testSection)); + } } } } + } finally { + IOUtils.close(fileSystem); } //sort the candidates so they will always be in the same order before being shuffled, for repeatability @@ -202,10 +219,46 @@ public abstract class ElasticsearchRestTestCase extends ElasticsearchIntegration } } + /** + * Returns a new FileSystem to read REST resources, or null if they + * are available from classpath. + */ + @SuppressForbidden(reason = "proper use of URL, hack around a JDK bug") + static FileSystem getFileSystem() throws IOException { + // REST suite handling is currently complicated, with lots of filtering and so on + // For now, to work embedded in a jar, return a ZipFileSystem over the jar contents. + URL codeLocation = FileUtils.class.getProtectionDomain().getCodeSource().getLocation(); + + if (codeLocation.getFile().endsWith(".jar")) { + try { + // hack around a bug in the zipfilesystem implementation before java 9, + // its checkWritable was incorrect and it won't work without write permissions. + // if we add the permission, it will open jars r/w, which is too scary! so copy to a safe r-w location. + Path tmp = Files.createTempFile(null, ".jar"); + try (InputStream in = codeLocation.openStream()) { + Files.copy(in, tmp, StandardCopyOption.REPLACE_EXISTING); + } + return FileSystems.newFileSystem(new URI("jar:" + tmp.toUri()), Collections.emptyMap()); + } catch (URISyntaxException e) { + throw new IOException("couldn't open zipfilesystem: ", e); + } + } else { + return null; + } + } + @BeforeClass public static void initExecutionContext() throws IOException, RestException { String[] specPaths = resolvePathsProperty(REST_TESTS_SPEC, DEFAULT_SPEC_PATH); - RestSpec restSpec = RestSpec.parseFrom(DEFAULT_SPEC_PATH, specPaths); + RestSpec restSpec = null; + FileSystem fileSystem = getFileSystem(); + // don't make a try-with, getFileSystem returns null + // ... and you can't close() the default filesystem + try { + restSpec = RestSpec.parseFrom(fileSystem, DEFAULT_SPEC_PATH, specPaths); + } finally { + IOUtils.close(fileSystem); + } validateSpec(restSpec); restTestExecutionContext = new RestTestExecutionContext(restSpec); } diff --git a/src/test/java/org/elasticsearch/test/rest/spec/RestSpec.java b/src/test/java/org/elasticsearch/test/rest/spec/RestSpec.java index 34b6315fc2e..979bacc26c2 100644 --- a/src/test/java/org/elasticsearch/test/rest/spec/RestSpec.java +++ b/src/test/java/org/elasticsearch/test/rest/spec/RestSpec.java @@ -19,12 +19,14 @@ package org.elasticsearch.test.rest.spec; import com.google.common.collect.Maps; + import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.common.xcontent.json.JsonXContent; import org.elasticsearch.test.rest.support.FileUtils; import java.io.IOException; import java.io.InputStream; +import java.nio.file.FileSystem; import java.nio.file.Files; import java.nio.file.Path; import java.util.Collection; @@ -54,10 +56,10 @@ public class RestSpec { /** * Parses the complete set of REST spec available under the provided directories */ - public static RestSpec parseFrom(String optionalPathPrefix, String... paths) throws IOException { + public static RestSpec parseFrom(FileSystem fileSystem, String optionalPathPrefix, String... paths) throws IOException { RestSpec restSpec = new RestSpec(); for (String path : paths) { - for (Path jsonFile : FileUtils.findJsonSpec(optionalPathPrefix, path)) { + for (Path jsonFile : FileUtils.findJsonSpec(fileSystem, optionalPathPrefix, path)) { try (InputStream stream = Files.newInputStream(jsonFile)) { XContentParser parser = JsonXContent.jsonXContent.createParser(stream); RestApi restApi = new RestApiParser().parse(parser); diff --git a/src/test/java/org/elasticsearch/test/rest/support/FileUtils.java b/src/test/java/org/elasticsearch/test/rest/support/FileUtils.java index 28c3f597f9f..5e230a6f993 100644 --- a/src/test/java/org/elasticsearch/test/rest/support/FileUtils.java +++ b/src/test/java/org/elasticsearch/test/rest/support/FileUtils.java @@ -25,17 +25,14 @@ import org.elasticsearch.common.Strings; import org.elasticsearch.common.io.PathUtils; import java.io.IOException; -import java.net.URI; -import java.net.URISyntaxException; import java.net.URL; import java.nio.file.DirectoryStream; -import java.nio.file.FileSystems; +import java.nio.file.FileSystem; import java.nio.file.FileVisitResult; import java.nio.file.Files; import java.nio.file.NoSuchFileException; import java.nio.file.NotDirectoryException; import java.nio.file.Path; -import java.nio.file.Paths; import java.nio.file.SimpleFileVisitor; import java.nio.file.attribute.BasicFileAttributes; import java.util.HashSet; @@ -53,10 +50,10 @@ public final class FileUtils { /** * Returns the json files found within the directory provided as argument. - * Files are looked up in the classpath first, then outside of it if not found. + * Files are looked up in the classpath, or optionally from {@code fileSystem} if its not null. */ - public static Set findJsonSpec(String optionalPathPrefix, String path) throws IOException { - Path dir = resolveFile(optionalPathPrefix, path, null); + public static Set findJsonSpec(FileSystem fileSystem, String optionalPathPrefix, String path) throws IOException { + Path dir = resolveFile(fileSystem, optionalPathPrefix, path, null); if (!Files.isDirectory(dir)) { throw new NotDirectoryException(path); @@ -81,37 +78,46 @@ public final class FileUtils { /** * Returns the yaml files found within the paths provided. * Each input path can either be a single file (the .yaml suffix is optional) or a directory. - * Each path is looked up in the classpath first, then outside of it if not found yet. + * Each path is looked up in the classpath, or optionally from {@code fileSystem} if its not null. */ - public static Map> findYamlSuites(final String optionalPathPrefix, final String... paths) throws IOException { + public static Map> findYamlSuites(FileSystem fileSystem, String optionalPathPrefix, final String... paths) throws IOException { Map> yamlSuites = Maps.newHashMap(); for (String path : paths) { - collectFiles(resolveFile(optionalPathPrefix, path, YAML_SUFFIX), YAML_SUFFIX, yamlSuites); + collectFiles(resolveFile(fileSystem, optionalPathPrefix, path, YAML_SUFFIX), YAML_SUFFIX, yamlSuites); } return yamlSuites; } - private static Path resolveFile(String optionalPathPrefix, String path, String optionalFileSuffix) throws IOException { - //try within classpath with and without file suffix (as it could be a single test suite) - URL resource = findResource(path, optionalFileSuffix); - if (resource == null) { - //try within classpath with optional prefix: /rest-api-spec/test (or /rest-api-spec/api) is optional - String newPath = optionalPathPrefix + "/" + path; - resource = findResource(newPath, optionalFileSuffix); - if (resource == null) { - //if it wasn't on classpath we look outside of the classpath - Path file = findFile(path, optionalFileSuffix); - if (!Files.exists(file)) { + private static Path resolveFile(FileSystem fileSystem, String optionalPathPrefix, String path, String optionalFileSuffix) throws IOException { + if (fileSystem != null) { + Path file = findFile(fileSystem, path, optionalFileSuffix); + if (!lenientExists(file)) { + // try with optional prefix: /rest-api-spec/test (or /rest-api-spec/api) is optional + String newPath = optionalPathPrefix + "/" + path; + file = findFile(fileSystem, newPath, optionalFileSuffix); + if (!lenientExists(file)) { throw new NoSuchFileException(path); } - return file; } - } - - try { - return PathUtils.get(resource.toURI()); - } catch (URISyntaxException e) { - throw new RuntimeException(e); + return file; + } else { + //try within classpath + URL resource = findResource(path, optionalFileSuffix); + if (resource == null) { + //try within classpath with optional prefix: /rest-api-spec/test (or /rest-api-spec/api) is optional + String newPath = optionalPathPrefix + "/" + path; + resource = findResource(newPath, optionalFileSuffix); + if (resource == null) { + throw new NoSuchFileException(path); + } + } + try { + return PathUtils.get(resource.toURI()); + } catch (Exception e) { + // some filesystems have REALLY useless exceptions here. + // ZipFileSystem I am looking at you. + throw new RuntimeException("couldn't retrieve URL: " + resource, e); + } } } @@ -125,11 +131,20 @@ public final class FileUtils { } return resource; } + + // used because this test "guesses" from like 4 different places from the filesystem! + private static boolean lenientExists(Path file) { + boolean exists = false; + try { + exists = Files.exists(file); + } catch (SecurityException ok) {} + return exists; + } - private static Path findFile(String path, String optionalFileSuffix) { - Path file = PathUtils.get(path); - if (!Files.exists(file)) { - file = PathUtils.get(path + optionalFileSuffix); + private static Path findFile(FileSystem fileSystem, String path, String optionalFileSuffix) { + Path file = fileSystem.getPath(path); + if (!lenientExists(file)) { + file = fileSystem.getPath(path + optionalFileSuffix); } return file; } diff --git a/src/test/java/org/elasticsearch/test/rest/test/FileUtilsTests.java b/src/test/java/org/elasticsearch/test/rest/test/FileUtilsTests.java index f35daa926a1..10db051e2b1 100644 --- a/src/test/java/org/elasticsearch/test/rest/test/FileUtilsTests.java +++ b/src/test/java/org/elasticsearch/test/rest/test/FileUtilsTests.java @@ -35,29 +35,29 @@ public class FileUtilsTests extends ElasticsearchTestCase { @Test public void testLoadSingleYamlSuite() throws Exception { - Map> yamlSuites = FileUtils.findYamlSuites("/rest-api-spec/test", "/rest-api-spec/test/get/10_basic"); + Map> yamlSuites = FileUtils.findYamlSuites(null, "/rest-api-spec/test", "/rest-api-spec/test/get/10_basic"); assertSingleFile(yamlSuites, "get", "10_basic.yaml"); //the path prefix is optional - yamlSuites = FileUtils.findYamlSuites("/rest-api-spec/test", "get/10_basic.yaml"); + yamlSuites = FileUtils.findYamlSuites(null, "/rest-api-spec/test", "get/10_basic.yaml"); assertSingleFile(yamlSuites, "get", "10_basic.yaml"); //extension .yaml is optional - yamlSuites = FileUtils.findYamlSuites("/rest-api-spec/test", "get/10_basic"); + yamlSuites = FileUtils.findYamlSuites(null, "/rest-api-spec/test", "get/10_basic"); assertSingleFile(yamlSuites, "get", "10_basic.yaml"); } @Test public void testLoadMultipleYamlSuites() throws Exception { //single directory - Map> yamlSuites = FileUtils.findYamlSuites("/rest-api-spec/test", "get"); + Map> yamlSuites = FileUtils.findYamlSuites(null, "/rest-api-spec/test", "get"); assertThat(yamlSuites, notNullValue()); assertThat(yamlSuites.size(), equalTo(1)); assertThat(yamlSuites.containsKey("get"), equalTo(true)); assertThat(yamlSuites.get("get").size(), greaterThan(1)); //multiple directories - yamlSuites = FileUtils.findYamlSuites("/rest-api-spec/test", "get", "index"); + yamlSuites = FileUtils.findYamlSuites(null, "/rest-api-spec/test", "get", "index"); assertThat(yamlSuites, notNullValue()); assertThat(yamlSuites.size(), equalTo(2)); assertThat(yamlSuites.containsKey("get"), equalTo(true)); @@ -66,7 +66,7 @@ public class FileUtilsTests extends ElasticsearchTestCase { assertThat(yamlSuites.get("index").size(), greaterThan(1)); //multiple paths, which can be both directories or yaml test suites (with optional file extension) - yamlSuites = FileUtils.findYamlSuites("/rest-api-spec/test", "indices.optimize/10_basic", "index"); + yamlSuites = FileUtils.findYamlSuites(null, "/rest-api-spec/test", "indices.optimize/10_basic", "index"); assertThat(yamlSuites, notNullValue()); assertThat(yamlSuites.size(), equalTo(2)); assertThat(yamlSuites.containsKey("indices.optimize"), equalTo(true)); @@ -81,22 +81,16 @@ public class FileUtilsTests extends ElasticsearchTestCase { Files.createFile(file); //load from directory outside of the classpath - yamlSuites = FileUtils.findYamlSuites("/rest-api-spec/test", "get/10_basic", dir.toAbsolutePath().toString()); + yamlSuites = FileUtils.findYamlSuites(dir.getFileSystem(), "/rest-api-spec/test", dir.toAbsolutePath().toString()); assertThat(yamlSuites, notNullValue()); - assertThat(yamlSuites.size(), equalTo(2)); - assertThat(yamlSuites.containsKey("get"), equalTo(true)); - assertThat(yamlSuites.get("get").size(), equalTo(1)); - assertSingleFile(yamlSuites.get("get"), "get", "10_basic.yaml"); + assertThat(yamlSuites.size(), equalTo(1)); assertThat(yamlSuites.containsKey(dir.getFileName().toString()), equalTo(true)); assertSingleFile(yamlSuites.get(dir.getFileName().toString()), dir.getFileName().toString(), file.getFileName().toString()); //load from external file (optional extension) - yamlSuites = FileUtils.findYamlSuites("/rest-api-spec/test", "get/10_basic", dir.resolve("test_loading").toAbsolutePath().toString()); + yamlSuites = FileUtils.findYamlSuites(dir.getFileSystem(), "/rest-api-spec/test", dir.resolve("test_loading").toAbsolutePath().toString()); assertThat(yamlSuites, notNullValue()); - assertThat(yamlSuites.size(), equalTo(2)); - assertThat(yamlSuites.containsKey("get"), equalTo(true)); - assertThat(yamlSuites.get("get").size(), equalTo(1)); - assertSingleFile(yamlSuites.get("get"), "get", "10_basic.yaml"); + assertThat(yamlSuites.size(), equalTo(1)); assertThat(yamlSuites.containsKey(dir.getFileName().toString()), equalTo(true)); assertSingleFile(yamlSuites.get(dir.getFileName().toString()), dir.getFileName().toString(), file.getFileName().toString()); }