Super randomized tests for fetch fields API (#70278)

We've had a few bugs in the fields API where is doesn't behave like we'd expect. Typically this happens because it isn't obvious what we expct. So we'll try and use randomized testing to ferret out what we want. This adds a test for most field types that asserts that `fields` works similarly to `docvalues_fields`. We expect this to be true for most fields. It does so by forcing all subclasses of `MapperTestCase` to define a method that makes random values. It declares a few other hooks that subclasses can override to further randomize the test. We skip the test for a few field types that don't have doc values: * `annotated_text` * `completion` * `search_as_you_type` * `text` We should come up with some way to test these without doc values, even if it isn't as nice. But that is a problem for another time, I think. We skip the test for a few more types just because I wanted to cut this PR in half so we could get to reviewing it earlier. We'll get to those in a follow up change. I've filed a few bugs for things that are inconsistent with `docvalues_fields`. Typically that means that we have to limit the random values that we generate to those that *do* round trip properly.
elastic · Mar 24, 2021 · 91c700b · 91c700b
1 parent afde502
commit 91c700b
Show file tree

Hide file tree

Showing 42 changed files with 544 additions and 36 deletions.
diff --git a/...pper-extras/src/test/java/org/elasticsearch/index/mapper/RankFeatureFieldMapperTests.java b/...pper-extras/src/test/java/org/elasticsearch/index/mapper/RankFeatureFieldMapperTests.java
@@ -136,4 +136,10 @@ public void testRejectMultiValuedFields() throws MapperParsingException, IOExcep
         assertEquals("[rank_feature] fields do not support indexing multiple values for the same field [foo.field] in the same document",
                 e.getCause().getMessage());
     }
+
+    @Override
+    protected Object generateRandomInputValue(MappedFieldType ft) {
+        assumeFalse("Test implemented in a follow up", true);
+        return null;
+    }
 }
diff --git a/...per-extras/src/test/java/org/elasticsearch/index/mapper/RankFeaturesFieldMapperTests.java b/...per-extras/src/test/java/org/elasticsearch/index/mapper/RankFeaturesFieldMapperTests.java
@@ -135,4 +135,10 @@ public void testRejectMultiValuedFields() throws MapperParsingException, IOExcep
         assertEquals("[rank_features] fields do not support indexing multiple values for the same rank feature [foo.field.bar] in " +
                 "the same document", e.getCause().getMessage());
     }
+
+    @Override
+    protected Object generateRandomInputValue(MappedFieldType ft) {
+        assumeFalse("Test implemented in a follow up", true);
+        return null;
+    }
 }
diff --git a/...pper-extras/src/test/java/org/elasticsearch/index/mapper/ScaledFloatFieldMapperTests.java b/...pper-extras/src/test/java/org/elasticsearch/index/mapper/ScaledFloatFieldMapperTests.java
@@ -271,4 +271,30 @@ public void testRejectIndexOptions() {
             containsString("Failed to parse mapping: unknown parameter [index_options] on mapper [field] of type [scaled_float]"));
     }
 
+    @Override
+    protected void randomFetchTestFieldConfig(XContentBuilder b) throws IOException {
+        // Large floats are a terrible idea but the round trip should still work no matter how badly you configure the field
+        b.field("type", "scaled_float").field("scaling_factor", randomDoubleBetween(0, Float.MAX_VALUE, true));
+    }
+
+    @Override
+    protected Object generateRandomInputValue(MappedFieldType ft) {
+        /*
+         * randomDoubleBetween will smear the random values out across a huge
+         * range of valid values.
+         */
+        double v = randomDoubleBetween(-Float.MAX_VALUE, Float.MAX_VALUE, true);
+        switch (between(0, 3)) {
+            case 0:
+                return v;
+            case 1:
+                return (float) v;
+            case 2:
+                return Double.toString(v);
+            case 3:
+                return Float.toString((float) v);
+            default:
+                throw new IllegalArgumentException();
+        }
+    }
 }
diff --git a/...-extras/src/test/java/org/elasticsearch/index/mapper/SearchAsYouTypeFieldMapperTests.java b/...-extras/src/test/java/org/elasticsearch/index/mapper/SearchAsYouTypeFieldMapperTests.java
@@ -745,4 +745,10 @@ private static PrefixFieldMapper getPrefixFieldMapper(DocumentMapper defaultMapp
         assertThat(mapper, instanceOf(PrefixFieldMapper.class));
         return (PrefixFieldMapper) mapper;
     }
