From 804bee7f4b28a5b812931a155b35d7d5953a6915 Mon Sep 17 00:00:00 2001 From: "ievgen.degtiarenko" Date: Tue, 25 Mar 2025 10:17:33 +0100 Subject: [PATCH 1/2] Improve StatementParserTests error message --- .../org/elasticsearch/test/ESTestCase.java | 14 ++++++++++ .../xpack/esql/IdentifierGenerator.java | 3 +- .../parser/AbstractStatementParserTests.java | 28 ++++++++++++------- 3 files changed, 33 insertions(+), 12 deletions(-) diff --git a/test/framework/src/main/java/org/elasticsearch/test/ESTestCase.java b/test/framework/src/main/java/org/elasticsearch/test/ESTestCase.java index bdb35ceed7d4e..9d779156e3630 100644 --- a/test/framework/src/main/java/org/elasticsearch/test/ESTestCase.java +++ b/test/framework/src/main/java/org/elasticsearch/test/ESTestCase.java @@ -2713,6 +2713,20 @@ public static T expectThrows(Class expectedType, Matche return e; } + /** + * Checks a specific exception class with matched message is thrown by the given runnable, and returns it. + */ + public static T expectThrows( + String reason, + Class expectedType, + Matcher messageMatcher, + ThrowingRunnable runnable + ) { + var e = expectThrows(expectedType, reason, runnable); + assertThat(reason, e.getMessage(), messageMatcher); + return e; + } + /** * Same as {@link #runInParallel(int, IntConsumer)} but also attempts to start all tasks at the same time by blocking execution on a * barrier until all threads are started and ready to execute their task. diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/IdentifierGenerator.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/IdentifierGenerator.java index 1f063912d51c5..8e594c96d84af 100644 --- a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/IdentifierGenerator.java +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/IdentifierGenerator.java @@ -90,8 +90,7 @@ public static String randomIndexPattern(Feature... features) { var cluster = maybeQuote(randomIdentifier()); pattern = maybeQuote(cluster + ":" + pattern); } else if (EsqlCapabilities.Cap.INDEX_COMPONENT_SELECTORS.isEnabled() && canAdd(Features.INDEX_SELECTOR, features)) { - var selector = ESTestCase.randomFrom(IndexComponentSelector.values()); - pattern = maybeQuote(pattern + "::" + selector.getKey()); + pattern = maybeQuote(pattern + "::" + randomFrom(IndexComponentSelector.values()).getKey()); } return pattern; } diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/parser/AbstractStatementParserTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/parser/AbstractStatementParserTests.java index 15ce478469474..915a82cdf6c89 100644 --- a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/parser/AbstractStatementParserTests.java +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/parser/AbstractStatementParserTests.java @@ -140,22 +140,30 @@ static MapExpression mapExpression(Map keyValuePairs) { } void expectError(String query, String errorMessage) { - ParsingException e = expectThrows(ParsingException.class, "Expected syntax error for " + query, () -> statement(query)); - assertThat(e.getMessage(), containsString(errorMessage)); - } - - void expectVerificationError(String query, String errorMessage) { - VerificationException e = expectThrows(VerificationException.class, "Expected syntax error for " + query, () -> statement(query)); - assertThat(e.getMessage(), containsString(errorMessage)); + expectThrows( + "Query [" + query + "] is expected to throw " + ParsingException.class + " with message [" + errorMessage + "]", + ParsingException.class, + containsString(errorMessage), + () -> parser.createStatement(query) + ); } void expectError(String query, List params, String errorMessage) { - ParsingException e = expectThrows( + expectThrows( + "Query [" + query + "] is expected to throw " + ParsingException.class + " with message [" + errorMessage + "]", ParsingException.class, - "Expected syntax error for " + query, + containsString(errorMessage), () -> statement(query, new QueryParams(params)) ); - assertThat(e.getMessage(), containsString(errorMessage)); + } + + void expectVerificationError(String query, String errorMessage) { + expectThrows( + "Query [" + query + "] is expected to throw " + VerificationException.class + " with message [" + errorMessage + "]", + VerificationException.class, + containsString(errorMessage), + () -> parser.createStatement(query) + ); } void expectInvalidIndexNameErrorWithLineNumber(String query, String indexString, String lineNumber) { From 3d2c1a00d22b8f4ffe7161a8c3596dd0718e8060 Mon Sep 17 00:00:00 2001 From: "ievgen.degtiarenko" Date: Tue, 25 Mar 2025 13:05:31 +0100 Subject: [PATCH 2/2] delegate call --- .../xpack/esql/parser/AbstractStatementParserTests.java | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/parser/AbstractStatementParserTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/parser/AbstractStatementParserTests.java index 915a82cdf6c89..27c2b2b67e984 100644 --- a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/parser/AbstractStatementParserTests.java +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/parser/AbstractStatementParserTests.java @@ -140,12 +140,7 @@ static MapExpression mapExpression(Map keyValuePairs) { } void expectError(String query, String errorMessage) { - expectThrows( - "Query [" + query + "] is expected to throw " + ParsingException.class + " with message [" + errorMessage + "]", - ParsingException.class, - containsString(errorMessage), - () -> parser.createStatement(query) - ); + expectError(query, null, errorMessage); } void expectError(String query, List params, String errorMessage) {