From b5f4b2444ce99792fdc31e44e1301544c2b82827 Mon Sep 17 00:00:00 2001 From: Luca Cavanna Date: Thu, 16 Jan 2014 14:40:35 +0100 Subject: [PATCH] [TEST] Left '.' as it is in REST suites and sections titles (was previously replaced with '_') With JUnit up to 4.10 there's no way to distinguish between what's shown in IDEs and test ids that need to be unique. A test Description is identified by just a string, that needs to be unique and is shown by IDEs. IDEs have slightly different behaviours when it comes to showing tests and suites titles. Some IDE (e.g. IntelliJ) strips the description on '.', which is why the '.' was replaced in the first place, in order to obtain the same behaviour on all IDEs. On the other hand the information printed out by RestReproduceInfoPrinter was wrong as the file path contained a '_' instead of a '.', which made the string to reproduce a failure useless in some cases. At the end of the day it seems better to just keep the dots and accept slightly different behaviours that are IDE dependent. --- .../elasticsearch/test/rest/junit/DescriptionHelper.java | 8 ++++---- .../elasticsearch/test/rest/section/RestTestSuite.java | 9 ++------- .../org/elasticsearch/test/rest/support/FileUtils.java | 3 +-- .../org/elasticsearch/test/rest/test/FileUtilsTests.java | 8 ++++---- 4 files changed, 11 insertions(+), 17 deletions(-) diff --git a/src/test/java/org/elasticsearch/test/rest/junit/DescriptionHelper.java b/src/test/java/org/elasticsearch/test/rest/junit/DescriptionHelper.java index 40d52e8bc99..44284a4d9e4 100644 --- a/src/test/java/org/elasticsearch/test/rest/junit/DescriptionHelper.java +++ b/src/test/java/org/elasticsearch/test/rest/junit/DescriptionHelper.java @@ -49,20 +49,20 @@ public final class DescriptionHelper { } static Description createTestSuiteDescription(RestTestSuite restTestSuite) { - //e.g. "indices_open (10_basic)", which leads to 10_basic being returned by Description#getDisplayName + //e.g. "indices.open (10_basic)", which leads to 10_basic being shown by IDEs String name = restTestSuite.getApi() + " (" + restTestSuite.getName() + ")"; return Description.createSuiteDescription(name); } static Description createTestSectionWithRepetitionsDescription(RestTestSuite restTestSuite, TestSection testSection) { - //e.g. "indices_open/10_basic (Basic test for index open/close)", which leads to - //"Basic test for index open/close" being returned by Description#getDisplayName + //e.g. "indices.open/10_basic (Basic test for index open/close)", which leads to + //"Basic test for index open/close" being shown by IDEs String name = restTestSuite.getDescription() + " (" + testSection.getName() + ")"; return Description.createSuiteDescription(name); } static Description createTestSectionIterationDescription(RestTestSuite restTestSuite, TestSection testSection, Map args) { - //e.g. "Basic test for index open/close {#0} (indices_open/10_basic)" some IDEs might strip out the part between parentheses + //e.g. "Basic test for index open/close {#0} (indices.open/10_basic)" some IDEs might strip out the part between parentheses String name = testSection.getName() + formatMethodArgs(args) + " (" + restTestSuite.getDescription() + ")"; return Description.createSuiteDescription(name); } diff --git a/src/test/java/org/elasticsearch/test/rest/section/RestTestSuite.java b/src/test/java/org/elasticsearch/test/rest/section/RestTestSuite.java index e905588bd59..286bb782a4e 100644 --- a/src/test/java/org/elasticsearch/test/rest/section/RestTestSuite.java +++ b/src/test/java/org/elasticsearch/test/rest/section/RestTestSuite.java @@ -37,8 +37,8 @@ public class RestTestSuite { private List testSections = Lists.newArrayList(); public RestTestSuite(String api, String name) { - this.api = replaceDot(api); - this.name = replaceDot(name); + this.api = api; + this.name = name; } public String getApi() { @@ -55,11 +55,6 @@ public class RestTestSuite { return api + File.separator + name; } - private static String replaceDot(String value) { - // '.' is used as separator internally and not expected to be within suite or test names, better replace it - return value.replace('.', '_'); - } - public SetupSection getSetupSection() { return setupSection; } 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 2798499a672..6c81703d716 100644 --- a/src/test/java/org/elasticsearch/test/rest/support/FileUtils.java +++ b/src/test/java/org/elasticsearch/test/rest/support/FileUtils.java @@ -116,8 +116,7 @@ public final class FileUtils { private static void collectFiles(final File file, final String fileSuffix, final Map> files) { if (file.isFile()) { - // '.' is uses as separator internally and not expected to be within suite or test names, better replace it - String groupName = file.getParentFile().getName().replace('.', '_'); + String groupName = file.getParentFile().getName(); Set filesSet = files.get(groupName); if (filesSet == null) { filesSet = Sets.newHashSet(); 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 90ee910ba18..66c7f042943 100644 --- a/src/test/java/org/elasticsearch/test/rest/test/FileUtilsTests.java +++ b/src/test/java/org/elasticsearch/test/rest/test/FileUtilsTests.java @@ -65,12 +65,12 @@ 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", "get/10_basic", "index"); + yamlSuites = FileUtils.findYamlSuites("/rest-api-spec/test", "indices.optimize/10_basic", "index"); 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.containsKey("indices.optimize"), equalTo(true)); + assertThat(yamlSuites.get("indices.optimize").size(), equalTo(1)); + assertSingleFile(yamlSuites.get("indices.optimize"), "indices.optimize", "10_basic.yaml"); assertThat(yamlSuites.containsKey("index"), equalTo(true)); assertThat(yamlSuites.get("index").size(), greaterThan(1));