Skip to content

Commit fe8d828

Browse files
craigtavernerfzowl
craigtaverner
authored and
fzowl
committed
Unmute flaky csv-spec tests with more logging on warnings context (elastic#137089)
These tests do not reproduce, even with reported random seeds. We are unmuting them in the hope that the additional logging will bring some clarity to the situation under which they fail.
1 parent 37cb28f commit fe8d828

File tree

4 files changed

+42
-39
lines changed

4 files changed

+42
-39
lines changed

‎muted-tests.yml‎

Lines changed: 0 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -360,9 +360,6 @@ tests:
360360
- class: org.elasticsearch.xpack.security.authc.AuthenticationServiceTests
361361
method: testInvalidToken
362362
issue: https://github.com/elastic/elasticsearch/issues/133328
363-
- class: org.elasticsearch.xpack.esql.ccq.MultiClusterSpecIT
364-
method: test {csv-spec:change_point.Values null column}
365-
issue: https://github.com/elastic/elasticsearch/issues/133334
366363
- class: org.elasticsearch.xpack.search.CrossClusterAsyncSearchIT
367364
method: testCancelViaExpirationOnRemoteResultsWithMinimizeRoundtrips
368365
issue: https://github.com/elastic/elasticsearch/issues/127302
@@ -372,21 +369,12 @@ tests:
372369
- class: org.elasticsearch.xpack.ml.integration.ClassificationIT
373370
method: testWithCustomFeatureProcessors
374371
issue: https://github.com/elastic/elasticsearch/issues/134001
375-
- class: org.elasticsearch.xpack.esql.ccq.MultiClusterSpecIT
376-
method: test {csv-spec:spatial.ConvertFromStringParseError}
377-
issue: https://github.com/elastic/elasticsearch/issues/134104
378-
- class: org.elasticsearch.xpack.esql.qa.single_node.GenerativeForkIT
379-
method: test {csv-spec:spatial_shapes.ConvertFromStringParseError}
380-
issue: https://github.com/elastic/elasticsearch/issues/134254
381372
- class: org.elasticsearch.action.admin.cluster.state.TransportClusterStateActionTests
382373
method: testGetClusterStateWithDefaultProjectOnly
383374
issue: https://github.com/elastic/elasticsearch/issues/134450
384375
- class: org.elasticsearch.xpack.esql.plugin.CanMatchIT
385376
method: testAliasFilters
386377
issue: https://github.com/elastic/elasticsearch/issues/134512
387-
- class: org.elasticsearch.xpack.esql.ccq.MultiClusterSpecIT
388-
method: test {csv-spec:spatial.ConvertCartesianFromStringParseError}
389-
issue: https://github.com/elastic/elasticsearch/issues/134635
390378
- class: org.elasticsearch.xpack.esql.analysis.AnalyzerTests
391379
method: testDenseVectorImplicitCastingKnnQueryParams
392380
issue: https://github.com/elastic/elasticsearch/issues/134639
@@ -444,9 +432,6 @@ tests:
444432
- class: org.elasticsearch.gradle.TestClustersPluginFuncTest
445433
method: override jdk usage via ES_JAVA_HOME for known jdk os incompatibilities
446434
issue: https://github.com/elastic/elasticsearch/issues/135413
447-
- class: org.elasticsearch.xpack.esql.qa.single_node.EsqlSpecIT
448-
method: test {csv-spec:spatial_shapes.ConvertCartesianShapeFromStringParseError}
449-
issue: https://github.com/elastic/elasticsearch/issues/135455
450435
- class: org.elasticsearch.xpack.esql.heap_attack.HeapAttackIT
451436
method: testAggTooManyMvLongs
452437
issue: https://github.com/elastic/elasticsearch/issues/135585
@@ -471,9 +456,6 @@ tests:
471456
- class: org.elasticsearch.test.rest.yaml.CcsCommonYamlTestSuiteIT
472457
method: test {p0=field_caps/10_basic/Field caps for number field with only doc values}
473458
issue: https://github.com/elastic/elasticsearch/issues/136244
474-
- class: org.elasticsearch.xpack.esql.qa.single_node.GenerativeForkIT
475-
method: test {csv-spec:string.SpaceNegative}
476-
issue: https://github.com/elastic/elasticsearch/issues/136249
477459
- class: org.elasticsearch.xpack.restart.FullClusterRestartIT
478460
method: testDataStreams {cluster=UPGRADED}
479461
issue: https://github.com/elastic/elasticsearch/issues/136353

‎x-pack/plugin/esql/qa/server/src/main/java/org/elasticsearch/xpack/esql/qa/rest/RestEsqlTestCase.java‎

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -405,10 +405,10 @@ public void testCSVNoHeaderMode() throws IOException {
405405
options.addHeader("Accept", "text/csv; header=absent");
406406
request.setOptions(options);
407407
Response response = performRequest(request);
408-
assertWarnings(response, new AssertWarnings.NoWarnings());
409408
HttpEntity entity = response.getEntity();
410409
String actual = Streams.copyToString(new InputStreamReader(entity.getContent(), StandardCharsets.UTF_8));
411410
assertEquals("keyword0,0\r\n", actual);
411+
assertWarnings(response, new AssertWarnings.NoWarnings(), actual);
412412
}
413413

414414
public void testOutOfRangeComparisons() throws IOException {
@@ -1433,7 +1433,7 @@ public static Map<String, Object> runEsqlSync(
14331433
if (supportsAsyncHeadersFix) {
14341434
assertNoAsyncHeaders(response);
14351435
}
1436-
assertWarnings(response, assertWarnings);
1436+
assertWarnings(response, assertWarnings, json);
14371437

14381438
return json;
14391439
}
@@ -1485,7 +1485,7 @@ public static Map<String, Object> runEsqlAsync(
14851485
if (profileLogger != null) {
14861486
profileLogger.extractProfile(json, profileEnabled);
14871487
}
1488-
assertWarnings(response, assertWarnings);
1488+
assertWarnings(response, assertWarnings, json);
14891489
json.remove("is_running"); // remove this to not mess up later map assertions
14901490
return Collections.unmodifiableMap(json);
14911491
} else {
@@ -1498,7 +1498,7 @@ public static Map<String, Object> runEsqlAsync(
14981498
if (profileLogger != null) {
14991499
profileLogger.extractProfile(json, profileEnabled);
15001500
}
1501-
assertWarnings(response, assertWarnings);
1501+
assertWarnings(response, assertWarnings, json);
15021502
// we already have the results, but let's remember them so that we can compare to async get
15031503
initialColumns = json.get("columns");
15041504
initialValues = json.get("values");
@@ -1538,7 +1538,7 @@ public static Map<String, Object> runEsqlAsync(
15381538
if (profileLogger != null) {
15391539
profileLogger.extractProfile(result, profileEnabled);
15401540
}
1541-
assertWarnings(response, assertWarnings);
1541+
assertWarnings(response, assertWarnings, result);
15421542
assertDeletable(id);
15431543
return removeAsyncProperties(result);
15441544
}
@@ -1801,12 +1801,12 @@ static String runEsqlAsTextWithFormat(RequestObjectBuilder builder, String forma
18011801
}
18021802

18031803
Response response = performRequest(request);
1804-
assertWarnings(response, new AssertWarnings.NoWarnings());
18051804
HttpEntity entity = response.getEntity();
18061805

18071806
// get the content, it could be empty because the request might have not completed
18081807
String initialValue = Streams.copyToString(new InputStreamReader(entity.getContent(), StandardCharsets.UTF_8));
18091808
String id = response.getHeader("X-Elasticsearch-Async-Id");
1809+
assertWarnings(response, new AssertWarnings.NoWarnings(), initialValue);
18101810

18111811
if (mode == SYNC) {
18121812
assertThat(id, is(emptyOrNullString()));
@@ -1855,10 +1855,10 @@ static String runEsqlAsTextWithFormat(RequestObjectBuilder builder, String forma
18551855
// if `addParam` is false, `options` will already have an `Accept` header
18561856
getRequest.setOptions(options);
18571857
response = performRequest(getRequest);
1858-
assertWarnings(response, new AssertWarnings.NoWarnings());
18591858
entity = response.getEntity();
18601859
}
18611860
String newValue = Streams.copyToString(new InputStreamReader(entity.getContent(), StandardCharsets.UTF_8));
1861+
assertWarnings(response, new AssertWarnings.NoWarnings(), newValue);
18621862

18631863
// assert initial contents, if any, are the same as async get contents
18641864
if (initialValue != null && initialValue.isEmpty() == false) {
@@ -1911,13 +1911,13 @@ static void assertNotPartial(Map<String, Object> answer) {
19111911
assertThat(reason, answer.get("is_partial"), anyOf(nullValue(), is(false)));
19121912
}
19131913

1914-
private static void assertWarnings(Response response, AssertWarnings assertWarnings) {
1914+
private static void assertWarnings(Response response, AssertWarnings assertWarnings, Object context) {
19151915
List<String> warnings = new ArrayList<>(response.getWarnings());
19161916
warnings.removeAll(mutedWarnings());
19171917
if (shouldLog()) {
19181918
LOGGER.info("RESPONSE warnings (after muted)={}", warnings);
19191919
}
1920-
assertWarnings.assertWarnings(warnings);
1920+
assertWarnings.assertWarnings(warnings, context);
19211921
}
19221922

19231923
private static Set<String> mutedWarnings() {

‎x-pack/plugin/esql/qa/testFixtures/src/main/java/org/elasticsearch/xpack/esql/AssertWarnings.java‎

Lines changed: 30 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -18,34 +18,55 @@
1818
* How should we assert the warnings returned by ESQL.
1919
*/
2020
public interface AssertWarnings {
21-
void assertWarnings(List<String> warnings);
21+
void assertWarnings(List<String> warnings, Object context);
22+
23+
default String contextMessage(Object context) {
24+
if (context == null) {
25+
return "";
26+
}
27+
StringBuilder contextBuilder = new StringBuilder();
28+
contextBuilder.append("Context: ").append(context);
29+
if (contextBuilder.length() > 1000) {
30+
contextBuilder.setLength(1000);
31+
contextBuilder.append("...(truncated)");
32+
}
33+
contextBuilder.append("\n");
34+
return contextBuilder.toString();
35+
}
2236

2337
record NoWarnings() implements AssertWarnings {
2438
@Override
25-
public void assertWarnings(List<String> warnings) {
26-
assertMap(warnings.stream().sorted().toList(), matchesList());
39+
public void assertWarnings(List<String> warnings, Object context) {
40+
assertMap(contextMessage(context), warnings.stream().sorted().toList(), matchesList());
2741
}
2842
}
2943

3044
record ExactStrings(List<String> expected) implements AssertWarnings {
3145
@Override
32-
public void assertWarnings(List<String> warnings) {
33-
assertMap(warnings.stream().sorted().toList(), matchesList(expected.stream().sorted().toList()));
46+
public void assertWarnings(List<String> warnings, Object context) {
47+
assertMap(contextMessage(context), warnings.stream().sorted().toList(), matchesList(expected.stream().sorted().toList()));
3448
}
3549
}
3650

3751
record DeduplicatedStrings(List<String> expected) implements AssertWarnings {
3852
@Override
39-
public void assertWarnings(List<String> warnings) {
40-
assertMap(warnings.stream().sorted().distinct().toList(), matchesList(expected.stream().sorted().toList()));
53+
public void assertWarnings(List<String> warnings, Object context) {
54+
assertMap(
55+
contextMessage(context),
56+
warnings.stream().sorted().distinct().toList(),
57+
matchesList(expected.stream().sorted().toList())
58+
);
4159
}
4260
}
4361

4462
record AllowedRegexes(List<Pattern> expected) implements AssertWarnings {
4563
@Override
46-
public void assertWarnings(List<String> warnings) {
64+
public void assertWarnings(List<String> warnings, Object context) {
4765
for (String warning : warnings) {
48-
assertTrue("Unexpected warning: " + warning, expected.stream().anyMatch(x -> x.matcher(warning).matches()));
66+
assertTrue(
67+
contextMessage(context) + "Unexpected warning: " + warning,
68+
expected.stream().anyMatch(x -> x.matcher(warning).matches())
69+
);
4970
}
5071
}
5172
}

‎x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/CsvTests.java‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -391,7 +391,7 @@ private void doTest() throws Exception {
391391

392392
var log = logResults() ? LOGGER : null;
393393
assertResults(expected, actualResults, testCase.ignoreOrder, log);
394-
assertWarnings(actualResults.responseHeaders().getOrDefault("Warning", List.of()));
394+
assertWarnings(actualResults.responseHeaders().getOrDefault("Warning", List.of()), actualResults);
395395
} finally {
396396
Releasables.close(() -> Iterators.map(actualResults.pages().iterator(), p -> p::releaseBlocks));
397397
// Give the breaker service some time to clear in case we got results before the rest of the driver had cleaned up
@@ -717,7 +717,7 @@ private void opportunisticallyAssertPlanSerialization(PhysicalPlan plan) {
717717
SerializationTestUtils.assertSerialization(plan, configuration);
718718
}
719719

720-
private void assertWarnings(List<String> warnings) {
720+
private void assertWarnings(List<String> warnings, Object context) {
721721
List<String> normalized = new ArrayList<>(warnings.size());
722722
for (String w : warnings) {
723723
String normW = HeaderWarning.extractWarningValueFromWarningHeader(w, false);
@@ -726,7 +726,7 @@ private void assertWarnings(List<String> warnings) {
726726
normalized.add(normW);
727727
}
728728
}
729-
testCase.assertWarnings(false).assertWarnings(normalized);
729+
testCase.assertWarnings(false).assertWarnings(normalized, context);
730730
}
731731

732732
PlanRunner planRunner(BigArrays bigArrays, TestPhysicalOperationProviders physicalOperationProviders) {

0 commit comments

Comments
 (0)