Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Super randomized tests for fetch fields API #70278

Merged
merged 24 commits into from
Mar 24, 2021
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
import java.util.Arrays;
import java.util.Collection;
import java.util.List;
import java.util.function.Supplier;

import static org.hamcrest.Matchers.instanceOf;

Expand Down Expand Up @@ -136,4 +137,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 Supplier<? extends Object> randomFetchTestValueVendor(MappedFieldType ft) {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I chose a Supplier here because we have a test that tests many values at once and I wanted to be able to build many values that have a similar "theme". Check out the tests for dates for an example of where this is useful.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you explain this more? I looked at the date tests but couldn't tell where having a 'supplier' was critical.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rereading this I don't think it really is critical. It was when was sorting the values and deduping them, but it isn't really critical any more. It is a bit more realistic to generate similarly themed values though. But we don't really need it any more. What do you think?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1 to making this just an Object

assumeFalse("Test implemented in a follow up", true);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did this because I wanted to be able to open the PR and for everything to compile. I do expect to handle this in a follow up.

return null;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import java.util.Collection;
import java.util.List;
import java.util.Map;
import java.util.function.Supplier;

public class RankFeaturesFieldMapperTests extends MapperTestCase {

Expand Down Expand Up @@ -135,4 +136,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 Supplier<? extends Object> randomFetchTestValueVendor(MappedFieldType ft) {
assumeFalse("Test implemented in a follow up", true);
return null;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
import java.util.Arrays;
import java.util.Collection;
import java.util.List;
import java.util.function.Supplier;

import static java.util.Collections.singletonList;
import static org.hamcrest.Matchers.containsString;
Expand Down Expand Up @@ -271,4 +272,22 @@ 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 Supplier<? extends Object> randomFetchTestValueVendor(MappedFieldType ft) {
/*
* randomDoubleBetween will smear the random values out across a huge
* range of valid values.
*/
Supplier<Double> values = () -> randomDoubleBetween(-Float.MAX_VALUE, Float.MAX_VALUE, true);
if (randomBoolean()) {
return () -> values.get().floatValue();
}
return values;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@
import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.function.Supplier;
import java.util.stream.Collectors;
import java.util.stream.Stream;

Expand Down Expand Up @@ -745,4 +746,10 @@ private static PrefixFieldMapper getPrefixFieldMapper(DocumentMapper defaultMapp
assertThat(mapper, instanceOf(PrefixFieldMapper.class));
return (PrefixFieldMapper) mapper;
}

@Override
protected Supplier<? extends Object> randomFetchTestValueVendor(MappedFieldType ft) {
assumeFalse("We don't have doc values or fielddata", true);
return null;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
import java.util.Collection;
import java.util.Collections;
import java.util.Map;
import java.util.function.Supplier;

import static org.hamcrest.Matchers.equalTo;

Expand Down Expand Up @@ -177,4 +178,22 @@ private ParseContext.Document parseDocument(DocumentMapper mapper, SourceToParse
return mapper.parse(request)
.docs().stream().findFirst().orElseThrow(() -> new IllegalStateException("Test object not parsed"));
}

@Override
protected Supplier<? extends Object> randomFetchTestValueVendor(MappedFieldType ft) {
return () -> {
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");
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
import java.util.Arrays;
import java.util.Collection;
import java.util.List;
import java.util.function.Supplier;

import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.equalTo;
Expand Down Expand Up @@ -280,4 +281,13 @@ public void testUpdateIgnoreAbove() throws IOException {
assertEquals(0, fields.length);
}

@Override
protected Supplier<? extends Object> randomFetchTestValueVendor(MappedFieldType ft) {
assumeFalse("docvalue_fields is broken", true);
// https://github.com/elastic/elasticsearch/issues/70276
/*
* docvalue_fields loads garbage bytes.
*/
return () -> randomAlphaOfLength(5);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -52,6 +53,7 @@
import java.util.HashSet;
import java.util.Map;
import java.util.Set;
import java.util.function.Supplier;

import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.equalTo;
Expand Down Expand Up @@ -552,4 +554,9 @@ public void testAnalyzedFieldPositionIncrementWithoutPositions() {
}
}

@Override
protected Supplier<? extends Object> randomFetchTestValueVendor(MappedFieldType ft) {
assumeFalse("annotated_text doesn't have fielddata so we can't check against anything here.", true);
return null;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -22,6 +23,7 @@
import java.util.Arrays;
import java.util.Collection;
import java.util.List;
import java.util.function.Supplier;

public class Murmur3FieldMapperTests extends MapperTestCase {

Expand Down Expand Up @@ -56,4 +58,9 @@ public void testDefaults() throws Exception {
assertEquals(DocValuesType.SORTED_NUMERIC, field.fieldType().docValuesType());
}

@Override
protected Supplier<? extends Object> randomFetchTestValueVendor(MappedFieldType ft) {
assumeFalse("Test implemented in a follow up", true);
return null;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,12 @@
import org.elasticsearch.common.compress.CompressorFactory;
import org.elasticsearch.common.io.stream.BytesStreamOutput;
import org.elasticsearch.common.xcontent.XContentBuilder;
import org.elasticsearch.search.DocValueFormat;

import java.io.IOException;
import java.io.OutputStream;
import java.util.Arrays;
import java.util.function.Supplier;

import static org.hamcrest.Matchers.instanceOf;

Expand Down Expand Up @@ -118,4 +120,23 @@ public void testStoredValue() throws IOException {
assertEquals(new BytesArray(value), originalValue);
}
}

@Override
protected Supplier<? extends Object> randomFetchTestValueVendor(MappedFieldType ft) {
assumeFalse("We can't parse the binary doc values we send", true);
// AwaitsFix https://github.com/elastic/elasticsearch/issues/70244
// One of these two should work:
// return () -> Base64.getEncoder().encode(randomByteArrayOfLength(between(1, 30)));
return () -> DocValueFormat.BINARY.format(new BytesRef(randomByteArrayOfLength(between(1, 30))));
/*
* but they don't produce the same results. DocValueFormat.BINARY doesn't
* include trailing padding (looks like `=`) while Base64.getEncoder does.
* The parser needs the padding.
*/
}

@Override
protected void randomFetchTestFieldConfig(XContentBuilder b) throws IOException {
b.field("type", "binary").field("doc_values", true); // enable doc_values so the test is happy
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,10 @@
import org.elasticsearch.common.xcontent.XContentBuilder;
import org.elasticsearch.common.xcontent.XContentFactory;
import org.elasticsearch.index.mapper.ParseContext.Document;
import org.elasticsearch.test.ESTestCase;

import java.io.IOException;
import java.util.function.Supplier;

public class BooleanFieldMapperTests extends MapperTestCase {

Expand Down Expand Up @@ -162,4 +164,9 @@ public void testDocValues() throws Exception {
assertEquals(DocValuesType.NONE, fields[0].fieldType().docValuesType());
assertEquals(DocValuesType.SORTED_NUMERIC, fields[1].fieldType().docValuesType());
}

@Override
protected Supplier<? extends Object> randomFetchTestValueVendor(MappedFieldType ft) {
return ESTestCase::randomBoolean;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It'd be great to generate other valid boolean values (like the strings "true", "false", plus the empty string "").

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure!

}
}
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@
import java.io.IOException;
import java.util.Map;
import java.util.function.Function;
import java.util.function.Supplier;

import static org.elasticsearch.common.xcontent.XContentFactory.jsonBuilder;
import static org.hamcrest.Matchers.arrayContainingInAnyOrder;
Expand Down Expand Up @@ -778,4 +779,9 @@ protected V featureValueOf(T actual) {
};
}

@Override
protected Supplier<? extends Object> randomFetchTestValueVendor(MappedFieldType ft) {
assumeFalse("We don't have doc values or fielddata", true);
return null;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -25,6 +26,7 @@
import java.time.ZonedDateTime;
import java.util.List;
import java.util.Locale;
import java.util.function.Supplier;

import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.equalTo;
Expand Down Expand Up @@ -489,6 +491,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 Supplier<Comparable<?>> randomFetchTestValueVendor(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()));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
import org.hamcrest.CoreMatchers;

import java.io.IOException;
import java.util.function.Supplier;

import static org.elasticsearch.geometry.utils.Geohash.stringEncode;
import static org.hamcrest.Matchers.arrayWithSize;
Expand Down Expand Up @@ -331,4 +332,10 @@ protected void assertSearchable(MappedFieldType fieldType) {
//always searchable even if it uses TextSearchInfo.NONE
assertTrue(fieldType.isSearchable());
}

@Override
protected Supplier<? extends Object> randomFetchTestValueVendor(MappedFieldType ft) {
assumeFalse("Test implemented in a follow up", true);
return null;
}
}
Loading