+
+    @Override
+    protected Object generateRandomInputValue(MappedFieldType ft) {
+        assumeFalse("We don't have doc values or fielddata", true);
+        return null;
+    }
 }
diff --git a/...apper-extras/src/test/java/org/elasticsearch/index/mapper/TokenCountFieldMapperTests.java b/...apper-extras/src/test/java/org/elasticsearch/index/mapper/TokenCountFieldMapperTests.java
@@ -177,4 +177,20 @@ private ParseContext.Document parseDocument(DocumentMapper mapper, SourceToParse
         return mapper.parse(request)
             .docs().stream().findFirst().orElseThrow(() -> new IllegalStateException("Test object not parsed"));
     }
+
+    @Override
+    protected String generateRandomInputValue(MappedFieldType ft) {
+        int words = between(1, 1000);
+        StringBuilder b = new StringBuilder(words * 5);
+        b.append(randomAlphaOfLength(4));
+        for (int w = 1; w < words; w++) {
+            b.append(' ').append(randomAlphaOfLength(4));
+        }
+        return b.toString();
+    }
+
+    @Override
+    protected void randomFetchTestFieldConfig(XContentBuilder b) throws IOException {
+        b.field("type", "token_count").field("analyzer", "standard");
+    }
 }
diff --git a/...ysis-icu/src/main/java/org/elasticsearch/index/mapper/ICUCollationKeywordFieldMapper.java b/...ysis-icu/src/main/java/org/elasticsearch/index/mapper/ICUCollationKeywordFieldMapper.java
@@ -136,7 +136,7 @@ public Query regexpQuery(String value, int syntaxFlags, int matchFlags, int maxD
             throw new UnsupportedOperationException("[regexp] queries are not supported on [" + CONTENT_TYPE + "] fields.");
         }
 
-        public static DocValueFormat COLLATE_FORMAT = new DocValueFormat() {
+        public static final DocValueFormat COLLATE_FORMAT = new DocValueFormat() {
             @Override
             public String getWriteableName() {
                 return "collate";

diff --git a/...icu/src/test/java/org/elasticsearch/index/mapper/ICUCollationKeywordFieldMapperTests.java b/...icu/src/test/java/org/elasticsearch/index/mapper/ICUCollationKeywordFieldMapperTests.java
@@ -10,6 +10,7 @@
 import com.ibm.icu.text.Collator;
 import com.ibm.icu.text.RawCollationKey;
 import com.ibm.icu.util.ULocale;
+
 import org.apache.lucene.index.DocValuesType;
 import org.apache.lucene.index.IndexOptions;
 import org.apache.lucene.index.IndexableField;
@@ -280,4 +281,13 @@ public void testUpdateIgnoreAbove() throws IOException {
         assertEquals(0, fields.length);
     }
 
+    @Override
+    protected String generateRandomInputValue(MappedFieldType ft) {
+        assumeFalse("docvalue_fields is broken", true);
+        // https://github.com/elastic/elasticsearch/issues/70276
+        /*
+         * docvalue_fields loads garbage bytes.
+         */
+        return null;
+    }
 }
diff --git a/...Test/java/org/elasticsearch/index/mapper/annotatedtext/AnnotatedTextFieldMapperTests.java b/...Test/java/org/elasticsearch/index/mapper/annotatedtext/AnnotatedTextFieldMapperTests.java
@@ -36,6 +36,7 @@
 import org.elasticsearch.index.analysis.StandardTokenizerFactory;
 import org.elasticsearch.index.analysis.TokenFilterFactory;
 import org.elasticsearch.index.mapper.DocumentMapper;
+import org.elasticsearch.index.mapper.MappedFieldType;
 import org.elasticsearch.index.mapper.MapperParsingException;
 import org.elasticsearch.index.mapper.MapperService;
 import org.elasticsearch.index.mapper.MapperTestCase;
@@ -552,4 +553,9 @@ public void testAnalyzedFieldPositionIncrementWithoutPositions() {
         }
     }
 
+    @Override
+    protected Object generateRandomInputValue(MappedFieldType ft) {
+        assumeFalse("annotated_text doesn't have fielddata so we can't check against anything here.", true);
+        return null;
+    }
 }
