honeymoose
/
OpenSearch
mirror of https://github.com/honeymoose/OpenSearch.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Clean up wire test case a bit (#50627) (#50632) 

* Adds JavaDoc to `AbstractWireTestCase` and
`AbstractWireSerializingTestCase` so it is more obvious you should prefer
the latter if you have a choice
* Moves the `instanceReader` method out of `AbstractWireTestCase` becaue
it is no longer used.
* Marks a bunch of methods final so it is more obvious which classes are
for what.
* Cleans up the side effects of the above.
This commit is contained in:
Nik Everett 2020-01-05 16:20:38 -05:00 committed by GitHub
parent 8819fa4ebe
commit 2362c430cd
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 44 additions and 28 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

7
libs/geo/src/test/java/org/elasticsearch/geometry/BaseGeometryTestCase.java
View File

@ -21,7 +21,6 @@ package org.elasticsearch.geometry;
import org.elasticsearch.ElasticsearchException;
import org.elasticsearch.Version;
import org.elasticsearch.common.io.stream.Writeable;
import org.elasticsearch.geometry.utils.GeographyValidator;
import org.elasticsearch.geometry.utils.WellKnownText;
import org.elasticsearch.test.AbstractWireTestCase;
@ -42,12 +41,6 @@ abstract class BaseGeometryTestCase<T extends Geometry> extends AbstractWireTest
protected abstract T createTestInstance(boolean hasAlt);
@Override
protected Writeable.Reader<T> instanceReader() {
throw new IllegalStateException("shouldn't be called in this test");
}
@SuppressWarnings("unchecked")
@Override
protected T copyInstance(T instance, Version version) throws IOException {

2
server/src/test/java/org/elasticsearch/action/admin/indices/rollover/RolloverResponseTests.java
View File

@ -19,8 +19,8 @@
package org.elasticsearch.action.admin.indices.rollover;
import org.elasticsearch.common.io.stream.Writeable;
import org.elasticsearch.Version;
import org.elasticsearch.common.io.stream.Writeable;
import org.elasticsearch.common.unit.ByteSizeValue;
import org.elasticsearch.common.unit.TimeValue;
import org.elasticsearch.common.xcontent.XContentParser;

14
test/framework/src/main/java/org/elasticsearch/test/AbstractWireSerializingTestCase.java
View File

@ -19,14 +19,26 @@
package org.elasticsearch.test;
import org.elasticsearch.Version;
import org.elasticsearch.common.io.stream.StreamInput;
import org.elasticsearch.common.io.stream.StreamOutput;
import org.elasticsearch.common.io.stream.Writeable;
import java.io.IOException;
/**
* Standard test case for testing the wire serialization of subclasses of {@linkplain Writeable}.
*/
public abstract class AbstractWireSerializingTestCase<T extends Writeable> extends AbstractWireTestCase<T> {
/**
* Returns a {@link Writeable.Reader} that can be used to de-serialize the instance
*/
protected abstract Writeable.Reader<T> instanceReader();
/**
* Copy the {@link Writeable} by round tripping it through {@linkplain StreamInput} and {@linkplain StreamOutput}.
*/
@Override
protected T copyInstance(T instance, Version version) throws IOException {
protected final T copyInstance(T instance, Version version) throws IOException {
return copyWriteable(instance, getNamedWriteableRegistry(), instanceReader(), version);
}
}

32
test/framework/src/main/java/org/elasticsearch/test/AbstractWireTestCase.java
View File

@ -27,6 +27,10 @@ import org.elasticsearch.common.io.stream.Writeable;
import java.io.IOException;
import java.util.Collections;
/**
* Standard test case for testing wire serialization. If the class being tested
* extends {@link Writeable} then prefer extending {@link AbstractWireSerializingTestCase}.
*/
public abstract class AbstractWireTestCase<T> extends ESTestCase {
protected static final int NUMBER_OF_TEST_RUNS = 20;
@ -38,11 +42,6 @@ public abstract class AbstractWireTestCase<T> extends ESTestCase {
*/
protected abstract T createTestInstance();
/**
* Returns a {@link Writeable.Reader} that can be used to de-serialize the instance
*/
protected abstract Writeable.Reader<T> instanceReader();
/**
* Returns an instance which is mutated slightly so it should not be equal
* to the given instance.
@ -73,18 +72,26 @@ public abstract class AbstractWireTestCase<T> extends ESTestCase {
}
/**
* Serialize the given instance and asserts that both are equal
* Serialize the given instance and asserts that both are equal.
*/
protected final T assertSerialization(T testInstance) throws IOException {
return assertSerialization(testInstance, Version.CURRENT);
protected final void assertSerialization(T testInstance) throws IOException {
assertSerialization(testInstance, Version.CURRENT);
}
protected final T assertSerialization(T testInstance, Version version) throws IOException {
/**
* Assert that instances copied at a particular version are equal. The version is useful
* for sanity checking the backwards compatibility of the wire. It isn't a substitute for
* real backwards compatibility tests but it is *so* much faster.
*/
protected final void assertSerialization(T testInstance, Version version) throws IOException {
T deserializedInstance = copyInstance(testInstance, version);
assertEqualInstances(testInstance, deserializedInstance);
return deserializedInstance;
}
/**
* Assert that two instances are equal. This is intentionally not final so we can override
* how equality is checked.
*/
protected void assertEqualInstances(T expectedInstance, T newInstance) {
assertNotSame(newInstance, expectedInstance);
assertEquals(expectedInstance, newInstance);
@ -95,6 +102,11 @@ public abstract class AbstractWireTestCase<T> extends ESTestCase {
return copyInstance(instance, Version.CURRENT);
}
/**
* Copy the instance as by reading and writing using the code specific to the provided version.
* The version is useful for sanity checking the backwards compatibility of the wire. It isn't
* a substitute for real backwards compatibility tests but it is *so* much faster.
*/
protected abstract T copyInstance(T instance, Version version) throws IOException;
/**

5
x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/AbstractSqlWireSerializingTestCase.java
View File

@ -33,6 +33,11 @@ public abstract class AbstractSqlWireSerializingTestCase<T extends Writeable> ex
}
}
/**
* Returns a {@link Writeable.Reader} that can be used to de-serialize the instance
*/
protected abstract Writeable.Reader<T> instanceReader();
protected ZoneId instanceZoneId(T instance) {
return randomSafeZone();
}

12
x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/session/ListCursorTests.java
View File

@ -7,15 +7,14 @@ package org.elasticsearch.xpack.sql.session;
import org.elasticsearch.Version;
import org.elasticsearch.common.io.stream.NamedWriteableRegistry;
import org.elasticsearch.common.io.stream.Writeable.Reader;
import org.elasticsearch.test.AbstractWireSerializingTestCase;
import org.elasticsearch.test.AbstractWireTestCase;
import java.io.IOException;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;
public class ListCursorTests extends AbstractWireSerializingTestCase<ListCursor> {
public class ListCursorTests extends AbstractWireTestCase<ListCursor> {
public static ListCursor randomPagingListCursor() {
int size = between(1, 20);
int depth = between(1, 20);
@ -45,17 +44,12 @@ public class ListCursorTests extends AbstractWireSerializingTestCase<ListCursor>
return randomPagingListCursor();
}
@Override
protected Reader<ListCursor> instanceReader() {
return ListCursor::new;
}
@Override
protected ListCursor copyInstance(ListCursor instance, Version version) throws IOException {
/* Randomly choose between internal protocol round trip and String based
* round trips used to toXContent. */
if (randomBoolean()) {
return super.copyInstance(instance, version);
return copyWriteable(instance, getNamedWriteableRegistry(), ListCursor::new, version);
}
return (ListCursor) Cursors.decodeFromString(Cursors.encodeToString(instance, randomZone()));
}