From fa028323e06b85b078b60f26b686060643c892fd Mon Sep 17 00:00:00 2001 From: Nik Everett Date: Fri, 1 Nov 2024 13:17:06 -0400 Subject: [PATCH 1/4] ESQL: Allow duplicate warnings in some tests This fixes a test, actually in serverless Elasticsearch, that gets duplicate warnings. We'd like not to emit these duplicate warnings, but at this point it isn't worth it. So, for now, in some tests we allow duplicate warnings. In most of our tests we do not allow duplicate warnings so that we don't make *more* duplicate warnings without thinking about it. --- .../esql/qa/mixed/MixedClusterEsqlSpecIT.java | 11 ++++ .../xpack/esql/qa/rest/EsqlSpecTestCase.java | 32 ++++++--- .../xpack/esql/qa/rest/RestEsqlTestCase.java | 66 ++++++++----------- .../xpack/esql/AssertWarnings.java | 52 +++++++++++++++ .../xpack/esql/CsvSpecReader.java | 16 +++++ .../xpack/esql/EsqlTestUtils.java | 10 --- 6 files changed, 128 insertions(+), 59 deletions(-) create mode 100644 x-pack/plugin/esql/qa/testFixtures/src/main/java/org/elasticsearch/xpack/esql/AssertWarnings.java diff --git a/x-pack/plugin/esql/qa/server/mixed-cluster/src/javaRestTest/java/org/elasticsearch/xpack/esql/qa/mixed/MixedClusterEsqlSpecIT.java b/x-pack/plugin/esql/qa/server/mixed-cluster/src/javaRestTest/java/org/elasticsearch/xpack/esql/qa/mixed/MixedClusterEsqlSpecIT.java index 0e23b29172c32..801e1d12b1d4a 100644 --- a/x-pack/plugin/esql/qa/server/mixed-cluster/src/javaRestTest/java/org/elasticsearch/xpack/esql/qa/mixed/MixedClusterEsqlSpecIT.java +++ b/x-pack/plugin/esql/qa/server/mixed-cluster/src/javaRestTest/java/org/elasticsearch/xpack/esql/qa/mixed/MixedClusterEsqlSpecIT.java @@ -91,4 +91,15 @@ protected boolean enableRoundingDoubleValuesOnAsserting() { protected boolean supportsInferenceTestService() { return false; } + + @Override + protected boolean deduplicateExactWarnings() { + /* + * In ESQL's main tests we shouldn't have to deduplicate but in + * serverless, where we reuse this test case exactly with *slightly* + * different configuration, we must deduplicate. So we do it here. + * It's a bit of a loss of precision, but that's ok. + */ + return true; + } } diff --git a/x-pack/plugin/esql/qa/server/src/main/java/org/elasticsearch/xpack/esql/qa/rest/EsqlSpecTestCase.java b/x-pack/plugin/esql/qa/server/src/main/java/org/elasticsearch/xpack/esql/qa/rest/EsqlSpecTestCase.java index 57f58fc448822..6ebf05755ef5e 100644 --- a/x-pack/plugin/esql/qa/server/src/main/java/org/elasticsearch/xpack/esql/qa/rest/EsqlSpecTestCase.java +++ b/x-pack/plugin/esql/qa/server/src/main/java/org/elasticsearch/xpack/esql/qa/rest/EsqlSpecTestCase.java @@ -26,6 +26,7 @@ import org.elasticsearch.test.rest.ESRestTestCase; import org.elasticsearch.test.rest.TestFeatureService; import org.elasticsearch.xcontent.XContentType; +import org.elasticsearch.xpack.esql.AssertWarnings; import org.elasticsearch.xpack.esql.CsvSpecReader.CsvTestCase; import org.elasticsearch.xpack.esql.CsvTestUtils; import org.elasticsearch.xpack.esql.EsqlTestUtils; @@ -47,7 +48,6 @@ import java.util.Locale; import java.util.Map; import java.util.TreeMap; -import java.util.regex.Pattern; import java.util.stream.Collectors; import java.util.stream.IntStream; import java.util.stream.LongStream; @@ -230,7 +230,7 @@ protected final void doTest() throws Throwable { builder.tables(tables()); } - Map answer = runEsql(builder.query(testCase.query), testCase.expectedWarnings(), testCase.expectedWarningsRegex()); + Map answer = runEsql(builder.query(testCase.query), testCase.assertWarnings(deduplicateExactWarnings())); var expectedColumnsWithValues = loadCsvSpecValues(testCase.expectedResults); @@ -248,16 +248,30 @@ protected final void doTest() throws Throwable { assertResults(expectedColumnsWithValues, actualColumns, actualValues, testCase.ignoreOrder, logger); } - private Map runEsql( - RequestObjectBuilder requestObject, - List expectedWarnings, - List expectedWarningsRegex - ) throws IOException { + /** + * Should warnings be de-duplicated before checking for exact matches. Defaults + * to {@code false}, but in some environments we emit duplicate warnings. We'd prefer + * not to emit duplicate warnings but for now it isn't worth fighting with. So! In + * those environments we override this to deduplicate. + *

