From 5a7ec5f1a95574cebcf341fd52853171484fa0b1 Mon Sep 17 00:00:00 2001 From: Martijn van Groningen Date: Mon, 7 Jul 2025 05:59:13 +0200 Subject: [PATCH] Fix TextFieldBlockLoaderTests test assumption. (#129361) Test assumes that values from keyword doc values fields are in sorted order, but that is no longer the case and values are sometimes in order as was provided during indexing. Actual fix was to check for `synthetic_source_keep` instead of `ignore_above`. Also improved blockLoaderResult assertion by making it readable when it fails. Closes #128012 Closes #128011 --- .../TextFieldBlockLoaderTests.java | 6 +- .../index/mapper/BlockLoaderTestRunner.java | 93 ++++++++++++++++++- 2 files changed, 96 insertions(+), 3 deletions(-) diff --git a/server/src/test/java/org/elasticsearch/index/mapper/blockloader/TextFieldBlockLoaderTests.java b/server/src/test/java/org/elasticsearch/index/mapper/blockloader/TextFieldBlockLoaderTests.java index 77c42740451ee..ce5482b15b0ee 100644 --- a/server/src/test/java/org/elasticsearch/index/mapper/blockloader/TextFieldBlockLoaderTests.java +++ b/server/src/test/java/org/elasticsearch/index/mapper/blockloader/TextFieldBlockLoaderTests.java @@ -27,6 +27,9 @@ public TextFieldBlockLoaderTests(Params params) { @Override protected Object expected(Map fieldMapping, Object value, TestContext testContext) { + logger.info("field mapping={}", fieldMapping); + logger.info("value={}", value); + logger.info("params={}", params.toString()); return expectedValue(fieldMapping, value, params, testContext); } @@ -82,7 +85,8 @@ public static Object expectedValue(Map fieldMapping, Object valu .map(BytesRef::new) .collect(Collectors.toList()); - if (store == false) { + String ssk = (String) keywordMultiFieldMapping.get("synthetic_source_keep"); + if (store == false && "arrays".equals(ssk) == false) { // using doc_values for synthetic source indexed = new ArrayList<>(new HashSet<>(indexed)); indexed.sort(BytesRef::compareTo); diff --git a/test/framework/src/main/java/org/elasticsearch/index/mapper/BlockLoaderTestRunner.java b/test/framework/src/main/java/org/elasticsearch/index/mapper/BlockLoaderTestRunner.java index eb5bbea9e6bba..e35a53c0ecca8 100644 --- a/test/framework/src/main/java/org/elasticsearch/index/mapper/BlockLoaderTestRunner.java +++ b/test/framework/src/main/java/org/elasticsearch/index/mapper/BlockLoaderTestRunner.java @@ -13,6 +13,7 @@ import org.apache.lucene.index.LeafReaderContext; import org.apache.lucene.store.Directory; import org.apache.lucene.tests.index.RandomIndexWriter; +import org.apache.lucene.util.BytesRef; import org.elasticsearch.common.bytes.BytesReference; import org.elasticsearch.index.IndexSettings; import org.elasticsearch.index.fieldvisitor.StoredFieldLoader; @@ -21,14 +22,20 @@ import org.elasticsearch.search.lookup.SearchLookup; import org.elasticsearch.xcontent.XContentBuilder; import org.elasticsearch.xcontent.XContentType; -import org.junit.Assert; +import org.hamcrest.BaseMatcher; +import org.hamcrest.Description; +import org.hamcrest.Matcher; import java.io.IOException; +import java.lang.reflect.Array; +import java.util.ArrayList; +import java.util.List; import java.util.Map; import java.util.Set; import static org.apache.lucene.tests.util.LuceneTestCase.newDirectory; import static org.apache.lucene.tests.util.LuceneTestCase.random; +import static org.elasticsearch.index.mapper.BlockLoaderTestRunner.PrettyEqual.prettyEqualTo; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.equalTo; @@ -44,7 +51,7 @@ public void runTest(MapperService mapperService, Map document, O var documentXContent = XContentBuilder.builder(XContentType.JSON.xContent()).map(document); Object blockLoaderResult = setupAndInvokeBlockLoader(mapperService, documentXContent, blockLoaderFieldName); - Assert.assertEquals(expected, blockLoaderResult); + assertThat(blockLoaderResult, prettyEqualTo(expected)); } private Object setupAndInvokeBlockLoader(MapperService mapperService, XContentBuilder document, String fieldName) throws IOException { @@ -145,4 +152,86 @@ public FieldNamesFieldMapper.FieldNamesFieldType fieldNames() { } }); } + + // Copied from org.hamcrest.core.IsEqual and modified to pretty print failure when bytesref + static class PrettyEqual extends BaseMatcher { + + private final Object expectedValue; + + PrettyEqual(T equalArg) { + expectedValue = equalArg; + } + + @Override + public boolean matches(Object actualValue) { + return areEqual(actualValue, expectedValue); + } + + @Override + public void describeTo(Description description) { + description.appendValue(attemptMakeReadable(expectedValue)); + } + + @Override + public void describeMismatch(Object item, Description description) { + super.describeMismatch(attemptMakeReadable(item), description); + } + + private static boolean areEqual(Object actual, Object expected) { + if (actual == null) { + return expected == null; + } + + if (expected != null && isArray(actual)) { + return isArray(expected) && areArraysEqual(actual, expected); + } + + return actual.equals(expected); + } + + private static boolean areArraysEqual(Object actualArray, Object expectedArray) { + return areArrayLengthsEqual(actualArray, expectedArray) && areArrayElementsEqual(actualArray, expectedArray); + } + + private static boolean areArrayLengthsEqual(Object actualArray, Object expectedArray) { + return Array.getLength(actualArray) == Array.getLength(expectedArray); + } + + private static boolean areArrayElementsEqual(Object actualArray, Object expectedArray) { + for (int i = 0; i < Array.getLength(actualArray); i++) { + if (areEqual(Array.get(actualArray, i), Array.get(expectedArray, i)) == false) { + return false; + } + } + return true; + } + + private static boolean isArray(Object o) { + return o.getClass().isArray(); + } + + // Attempt to make assertions readable: + static Object attemptMakeReadable(Object expected) { + try { + if (expected instanceof BytesRef bytesRef) { + expected = bytesRef.utf8ToString(); + } else if (expected instanceof List list && list.getFirst() instanceof BytesRef) { + List expectedList = new ArrayList<>(list.size()); + for (Object e : list) { + expectedList.add(((BytesRef) e).utf8ToString()); + } + expected = expectedList; + } + return expected; + } catch (Exception | AssertionError e) { + // ip/geo fields can't be converted to strings: + return expected; + } + } + + public static Matcher prettyEqualTo(T operand) { + return new PrettyEqual<>(operand); + } + + } }