diff --git a/...murmur3/src/test/java/org/elasticsearch/index/mapper/murmur3/Murmur3FieldMapperTests.java b/...murmur3/src/test/java/org/elasticsearch/index/mapper/murmur3/Murmur3FieldMapperTests.java
@@ -13,6 +13,7 @@
 import org.apache.lucene.index.IndexableField;
 import org.elasticsearch.common.xcontent.XContentBuilder;
 import org.elasticsearch.index.mapper.DocumentMapper;
+import org.elasticsearch.index.mapper.MappedFieldType;
 import org.elasticsearch.index.mapper.MapperTestCase;
 import org.elasticsearch.index.mapper.ParsedDocument;
 import org.elasticsearch.plugin.mapper.MapperMurmur3Plugin;
@@ -56,4 +57,9 @@ public void testDefaults() throws Exception {
         assertEquals(DocValuesType.SORTED_NUMERIC, field.fieldType().docValuesType());
     }
 
+    @Override
+    protected Object generateRandomInputValue(MappedFieldType ft) {
+        assumeFalse("Test implemented in a follow up", true);
+        return null;
+    }
 }
diff --git a/server/src/main/java/org/elasticsearch/index/mapper/RangeType.java b/server/src/main/java/org/elasticsearch/index/mapper/RangeType.java
@@ -546,6 +546,10 @@ public Query intersectsQuery(String field, Object from, Object to, boolean inclu
         }
     };
 
+    public final String name;
+    private final NumberFieldMapper.NumberType numberType;
+    public final LengthType lengthType;
+
     RangeType(String name, LengthType lengthType) {
         this.name = name;
         this.numberType = null;
@@ -699,9 +703,9 @@ public final Mapper.TypeParser parser() {
         return new FieldMapper.TypeParser((n, c) -> new RangeFieldMapper.Builder(n, this, c.getSettings()));
     }
 
-    public final String name;
-    private final NumberFieldMapper.NumberType numberType;
-    public final LengthType lengthType;
+    NumberFieldMapper.NumberType numberType() {
+        return numberType;
+    }
 
     public enum LengthType {
         FIXED_4 {

diff --git a/server/src/test/java/org/elasticsearch/index/mapper/BinaryFieldMapperTests.java b/server/src/test/java/org/elasticsearch/index/mapper/BinaryFieldMapperTests.java
@@ -118,4 +118,16 @@ public void testStoredValue() throws IOException {
             assertEquals(new BytesArray(value), originalValue);
         }
     }
+
+    @Override
+    protected Object generateRandomInputValue(MappedFieldType ft) {
+        assumeFalse("We can't parse the binary doc values we send", true);
+        // AwaitsFix https://github.com/elastic/elasticsearch/issues/70244
+        return null;
+    }
+
+    @Override
+    protected void randomFetchTestFieldConfig(XContentBuilder b) throws IOException {
+        b.field("type", "binary").field("doc_values", true); // enable doc_values so the test is happy
+    }
 }
diff --git a/server/src/test/java/org/elasticsearch/index/mapper/BooleanFieldMapperTests.java b/server/src/test/java/org/elasticsearch/index/mapper/BooleanFieldMapperTests.java
@@ -162,4 +162,20 @@ public void testDocValues() throws Exception {
         assertEquals(DocValuesType.NONE, fields[0].fieldType().docValuesType());
         assertEquals(DocValuesType.SORTED_NUMERIC, fields[1].fieldType().docValuesType());
     }
+
+    @Override
+    protected Object generateRandomInputValue(MappedFieldType ft) {
+        switch (between(0, 3)) {
+            case 0:
+                return randomBoolean();
+            case 1:
+                return randomBoolean() ? "true" : "false";
+            case 2:
+                return randomBoolean() ? "true" : "";
+            case 3:
+                return randomBoolean() ? "true" : null;
+            default:
+                throw new IllegalStateException();
+        }
+    }
 }