+ * Note: This only applies to warnings declared as {@code warning:}. Those + * declared as {@code warningRegex:} are always a list of + * allowed warnings. {@code warningRegex:} matches 0 or more + * warnings. There is no need to deduplicate because there's no expectation + * of an exact match. + *

+ * + */ + protected boolean deduplicateExactWarnings() { + return false; + } + + private Map runEsql(RequestObjectBuilder requestObject, AssertWarnings assertWarnings) throws IOException { if (mode == Mode.ASYNC) { assert supportsAsync(); - return RestEsqlTestCase.runEsqlAsync(requestObject, expectedWarnings, expectedWarningsRegex); + return RestEsqlTestCase.runEsqlAsync(requestObject, assertWarnings); } else { - return RestEsqlTestCase.runEsqlSync(requestObject, expectedWarnings, expectedWarningsRegex); + return RestEsqlTestCase.runEsqlSync(requestObject, assertWarnings); } } diff --git a/x-pack/plugin/esql/qa/server/src/main/java/org/elasticsearch/xpack/esql/qa/rest/RestEsqlTestCase.java b/x-pack/plugin/esql/qa/server/src/main/java/org/elasticsearch/xpack/esql/qa/rest/RestEsqlTestCase.java index ef1e77280d0ee..466c20524cf34 100644 --- a/x-pack/plugin/esql/qa/server/src/main/java/org/elasticsearch/xpack/esql/qa/rest/RestEsqlTestCase.java +++ b/x-pack/plugin/esql/qa/server/src/main/java/org/elasticsearch/xpack/esql/qa/rest/RestEsqlTestCase.java @@ -31,6 +31,7 @@ import org.elasticsearch.xcontent.ToXContent; import org.elasticsearch.xcontent.XContentBuilder; import org.elasticsearch.xcontent.XContentType; +import org.elasticsearch.xpack.esql.AssertWarnings; import org.elasticsearch.xpack.esql.EsqlTestUtils; import org.elasticsearch.xpack.esql.action.EsqlCapabilities; import org.junit.After; @@ -83,9 +84,6 @@ public abstract class RestEsqlTestCase extends ESRestTestCase { private static final Logger LOGGER = LogManager.getLogger(RestEsqlTestCase.class); - private static final List NO_WARNINGS = List.of(); - private static final List NO_WARNINGS_REGEX = List.of(); - private static final String MAPPING_ALL_TYPES; static { @@ -379,7 +377,7 @@ public void testCSVNoHeaderMode() throws IOException { options.addHeader("Content-Type", mediaType); options.addHeader("Accept", "text/csv; header=absent"); request.setOptions(options); - HttpEntity entity = performRequest(request, NO_WARNINGS, NO_WARNINGS_REGEX); + HttpEntity entity = performRequest(request, new AssertWarnings.NoWarnings()); String actual = Streams.copyToString(new InputStreamReader(entity.getContent(), StandardCharsets.UTF_8)); assertEquals("keyword0,0\r\n", actual); } @@ -430,11 +428,13 @@ public void testOutOfRangeComparisons() throws IOException { for (String truePredicate : trueForSingleValuesPredicates) { String comparison = fieldWithType + truePredicate; var query = requestObjectBuilder().query(format(null, "from {} | where {}", testIndexName(), comparison)); - List expectedWarnings = List.of( - "Line 1:29: evaluation of [" + comparison + "] failed, treating result as null. Only first 20 failures recorded.", - "Line 1:29: java.lang.IllegalArgumentException: single-value function encountered multi-value" + AssertWarnings assertWarnings = new AssertWarnings.ExactStrings( + List.of( + "Line 1:29: evaluation of [" + comparison + "] failed, treating result as null. Only first 20 failures recorded.", + "Line 1:29: java.lang.IllegalArgumentException: single-value function encountered multi-value" + ) ); - var result = runEsql(query, expectedWarnings, NO_WARNINGS_REGEX, mode); + var result = runEsql(query, assertWarnings, mode); var values = as(result.get("values"), ArrayList.class); assertThat( @@ -504,7 +504,7 @@ public void testInternalRange() throws IOException { for (String p : predicates) { var query = requestObjectBuilder().query(format(null, "from {} | where {}", testIndexName(), p)); - var result = runEsql(query, List.of(), NO_WARNINGS_REGEX, mode); + var result = runEsql(query, new AssertWarnings.NoWarnings(), mode); var values = as(result.get("values"), ArrayList.class); assertThat( format(null, "Comparison [{}] should return all rows with single values.", p), @@ -996,35 +996,26 @@ private static String expectedTextBody(String format, int count, @Nullable Chara } public Map runEsql(RequestObjectBuilder requestObject) throws IOException { - return runEsql(requestObject, NO_WARNINGS, NO_WARNINGS_REGEX, mode); + return runEsql(requestObject, new AssertWarnings.NoWarnings(), mode); } public static Map runEsqlSync(RequestObjectBuilder requestObject) throws IOException { - return runEsqlSync(requestObject, NO_WARNINGS, NO_WARNINGS_REGEX); + return runEsqlSync(requestObject, new AssertWarnings.NoWarnings()); } public static Map runEsqlAsync(RequestObjectBuilder requestObject) throws IOException { - return runEsqlAsync(requestObject, NO_WARNINGS, NO_WARNINGS_REGEX); + return runEsqlAsync(requestObject, new AssertWarnings.NoWarnings()); } - static Map runEsql( - RequestObjectBuilder requestObject, - List expectedWarnings, - List expectedWarningsRegex, - Mode mode - ) throws IOException { + static Map runEsql(RequestObjectBuilder requestObject, AssertWarnings assertWarnings, Mode mode) throws IOException { if (mode == ASYNC) { - return runEsqlAsync(requestObject, expectedWarnings, expectedWarningsRegex); + return runEsqlAsync(requestObject, assertWarnings); } else { - return runEsqlSync(requestObject, expectedWarnings, expectedWarningsRegex); + return runEsqlSync(requestObject, assertWarnings); } } - public static Map runEsqlSync( - RequestObjectBuilder requestObject, - List expectedWarnings, - List expectedWarningsRegex - ) throws IOException { + public static Map runEsqlSync(RequestObjectBuilder requestObject, AssertWarnings assertWarnings) throws IOException { requestObject.build(); Request request = prepareRequest(SYNC); String mediaType = attachBody(requestObject, request); @@ -1040,15 +1031,11 @@ public static Map runEsqlSync( } request.setOptions(options); - HttpEntity entity = performRequest(request, expectedWarnings, expectedWarningsRegex); + HttpEntity entity = performRequest(request, assertWarnings); return entityToMap(entity, requestObject.contentType()); } - public static Map runEsqlAsync( - RequestObjectBuilder requestObject, - List expectedWarnings, - List expectedWarningsRegex - ) throws IOException { + public static Map runEsqlAsync(RequestObjectBuilder requestObject, AssertWarnings assertWarnings) throws IOException { addAsyncParameters(requestObject); requestObject.build(); Request request = prepareRequest(ASYNC); @@ -1088,7 +1075,7 @@ public static Map runEsqlAsync( assertThat(response.getHeader("X-Elasticsearch-Async-Id"), nullValue()); assertThat(response.getHeader("X-Elasticsearch-Async-Is-Running"), is("?0")); } - assertWarnings(response, expectedWarnings, expectedWarningsRegex); + assertWarnings(response, assertWarnings); json.remove("is_running"); // remove this to not mess up later map assertions return Collections.unmodifiableMap(json); } else { @@ -1098,7 +1085,7 @@ public static Map runEsqlAsync( if (isRunning == false) { // must have completed immediately so keep_on_completion must be true assertThat(requestObject.keepOnCompletion(), is(true)); - assertWarnings(response, expectedWarnings, expectedWarningsRegex); + assertWarnings(response, assertWarnings); // we already have the results, but let's remember them so that we can compare to async get initialColumns = json.get("columns"); initialValues = json.get("values"); @@ -1128,7 +1115,7 @@ public static Map runEsqlAsync( assertEquals(initialValues, result.get("values")); } - assertWarnings(response, expectedWarnings, expectedWarningsRegex); + assertWarnings(response, assertWarnings); assertDeletable(id); return removeAsyncProperties(result); } @@ -1202,7 +1189,7 @@ static String runEsqlAsTextWithFormat(RequestObjectBuilder builder, String forma } request.setOptions(options); - HttpEntity entity = performRequest(request, NO_WARNINGS, NO_WARNINGS_REGEX); + HttpEntity entity = performRequest(request, new AssertWarnings.NoWarnings()); return Streams.copyToString(new InputStreamReader(entity.getContent(), StandardCharsets.UTF_8)); } @@ -1235,9 +1222,8 @@ private static String attachBody(RequestObjectBuilder requestObject, Request req return mediaType; } - private static HttpEntity performRequest(Request request, List allowedWarnings, List allowedWarningsRegex) - throws IOException { - return assertWarnings(performRequest(request), allowedWarnings, allowedWarningsRegex); + private static HttpEntity performRequest(Request request, AssertWarnings assertWarnings) throws IOException { + return assertWarnings(performRequest(request), assertWarnings); } private static Response performRequest(Request request) throws IOException { @@ -1250,13 +1236,13 @@ private static Response performRequest(Request request) throws IOException { return response; } - private static HttpEntity assertWarnings(Response response, List allowedWarnings, List allowedWarningsRegex) { + private static HttpEntity assertWarnings(Response response, AssertWarnings assertWarnings) { List warnings = new ArrayList<>(response.getWarnings()); warnings.removeAll(mutedWarnings()); if (shouldLog()) { LOGGER.info("RESPONSE warnings (after muted)={}", warnings); } - EsqlTestUtils.assertWarnings(warnings, allowedWarnings, allowedWarningsRegex); + assertWarnings.assertWarnings(warnings); return response.getEntity(); } diff --git a/x-pack/plugin/esql/qa/testFixtures/src/main/java/org/elasticsearch/xpack/esql/AssertWarnings.java b/x-pack/plugin/esql/qa/testFixtures/src/main/java/org/elasticsearch/xpack/esql/AssertWarnings.java new file mode 100644 index 0000000000000..f606d36ee6b6c --- /dev/null +++ b/x-pack/plugin/esql/qa/testFixtures/src/main/java/org/elasticsearch/xpack/esql/AssertWarnings.java @@ -0,0 +1,52 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +package org.elasticsearch.xpack.esql; + +import java.util.List; +import java.util.regex.Pattern; + +import static org.elasticsearch.test.ListMatcher.matchesList; +import static org.elasticsearch.test.MapMatcher.assertMap; +import static org.junit.Assert.assertTrue; + +/** + * How should we assert the warnings returned by ESQL. + */ +public interface AssertWarnings { + void assertWarnings(List warnings); + + record NoWarnings() implements AssertWarnings { + @Override + public void assertWarnings(List warnings) { + assertMap(warnings.stream().sorted().toList(), matchesList()); + } + } + + record ExactStrings(List expected) implements AssertWarnings { + @Override + public void assertWarnings(List warnings) { + assertMap(warnings.stream().sorted().toList(), matchesList(expected.stream().sorted().toList())); + } + } + + record DeduplicatedStrings(List expected) implements AssertWarnings { + @Override + public void assertWarnings(List warnings) { + assertMap(warnings.stream().sorted().distinct().toList(), matchesList(expected.stream().sorted().toList())); + } + } + + record AllowedRegexes(List expected) implements AssertWarnings { + @Override + public void assertWarnings(List warnings) { + for (String warning : warnings) { + assertTrue("Unexpected warning: " + warning, expected.stream().anyMatch(x -> x.matcher(warning).matches())); + } + } + } +} diff --git a/x-pack/plugin/esql/qa/testFixtures/src/main/java/org/elasticsearch/xpack/esql/CsvSpecReader.java b/x-pack/plugin/esql/qa/testFixtures/src/main/java/org/elasticsearch/xpack/esql/CsvSpecReader.java index 781ae5531c6f0..a78eaf20a8fab 100644 --- a/x-pack/plugin/esql/qa/testFixtures/src/main/java/org/elasticsearch/xpack/esql/CsvSpecReader.java +++ b/x-pack/plugin/esql/qa/testFixtures/src/main/java/org/elasticsearch/xpack/esql/CsvSpecReader.java @@ -142,6 +142,22 @@ public void adjustExpectedWarnings(Function updater) { public List expectedWarningsRegex() { return expectedWarningsRegex; } + + /** + * How should we assert the warnings returned by ESQL. + */ + public AssertWarnings assertWarnings(boolean deduplicateExact) { + if (expectedWarnings.isEmpty() == false) { + if (deduplicateExact) { + return new AssertWarnings.DeduplicatedStrings(expectedWarnings); + } + return new AssertWarnings.ExactStrings(expectedWarnings); + } + if (expectedWarningsRegex.isEmpty() == false) { + return new AssertWarnings.AllowedRegexes(expectedWarningsRegex); + } + return new AssertWarnings.NoWarnings(); + } } } diff --git a/x-pack/plugin/esql/qa/testFixtures/src/main/java/org/elasticsearch/xpack/esql/EsqlTestUtils.java b/x-pack/plugin/esql/qa/testFixtures/src/main/java/org/elasticsearch/xpack/esql/EsqlTestUtils.java index e755ddb4d0d10..f8e04cc73cfe4 100644 --- a/x-pack/plugin/esql/qa/testFixtures/src/main/java/org/elasticsearch/xpack/esql/EsqlTestUtils.java +++ b/x-pack/plugin/esql/qa/testFixtures/src/main/java/org/elasticsearch/xpack/esql/EsqlTestUtils.java @@ -407,16 +407,6 @@ public static String randomEnrichCommand(String name, Enrich.Mode mode, String m return String.join(" | ", all); } - public static void assertWarnings(List warnings, List allowedWarnings, List allowedWarningsRegex) { - if (allowedWarningsRegex.isEmpty()) { - assertMap(warnings.stream().sorted().toList(), matchesList(allowedWarnings.stream().sorted().toList())); - } else { - for (String warning : warnings) { - assertTrue("Unexpected warning: " + warning, allowedWarningsRegex.stream().anyMatch(x -> x.matcher(warning).matches())); - } - } - } - /** * "tables" provided in the context for the LOOKUP command. If you * add to this, you must also add to {@code EsqlSpecTestCase#tables}; From 7f5a47834fbd3c3384b7e6ca888e37c04844a643 Mon Sep 17 00:00:00 2001 From: Nik Everett Date: Mon, 4 Nov 2024 14:04:34 -0500 Subject: [PATCH 2/4] Spotless --- .../elasticsearch/xpack/esql/qa/rest/RestEsqlTestCase.java | 1 - .../main/java/org/elasticsearch/xpack/esql/EsqlTestUtils.java | 4 ---- 2 files changed, 5 deletions(-) diff --git a/x-pack/plugin/esql/qa/server/src/main/java/org/elasticsearch/xpack/esql/qa/rest/RestEsqlTestCase.java b/x-pack/plugin/esql/qa/server/src/main/java/org/elasticsearch/xpack/esql/qa/rest/RestEsqlTestCase.java index 466c20524cf34..505ab3adc553b 100644 --- a/x-pack/plugin/esql/qa/server/src/main/java/org/elasticsearch/xpack/esql/qa/rest/RestEsqlTestCase.java +++ b/x-pack/plugin/esql/qa/server/src/main/java/org/elasticsearch/xpack/esql/qa/rest/RestEsqlTestCase.java @@ -54,7 +54,6 @@ import java.util.Map; import java.util.Set; import java.util.function.IntFunction; -import java.util.regex.Pattern; import static java.util.Collections.emptySet; import static java.util.Map.entry; diff --git a/x-pack/plugin/esql/qa/testFixtures/src/main/java/org/elasticsearch/xpack/esql/EsqlTestUtils.java b/x-pack/plugin/esql/qa/testFixtures/src/main/java/org/elasticsearch/xpack/esql/EsqlTestUtils.java index f8e04cc73cfe4..c5c3788fbce47 100644 --- a/x-pack/plugin/esql/qa/testFixtures/src/main/java/org/elasticsearch/xpack/esql/EsqlTestUtils.java +++ b/x-pack/plugin/esql/qa/testFixtures/src/main/java/org/elasticsearch/xpack/esql/EsqlTestUtils.java @@ -97,7 +97,6 @@ import java.util.Set; import java.util.TreeMap; import java.util.jar.JarInputStream; -import java.util.regex.Pattern; import java.util.zip.ZipEntry; import static java.util.Collections.emptyList; @@ -118,8 +117,6 @@ import static org.elasticsearch.test.ESTestCase.randomMillisUpToYear9999; import static org.elasticsearch.test.ESTestCase.randomShort; import static org.elasticsearch.test.ESTestCase.randomZone; -import static org.elasticsearch.test.ListMatcher.matchesList; -import static org.elasticsearch.test.MapMatcher.assertMap; import static org.elasticsearch.xpack.esql.core.tree.Source.EMPTY; import static org.elasticsearch.xpack.esql.core.type.DataType.INTEGER; import static org.elasticsearch.xpack.esql.core.type.DataType.NULL; @@ -129,7 +126,6 @@ import static org.elasticsearch.xpack.esql.parser.ParserUtils.ParamClassification.PATTERN; import static org.elasticsearch.xpack.esql.parser.ParserUtils.ParamClassification.VALUE; import static org.hamcrest.Matchers.instanceOf; -import static org.junit.Assert.assertTrue; public final class EsqlTestUtils { From 8bfc93d67364e5578f19905b4471dddbdf6b30c0 Mon Sep 17 00:00:00 2001 From: Nik Everett Date: Wed, 6 Nov 2024 14:06:29 -0500 Subject: [PATCH 3/4] Fix compile --- .../src/test/java/org/elasticsearch/xpack/esql/CsvTests.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/CsvTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/CsvTests.java index 4bf02d947c1e0..ecc8408b19a18 100644 --- a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/CsvTests.java +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/CsvTests.java @@ -496,7 +496,7 @@ private void assertWarnings(List warnings) { normalized.add(normW); } } - EsqlTestUtils.assertWarnings(normalized, testCase.expectedWarnings(), testCase.expectedWarningsRegex()); + testCase.assertWarnings(false).assertWarnings(normalized); } PlanRunner planRunner(BigArrays bigArrays, TestPhysicalOperationProviders physicalOperationProviders) { From e8d944e9b6b3dabc77fc1a77914e4c6ee24573a9 Mon Sep 17 00:00:00 2001 From: Nik Everett Date: Wed, 6 Nov 2024 16:24:46 -0500 Subject: [PATCH 4/4] Ternary --- .../org/elasticsearch/xpack/esql/CsvSpecReader.java | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/x-pack/plugin/esql/qa/testFixtures/src/main/java/org/elasticsearch/xpack/esql/CsvSpecReader.java b/x-pack/plugin/esql/qa/testFixtures/src/main/java/org/elasticsearch/xpack/esql/CsvSpecReader.java index a78eaf20a8fab..84e06e0c1b674 100644 --- a/x-pack/plugin/esql/qa/testFixtures/src/main/java/org/elasticsearch/xpack/esql/CsvSpecReader.java +++ b/x-pack/plugin/esql/qa/testFixtures/src/main/java/org/elasticsearch/xpack/esql/CsvSpecReader.java @@ -145,13 +145,17 @@ public List expectedWarningsRegex() { /** * How should we assert the warnings returned by ESQL. + * @param deduplicateExact Should tests configured with {@code warnings:} deduplicate + * the warnings before asserting? Normally don't do it because + * duplicate warnings are lame. We'd like to fix them all. But + * in multi-node and multi-shard tests we can emit duplicate + * warnings and it isn't worth fixing them now. */ public AssertWarnings assertWarnings(boolean deduplicateExact) { if (expectedWarnings.isEmpty() == false) { - if (deduplicateExact) { - return new AssertWarnings.DeduplicatedStrings(expectedWarnings); - } - return new AssertWarnings.ExactStrings(expectedWarnings); + return deduplicateExact + ? new AssertWarnings.DeduplicatedStrings(expectedWarnings) + : new AssertWarnings.ExactStrings(expectedWarnings); } if (expectedWarningsRegex.isEmpty() == false) { return new AssertWarnings.AllowedRegexes(expectedWarningsRegex);