Fix TextFieldBlockLoaderTests test assumption. (elastic#129361) (elastic#130682)

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 elastic#128012
Closes elastic#128011

Author: martijnvg

server/src/test/java/org/elasticsearch/index/mapper/blockloader/TextFieldBlockLoaderTests.java

@@ -27,6 +27,9 @@ public TextFieldBlockLoaderTests(Params params) {
 
     @Override
     protected Object expected(Map<String, Object> 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<String, Object> 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);

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<String, Object> 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<T> extends BaseMatcher<T> {
+
+        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<String> 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 <T> Matcher<T> prettyEqualTo(T operand) {
+            return new PrettyEqual<>(operand);
+        }
+
+    }
 }