diff --git a/server/src/test/java/org/elasticsearch/index/mapper/ByteFieldMapperTests.java b/server/src/test/java/org/elasticsearch/index/mapper/ByteFieldMapperTests.java
@@ -35,4 +35,15 @@ protected List<OutOfRangeSpec> outOfRangeSpecs() {
     protected void minimalMapping(XContentBuilder b) throws IOException {
         b.field("type", "byte");
     }
+
+    @Override
+    protected Number randomNumber() {
+        if (randomBoolean()) {
+            return randomByte();
+        }
+        if (randomBoolean()) {
+            return randomDouble();
+        }
+        return randomDoubleBetween(Byte.MIN_VALUE, Byte.MAX_VALUE, true);
+    }
 }
diff --git a/server/src/test/java/org/elasticsearch/index/mapper/CompletionFieldMapperTests.java b/server/src/test/java/org/elasticsearch/index/mapper/CompletionFieldMapperTests.java
@@ -778,4 +778,9 @@ protected V featureValueOf(T actual) {
         };
     }
 
+    @Override
+    protected Object generateRandomInputValue(MappedFieldType ft) {
+        assumeFalse("We don't have doc values or fielddata", true);
+        return null;
+    }
 }
diff --git a/server/src/test/java/org/elasticsearch/index/mapper/DateFieldMapperTests.java b/server/src/test/java/org/elasticsearch/index/mapper/DateFieldMapperTests.java
@@ -15,6 +15,7 @@
 import org.elasticsearch.common.time.DateFormatter;
 import org.elasticsearch.common.time.DateUtils;
 import org.elasticsearch.common.xcontent.XContentBuilder;
+import org.elasticsearch.index.mapper.DateFieldMapper.DateFieldType;
 import org.elasticsearch.index.termvectors.TermVectorsService;
 import org.elasticsearch.search.DocValueFormat;
 
@@ -500,6 +501,50 @@ public void testFetchNanosFromFixedNanosFormatted() throws IOException {
         );
     }
 
+    @Override
+    protected void randomFetchTestFieldConfig(XContentBuilder b) throws IOException {
+        b.field("type", randomBoolean() ? "date" : "date_nanos");
+    }
+
+    @Override
+    protected String randomFetchTestFormat() {
+        // TODO more choices! The test should work fine even for choices that throw out a ton of precision.
+        switch (randomInt(2)) {
+            case 0:
+                return null;
+            case 1:
+                return "epoch_millis";
+            case 2:
+                return "iso8601";
+            default:
+                throw new IllegalStateException();
+        }
+    }
+
+    @Override
+    protected Object generateRandomInputValue(MappedFieldType ft) {
+        switch (((DateFieldType) ft).resolution()) {
+            case MILLISECONDS:
+                if (randomBoolean()) {
+                    return randomIs8601Nanos(MAX_ISO_DATE);
+                }
+                return randomLongBetween(0, Long.MAX_VALUE);
+            case NANOSECONDS:
+                switch (randomInt(2)) {
+                    case 0:
+                        return randomLongBetween(0, MAX_NANOS);
+                    case 1:
+                        return randomIs8601Nanos(MAX_NANOS);
+                    case 2:
+                        return new BigDecimal(randomDecimalNanos(MAX_MILLIS_DOUBLE_NANOS_KEEPS_PRECISION));
+                    default:
+                        throw new IllegalStateException();
+                }
+            default:
+                throw new IllegalStateException();
+        }
+    }
+
     private MapperService dateNanosMapperService() throws IOException {
         return createMapperService(mapping(b -> b.startObject("field").field("type", "date_nanos").endObject()));
     }

diff --git a/server/src/test/java/org/elasticsearch/index/mapper/DoubleFieldMapperTests.java b/server/src/test/java/org/elasticsearch/index/mapper/DoubleFieldMapperTests.java
@@ -36,4 +36,15 @@ protected List<OutOfRangeSpec> outOfRangeSpecs() {
     protected void minimalMapping(XContentBuilder b) throws IOException {
         b.field("type", "double");
     }
+
+    @Override
+    protected Number randomNumber() {
+        /*
+         * The source parser and doc values round trip will both increase
+         * the precision to 64 bits if the value is less precise.
+         * randomDoubleBetween will smear the values out across a wide
+         * range of valid values.
+         */
+        return randomBoolean() ? randomDoubleBetween(-Double.MAX_VALUE, Double.MAX_VALUE, true) : randomFloat();
+    }
 }
