Skip to content

Commit bb3ae18

Browse files
javannajavanna
authored
Increase coverage in SearchSortValuesTests (#36597)
SearchSortValuesTests extends now `AbstractSerializingTestCase` which removes some code duplication and standardizes the way we test `fromXContent`, serialization and equals/hashcode. Also, we were never creating `SearchSortValues` through their public constructor that accept an array of `DocValueFormat` together with the array of raw sort values. That is covered now, which involved some conversion from `BytesRef` to String in the test. Also, the previous test was not using doing any equality check against the original and parsed versions in `testFromXContent` due to values being parsed with different types in some cases, which is now covered by converting those values using a new method added to `RandomObjects`. The code was already there as part of `randomStoredFieldValues`, but it is now exposed to be used in other scenarios.
1 parent 8f04536 commit bb3ae18

File tree

5 files changed

+152
-132
lines changed

5 files changed

+152
-132
lines changed

‎server/src/main/java/org/elasticsearch/search/SearchSortValues.java‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,6 @@
2323
import org.elasticsearch.common.io.stream.StreamInput;
2424
import org.elasticsearch.common.io.stream.StreamOutput;
2525
import org.elasticsearch.common.io.stream.Writeable;
26-
import org.elasticsearch.common.xcontent.ToXContent.Params;
2726
import org.elasticsearch.common.xcontent.ToXContentFragment;
2827
import org.elasticsearch.common.xcontent.XContentBuilder;
2928
import org.elasticsearch.common.xcontent.XContentParser;

‎server/src/test/java/org/elasticsearch/search/SearchHitTests.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,7 @@ public static SearchHit createTestItem(XContentType xContentType, boolean withOp
9292
hit.version(randomLong());
9393
}
9494
if (randomBoolean()) {
95-
hit.sortValues(SearchSortValuesTests.createTestItem());
95+
hit.sortValues(SearchSortValuesTests.createTestItem(xContentType, transportSerialization));
9696
}
9797
if (randomBoolean()) {
9898
int size = randomIntBetween(0, 5);

‎server/src/test/java/org/elasticsearch/search/SearchSortValuesTests.java‎

Lines changed: 78 additions & 72 deletions
Original file line numberDiff line numberDiff line change
@@ -19,108 +19,114 @@
1919

2020
package org.elasticsearch.search;
2121

22+
import org.apache.lucene.util.BytesRef;
23+
import org.elasticsearch.Version;
2224
import org.elasticsearch.common.Strings;
23-
import org.elasticsearch.common.bytes.BytesReference;
24-
import org.elasticsearch.common.io.stream.BytesStreamOutput;
25-
import org.elasticsearch.common.io.stream.StreamInput;
25+
import org.elasticsearch.common.io.stream.Writeable;
26+
import org.elasticsearch.common.lucene.LuceneTests;
2627
import org.elasticsearch.common.xcontent.ToXContent;
2728
import org.elasticsearch.common.xcontent.XContentBuilder;
2829
import org.elasticsearch.common.xcontent.XContentParser;
2930
import org.elasticsearch.common.xcontent.XContentType;
3031
import org.elasticsearch.common.xcontent.json.JsonXContent;
31-
import org.elasticsearch.test.ESTestCase;
32+
import org.elasticsearch.test.AbstractSerializingTestCase;
33+
import org.elasticsearch.test.RandomObjects;
3234

3335
import java.io.IOException;
34-
import java.util.ArrayList;
3536
import java.util.Arrays;
36-
import java.util.List;
37-
import java.util.function.Supplier;
3837

39-
import static org.elasticsearch.common.xcontent.XContentHelper.toXContent;
40-
import static org.elasticsearch.test.EqualsHashCodeTestUtils.checkEqualsAndHashCode;
41-
import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertToXContentEquivalent;
42-
43-
public class SearchSortValuesTests extends ESTestCase {
44-
45-
public static SearchSortValues createTestItem() {
46-
List<Supplier<Object>> valueSuppliers = new ArrayList<>();
47-
// this should reflect all values that are allowed to go through the transport layer
48-
valueSuppliers.add(() -> null);
49-
valueSuppliers.add(ESTestCase::randomInt);
50-
valueSuppliers.add(ESTestCase::randomLong);
51-
valueSuppliers.add(ESTestCase::randomDouble);
52-
valueSuppliers.add(ESTestCase::randomFloat);
53-
valueSuppliers.add(ESTestCase::randomByte);
54-
valueSuppliers.add(ESTestCase::randomShort);
55-
valueSuppliers.add(ESTestCase::randomBoolean);
56-
valueSuppliers.add(() -> frequently() ? randomAlphaOfLengthBetween(1, 30) : randomRealisticUnicodeOfCodepointLength(30));
38+
public class SearchSortValuesTests extends AbstractSerializingTestCase<SearchSortValues> {
5739

40+
public static SearchSortValues createTestItem(XContentType xContentType, boolean transportSerialization) {
5841
int size = randomIntBetween(1, 20);
5942
Object[] values = new Object[size];
43+
DocValueFormat[] sortValueFormats = new DocValueFormat[size];
6044
for (int i = 0; i < size; i++) {
61-
Supplier<Object> supplier = randomFrom(valueSuppliers);
62-
values[i] = supplier.get();
45+
Object sortValue = randomSortValue(xContentType, transportSerialization);
46+
values[i] = sortValue;
47+
//make sure that for BytesRef, we provide a specific doc value format that overrides format(BytesRef)
48+
sortValueFormats[i] = sortValue instanceof BytesRef ? DocValueFormat.RAW : randomDocValueFormat();
6349
}
64-
return new SearchSortValues(values);
50+
return new SearchSortValues(values, sortValueFormats);
51+
}
52+
53+
private static Object randomSortValue(XContentType xContentType, boolean transportSerialization) {
54+
Object randomSortValue = LuceneTests.randomSortValue();
55+
//to simplify things, we directly serialize what we expect we would parse back when testing xcontent serialization
56+
return transportSerialization ? randomSortValue : RandomObjects.getExpectedParsedValue(xContentType, randomSortValue);
6557
}
6658

67-
public void testFromXContent() throws IOException {
68-
SearchSortValues sortValues = createTestItem();
69-
XContentType xcontentType = randomFrom(XContentType.values());
70-
boolean humanReadable = randomBoolean();
71-
BytesReference originalBytes = toShuffledXContent(sortValues, xcontentType, ToXContent.EMPTY_PARAMS, humanReadable);
59+
private static DocValueFormat randomDocValueFormat() {
60+
return randomFrom(DocValueFormat.BOOLEAN, DocValueFormat.RAW, DocValueFormat.IP, DocValueFormat.BINARY, DocValueFormat.GEOHASH);
61+
}
7262

73-
SearchSortValues parsed;
74-
try (XContentParser parser = createParser(xcontentType.xContent(), originalBytes)) {
75-
parser.nextToken(); // skip to the elements start array token, fromXContent advances from there if called
76-
parser.nextToken();
77-
parser.nextToken();
78-
parsed = SearchSortValues.fromXContent(parser);
79-
parser.nextToken();
80-
assertEquals(XContentParser.Token.END_OBJECT, parser.currentToken());
81-
assertNull(parser.nextToken());
82-
}
83-
assertToXContentEquivalent(originalBytes, toXContent(parsed, xcontentType, humanReadable), xcontentType);
63+
@Override
64+
protected SearchSortValues doParseInstance(XContentParser parser) throws IOException {
65+
parser.nextToken(); // skip to the elements start array token, fromXContent advances from there if called
66+
parser.nextToken();
67+
parser.nextToken();
68+
SearchSortValues searchSortValues = SearchSortValues.fromXContent(parser);
69+
parser.nextToken();
70+
assertEquals(XContentParser.Token.END_OBJECT, parser.currentToken());
71+
assertNull(parser.nextToken());
72+
return searchSortValues;
8473
}
8574

86-
public void testToXContent() throws IOException {
87-
SearchSortValues sortValues = new SearchSortValues(new Object[]{ 1, "foo", 3.0});
88-
XContentBuilder builder = JsonXContent.contentBuilder();
89-
builder.startObject();
90-
sortValues.toXContent(builder, ToXContent.EMPTY_PARAMS);
91-
builder.endObject();
92-
assertEquals("{\"sort\":[1,\"foo\",3.0]}", Strings.toString(builder));
75+
@Override
76+
protected SearchSortValues createXContextTestInstance(XContentType xContentType) {
77+
return createTestItem(xContentType, false);
9378
}
9479

95-
/**
96-
* Test equality and hashCode properties
97-
*/
98-
public void testEqualsAndHashcode() {
99-
checkEqualsAndHashCode(createTestItem(), SearchSortValuesTests::copy, SearchSortValuesTests::mutate);
80+
@Override
81+
protected SearchSortValues createTestInstance() {
82+
return createTestItem(randomFrom(XContentType.values()), true);
10083
}
10184

102-
public void testSerialization() throws IOException {
103-
SearchSortValues sortValues = createTestItem();
104-
try (BytesStreamOutput output = new BytesStreamOutput()) {
105-
sortValues.writeTo(output);
106-
try (StreamInput in = output.bytes().streamInput()) {
107-
SearchSortValues deserializedCopy = new SearchSortValues(in);
108-
assertEquals(sortValues, deserializedCopy);
109-
assertEquals(sortValues.hashCode(), deserializedCopy.hashCode());
110-
assertNotSame(sortValues, deserializedCopy);
111-
}
85+
@Override
86+
protected Writeable.Reader<SearchSortValues> instanceReader() {
87+
return SearchSortValues::new;
88+
}
89+
90+
@Override
91+
protected String[] getShuffleFieldsExceptions() {
92+
return new String[]{"sort"};
93+
}
94+
95+
public void testToXContent() throws IOException {
96+
{
97+
SearchSortValues sortValues = new SearchSortValues(new Object[]{ 1, "foo", 3.0});
98+
XContentBuilder builder = JsonXContent.contentBuilder();
99+
builder.startObject();
100+
sortValues.toXContent(builder, ToXContent.EMPTY_PARAMS);
101+
builder.endObject();
102+
assertEquals("{\"sort\":[1,\"foo\",3.0]}", Strings.toString(builder));
103+
}
104+
{
105+
SearchSortValues sortValues = new SearchSortValues(new Object[0]);
106+
XContentBuilder builder = JsonXContent.contentBuilder();
107+
builder.startObject();
108+
sortValues.toXContent(builder, ToXContent.EMPTY_PARAMS);
109+
builder.endObject();
110+
assertEquals("{}", Strings.toString(builder));
112111
}
113112
}
114113

115-
private static SearchSortValues mutate(SearchSortValues original) {
116-
Object[] sortValues = original.sortValues();
114+
@Override
115+
protected SearchSortValues mutateInstance(SearchSortValues instance) {
116+
Object[] sortValues = instance.sortValues();
117117
if (sortValues.length == 0) {
118-
return new SearchSortValues(new Object[] { 1 });
118+
return createTestInstance();
119119
}
120-
return new SearchSortValues(Arrays.copyOf(sortValues, sortValues.length + 1));
120+
if (randomBoolean()) {
121+
return new SearchSortValues(new Object[0]);
122+
}
123+
Object[] values = Arrays.copyOf(sortValues, sortValues.length + 1);
124+
values[sortValues.length] = randomSortValue(randomFrom(XContentType.values()), true);
125+
return new SearchSortValues(values);
121126
}
122127

123-
private static SearchSortValues copy(SearchSortValues original) {
124-
return new SearchSortValues(Arrays.copyOf(original.sortValues(), original.sortValues().length));
128+
@Override
129+
protected SearchSortValues copyInstance(SearchSortValues instance, Version version) {
130+
return new SearchSortValues(Arrays.copyOf(instance.sortValues(), instance.sortValues().length));
125131
}
126132
}

‎test/framework/src/main/java/org/elasticsearch/test/AbstractSerializingTestCase.java‎

Lines changed: 20 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -24,35 +24,44 @@
2424
import org.elasticsearch.common.xcontent.ToXContent;
2525
import org.elasticsearch.common.xcontent.XContentBuilder;
2626
import org.elasticsearch.common.xcontent.XContentParser;
27+
import org.elasticsearch.common.xcontent.XContentType;
2728

2829
import java.io.IOException;
2930
import java.util.function.Predicate;
3031

32+
import static org.elasticsearch.test.AbstractXContentTestCase.xContentTester;
33+
3134
public abstract class AbstractSerializingTestCase<T extends ToXContent & Writeable> extends AbstractWireSerializingTestCase<T> {
3235

3336
/**
3437
* Generic test that creates new instance from the test instance and checks
3538
* both for equality and asserts equality on the two instances.
3639
*/
3740
public final void testFromXContent() throws IOException {
38-
AbstractXContentTestCase.testFromXContent(
39-
NUMBER_OF_TEST_RUNS,
40-
this::createTestInstance,
41-
supportsUnknownFields(),
42-
getShuffleFieldsExceptions(),
43-
getRandomFieldsExcludeFilter(),
44-
this::createParser,
45-
this::doParseInstance,
46-
this::assertEqualInstances,
47-
assertToXContentEquivalence(),
48-
getToXContentParams());
41+
xContentTester(this::createParser, this::createXContextTestInstance, getToXContentParams(), this::doParseInstance)
42+
.numberOfTestRuns(NUMBER_OF_TEST_RUNS)
43+
.supportsUnknownFields(supportsUnknownFields())
44+
.shuffleFieldsExceptions(getShuffleFieldsExceptions())
45+
.randomFieldsExcludeFilter(getRandomFieldsExcludeFilter())
46+
.assertEqualsConsumer(this::assertEqualInstances)
47+
.assertToXContentEquivalence(assertToXContentEquivalence())
48+
.test();
4949
}
5050

5151
/**
5252
* Parses to a new instance using the provided {@link XContentParser}
5353
*/
5454
protected abstract T doParseInstance(XContentParser parser) throws IOException;
5555

56+
/**
57+
* Creates a random instance to use in the xcontent tests.
58+
* Override this method if the random instance that you build
59+
* should be aware of the {@link XContentType} used in the test.
60+
*/
61+
protected T createXContextTestInstance(XContentType xContentType) {
62+
return createTestInstance();
63+
}
64+
5665
/**
5766
* Indicates whether the parser supports unknown fields or not. In case it does, such behaviour will be tested by
5867
* inserting random fields before parsing and checking that they don't make parsing fail.

‎test/framework/src/main/java/org/elasticsearch/test/RandomObjects.java‎

Lines changed: 53 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,6 @@
2121

2222
import com.carrotsearch.randomizedtesting.generators.RandomPicks;
2323
import com.carrotsearch.randomizedtesting.generators.RandomStrings;
24-
2524
import org.elasticsearch.ElasticsearchException;
2625
import org.elasticsearch.action.support.replication.ReplicationResponse.ShardInfo;
2726
import org.elasticsearch.action.support.replication.ReplicationResponse.ShardInfo.Failure;
@@ -72,78 +71,85 @@ private RandomObjects() {
7271
*/
7372
public static Tuple<List<Object>, List<Object>> randomStoredFieldValues(Random random, XContentType xContentType) {
7473
int numValues = randomIntBetween(random, 1, 5);
75-
List<Object> originalValues = new ArrayList<>();
76-
List<Object> expectedParsedValues = new ArrayList<>();
74+
List<Object> originalValues = randomStoredFieldValues(random, numValues);
75+
List<Object> expectedParsedValues = new ArrayList<>(numValues);
76+
for (Object originalValue : originalValues) {
77+
expectedParsedValues.add(getExpectedParsedValue(xContentType, originalValue));
78+
}
79+
return Tuple.tuple(originalValues, expectedParsedValues);
80+
}
81+
82+
private static List<Object> randomStoredFieldValues(Random random, int numValues) {
83+
List<Object> values = new ArrayList<>(numValues);
7784
int dataType = randomIntBetween(random, 0, 8);
7885
for (int i = 0; i < numValues; i++) {
7986
switch(dataType) {
8087
case 0:
81-
long randomLong = random.nextLong();
82-
originalValues.add(randomLong);
83-
expectedParsedValues.add(randomLong);
88+
values.add(random.nextLong());
8489
break;
8590
case 1:
86-
int randomInt = random.nextInt();
87-
originalValues.add(randomInt);
88-
expectedParsedValues.add(randomInt);
91+
values.add(random.nextInt());
8992
break;
9093
case 2:
91-
Short randomShort = (short) random.nextInt();
92-
originalValues.add(randomShort);
93-
expectedParsedValues.add(randomShort.intValue());
94+
values.add((short) random.nextInt());
9495
break;
9596
case 3:
96-
Byte randomByte = (byte)random.nextInt();
97-
originalValues.add(randomByte);
98-
expectedParsedValues.add(randomByte.intValue());
97+
values.add((byte) random.nextInt());
9998
break;
10099
case 4:
101-
double randomDouble = random.nextDouble();
102-
originalValues.add(randomDouble);
103-
expectedParsedValues.add(randomDouble);
100+
values.add(random.nextDouble());
104101
break;
105102
case 5:
106-
Float randomFloat = random.nextFloat();
107-
originalValues.add(randomFloat);
108-
if (xContentType == XContentType.CBOR) {
109-
//with CBOR we get back a float
110-
expectedParsedValues.add(randomFloat);
111-
} else if (xContentType == XContentType.SMILE) {
112-
//with SMILE we get back a double (this will change in Jackson 2.9 where it will return a Float)
113-
expectedParsedValues.add(randomFloat.doubleValue());
114-
} else {
115-
//with JSON AND YAML we get back a double, but with float precision.
116-
expectedParsedValues.add(Double.parseDouble(randomFloat.toString()));
117-
}
103+
values.add(random.nextFloat());
118104
break;
119105
case 6:
120-
boolean randomBoolean = random.nextBoolean();
121-
originalValues.add(randomBoolean);
122-
expectedParsedValues.add(randomBoolean);
106+
values.add(random.nextBoolean());
123107
break;
124108
case 7:
125-
String randomString = random.nextBoolean() ? RandomStrings.randomAsciiLettersOfLengthBetween(random, 3, 10) :
126-
randomUnicodeOfLengthBetween(random, 3, 10);
127-
originalValues.add(randomString);
128-
expectedParsedValues.add(randomString);
109+
values.add(random.nextBoolean() ? RandomStrings.randomAsciiLettersOfLengthBetween(random, 3, 10) :
110+
randomUnicodeOfLengthBetween(random, 3, 10));
129111
break;
130112
case 8:
131113
byte[] randomBytes = RandomStrings.randomUnicodeOfLengthBetween(random, 10, 50).getBytes(StandardCharsets.UTF_8);
132-
BytesArray randomBytesArray = new BytesArray(randomBytes);
133-
originalValues.add(randomBytesArray);
134-
if (xContentType == XContentType.JSON || xContentType == XContentType.YAML) {
135-
//JSON and YAML write the base64 format
136-
expectedParsedValues.add(Base64.getEncoder().encodeToString(randomBytes));
137-
} else {
138-
//SMILE and CBOR write the original bytes as they support binary format
139-
expectedParsedValues.add(randomBytesArray);
140-
}
114+
values.add(new BytesArray(randomBytes));
141115
break;
142116
default:
143117
throw new UnsupportedOperationException();
144118
}
145119
}
146-
return Tuple.tuple(originalValues, expectedParsedValues);
120+
return values;
121+
}
122+
123+
/**
124+
* Converts the provided field value to its corresponding expected value once printed out
125+
* via {@link org.elasticsearch.common.xcontent.ToXContent#toXContent(XContentBuilder, ToXContent.Params)} and parsed back via
126+
* {@link org.elasticsearch.common.xcontent.XContentParser#objectText()}.
127+
* Generates values based on what can get printed out. Stored fields values are retrieved from lucene and converted via
128+
* {@link org.elasticsearch.index.mapper.MappedFieldType#valueForDisplay(Object)} to either strings, numbers or booleans.
129+
*/
130+
public static Object getExpectedParsedValue(XContentType xContentType, Object value) {
131+
if (value instanceof BytesArray) {
132+
if (xContentType == XContentType.JSON || xContentType == XContentType.YAML) {
133+
//JSON and YAML write the base64 format
134+
return Base64.getEncoder().encodeToString(((BytesArray)value).toBytesRef().bytes);
135+
}
136+
}
137+
if (value instanceof Float) {
138+
if (xContentType == XContentType.SMILE) {
139+
//with SMILE we get back a double (this will change in Jackson 2.9 where it will return a Float)
140+
return ((Float)value).doubleValue();
141+
} else {
142+
//with JSON AND YAML we get back a double, but with float precision.
143+
return Double.parseDouble(value.toString());
144+
}
145+
}
146+
if (value instanceof Byte) {
147+
return ((Byte)value).intValue();
148+
}
149+
if (value instanceof Short) {
150+
return ((Short)value).intValue();
151+
}
152+
return value;
147153
}
148154

149155
/**

0 commit comments

Comments
 (0)