Skip to content

Commit 969dd4a

Browse files
craigtavernercraigtaverner
authored
Unmute flaky csv-spec tests with more logging on warnings context (elastic#137089) (elastic#137107)
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 cadb304 commit 969dd4a

File tree

4 files changed

+42
-36
lines changed

4 files changed

+42
-36
lines changed

‎muted-tests.yml‎

Lines changed: 0 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -414,9 +414,6 @@ tests:
414414
- class: org.elasticsearch.xpack.security.authc.AuthenticationServiceTests
415415
method: testInvalidToken
416416
issue: https://github.com/elastic/elasticsearch/issues/133328
417-
- class: org.elasticsearch.xpack.esql.ccq.MultiClusterSpecIT
418-
method: test {csv-spec:change_point.Values null column}
419-
issue: https://github.com/elastic/elasticsearch/issues/133334
420417
- class: org.elasticsearch.xpack.search.CrossClusterAsyncSearchIT
421418
method: testCancelViaExpirationOnRemoteResultsWithMinimizeRoundtrips
422419
issue: https://github.com/elastic/elasticsearch/issues/127302
@@ -438,18 +435,12 @@ tests:
438435
- class: org.elasticsearch.xpack.esql.ccq.MultiClusterSpecIT
439436
method: test {csv-spec:fork.ForkBeforeStats}
440437
issue: https://github.com/elastic/elasticsearch/issues/134100
441-
- class: org.elasticsearch.xpack.esql.ccq.MultiClusterSpecIT
442-
method: test {csv-spec:spatial.ConvertFromStringParseError}
443-
issue: https://github.com/elastic/elasticsearch/issues/134104
444438
- class: org.elasticsearch.xpack.esql.ccq.MultiClusterSpecIT
445439
method: test {csv-spec:fork.ForkWithMixOfCommands}
446440
issue: https://github.com/elastic/elasticsearch/issues/134135
447441
- class: org.elasticsearch.xpack.esql.qa.single_node.GenerativeForkIT
448442
method: test {csv-spec:inlinestats.MultiIndexInlinestatsOfMultiTypedField}
449443
issue: https://github.com/elastic/elasticsearch/issues/133973
450-
- class: org.elasticsearch.xpack.esql.qa.single_node.GenerativeForkIT
451-
method: test {csv-spec:spatial_shapes.ConvertFromStringParseError}
452-
issue: https://github.com/elastic/elasticsearch/issues/134254
453444
- class: org.elasticsearch.xpack.esql.expression.function.scalar.score.DecayTests
454445
method: "testEvaluateBlockWithNulls {TestCase=<double>, <double>, <double>, <_source> #11}"
455446
issue: https://github.com/elastic/elasticsearch/issues/134447
@@ -468,9 +459,6 @@ tests:
468459
- class: org.elasticsearch.xpack.esql.ccq.MultiClusterSpecIT
469460
method: test {csv-spec:fork.FiveFork}
470461
issue: https://github.com/elastic/elasticsearch/issues/134560
471-
- class: org.elasticsearch.xpack.esql.ccq.MultiClusterSpecIT
472-
method: test {csv-spec:spatial.ConvertCartesianFromStringParseError}
473-
issue: https://github.com/elastic/elasticsearch/issues/134635
474462
- class: org.elasticsearch.xpack.esql.analysis.AnalyzerTests
475463
method: testDenseVectorImplicitCastingKnnQueryParams
476464
issue: https://github.com/elastic/elasticsearch/issues/134639
@@ -549,9 +537,6 @@ tests:
549537
- class: org.elasticsearch.gradle.TestClustersPluginFuncTest
550538
method: override jdk usage via ES_JAVA_HOME for known jdk os incompatibilities
551539
issue: https://github.com/elastic/elasticsearch/issues/135413
552-
- class: org.elasticsearch.xpack.esql.qa.single_node.EsqlSpecIT
553-
method: test {csv-spec:spatial_shapes.ConvertCartesianShapeFromStringParseError}
554-
issue: https://github.com/elastic/elasticsearch/issues/135455
555540
- class: org.elasticsearch.xpack.esql.inference.textembedding.TextEmbeddingOperatorTests
556541
method: testSimpleCircuitBreaking
557542
issue: https://github.com/elastic/elasticsearch/issues/135569

‎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
@@ -387,7 +387,7 @@ private void doTest() throws Exception {
387387

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

676-
private void assertWarnings(List<String> warnings) {
676+
private void assertWarnings(List<String> warnings, Object context) {
677677
List<String> normalized = new ArrayList<>(warnings.size());
678678
for (String w : warnings) {
679679
String normW = HeaderWarning.extractWarningValueFromWarningHeader(w, false);
@@ -682,7 +682,7 @@ private void assertWarnings(List<String> warnings) {
682682
normalized.add(normW);
683683
}
684684
}
685-
testCase.assertWarnings(false).assertWarnings(normalized);
685+
testCase.assertWarnings(false).assertWarnings(normalized, context);
686686
}
687687

688688
PlanRunner planRunner(BigArrays bigArrays, TestPhysicalOperationProviders physicalOperationProviders) {

0 commit comments

Comments
 (0)