diff --git a/server/src/test/java/org/elasticsearch/index/mapper/FloatFieldMapperTests.java b/server/src/test/java/org/elasticsearch/index/mapper/FloatFieldMapperTests.java
@@ -36,4 +36,15 @@ protected List<OutOfRangeSpec> outOfRangeSpecs() {
     protected void minimalMapping(XContentBuilder b) throws IOException {
         b.field("type", "float");
     }
+
+    @Override
+    protected Number randomNumber() {
+        /*
+         * The source parser and doc values round trip will both reduce
+         * the precision to 32 bits if the value is more precise.
+         * randomDoubleBetween will smear the values out across a wide
+         * range of valid values.
+         */
+        return randomBoolean() ? randomDoubleBetween(-Float.MAX_VALUE, Float.MAX_VALUE, true) : randomFloat();
+    }
 }
diff --git a/server/src/test/java/org/elasticsearch/index/mapper/GeoPointFieldMapperTests.java b/server/src/test/java/org/elasticsearch/index/mapper/GeoPointFieldMapperTests.java
@@ -340,4 +340,10 @@ protected void assertSearchable(MappedFieldType fieldType) {
         //always searchable even if it uses TextSearchInfo.NONE
         assertTrue(fieldType.isSearchable());
     }
+
+    @Override
+    protected Object generateRandomInputValue(MappedFieldType ft) {
+        assumeFalse("Test implemented in a follow up", true);
+        return null;
+    }
 }
diff --git a/server/src/test/java/org/elasticsearch/index/mapper/GeoShapeFieldMapperTests.java b/server/src/test/java/org/elasticsearch/index/mapper/GeoShapeFieldMapperTests.java
@@ -226,4 +226,10 @@ protected void assertSearchable(MappedFieldType fieldType) {
         //always searchable even if it uses TextSearchInfo.NONE
         assertTrue(fieldType.isSearchable());
     }
+
+    @Override
+    protected Object generateRandomInputValue(MappedFieldType ft) {
+        assumeFalse("Test implemented in a follow up", true);
+        return null;
+    }
 }
diff --git a/server/src/test/java/org/elasticsearch/index/mapper/HalfFloatFieldMapperTests.java b/server/src/test/java/org/elasticsearch/index/mapper/HalfFloatFieldMapperTests.java
@@ -8,6 +8,7 @@
 
 package org.elasticsearch.index.mapper;
 
+import org.apache.lucene.document.HalfFloatPoint;
 import org.elasticsearch.common.xcontent.XContentBuilder;
 import org.elasticsearch.index.mapper.NumberFieldMapper.NumberType;
 import org.elasticsearch.index.mapper.NumberFieldTypeTests.OutOfRangeSpec;
@@ -38,4 +39,16 @@ protected List<OutOfRangeSpec> outOfRangeSpecs() {
     protected void minimalMapping(XContentBuilder b) throws IOException {
         b.field("type", "half_float");
     }
+
+    @Override
+    protected Number randomNumber() {
+        /*
+         * The native valueFetcher returns 32 bits of precision but the
+         * doc values fetcher returns 16 bits of precision. To make it
+         * all line up we round here instead of in the fetcher. This bug
+         * is tracked in:
+         * https://github.com/elastic/elasticsearch/issues/70260
+         */
+        return HalfFloatPoint.sortableShortToHalfFloat(HalfFloatPoint.halfFloatToSortableShort(randomFloat()));
+    }
 }
diff --git a/server/src/test/java/org/elasticsearch/index/mapper/IntegerFieldMapperTests.java b/server/src/test/java/org/elasticsearch/index/mapper/IntegerFieldMapperTests.java
@@ -36,4 +36,15 @@ protected List<OutOfRangeSpec> outOfRangeSpecs() {
     protected void minimalMapping(XContentBuilder b) throws IOException {
         b.field("type", "integer");
     }
+
+    @Override
+    protected Number randomNumber() {
+        if (randomBoolean()) {
+            return randomInt();
+        }
+        if (randomBoolean()) {
+            return randomDouble();
+        }
+        return randomDoubleBetween(Integer.MIN_VALUE, Integer.MAX_VALUE, true);
+    }
 }