Skip to content

Commit

Permalink
[test] remove Streamable serde assertions (#29307)
Browse files Browse the repository at this point in the history
Removes a set of assertions in the test framework that verified that
Streamable objects could be serialized and deserialized across different
versions. When this was discussed the consensus was that this approach
has not caught many bugs in a long time and that serialization testing of
objects was best left to their respective unit and integration tests.

This commit also removes a transport interceptor that was used in
ESIntegTestCase tests to make these assertions about objects coming in
or off the wire.
  • Loading branch information
andyb-elastic authored Mar 30, 2018
1 parent 199d131 commit b7e6fb9
Show file tree
Hide file tree
Showing 7 changed files with 11 additions and 364 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,6 @@
import org.elasticsearch.test.ESTestCase;
import org.elasticsearch.test.TestSearchContext;
import org.elasticsearch.test.VersionUtils;
import org.elasticsearch.test.hamcrest.ElasticsearchAssertions;
import org.elasticsearch.transport.ActionNotFoundTransportException;
import org.elasticsearch.transport.ActionTransportException;
import org.elasticsearch.transport.ConnectTransportException;
Expand Down Expand Up @@ -116,7 +115,6 @@
import static java.util.Collections.emptyMap;
import static java.util.Collections.emptySet;
import static java.util.Collections.singleton;
import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertVersionSerializable;
import static org.hamcrest.Matchers.greaterThanOrEqualTo;
import static org.hamcrest.Matchers.instanceOf;

Expand Down Expand Up @@ -233,7 +231,6 @@ private <T extends Exception> T serialize(T exception) throws IOException {
}

private <T extends Exception> T serialize(T exception, Version version) throws IOException {
ElasticsearchAssertions.assertVersionSerializable(version, exception);
BytesStreamOutput out = new BytesStreamOutput();
out.setVersion(version);
out.writeException(exception);
Expand Down Expand Up @@ -578,9 +575,6 @@ public void testWriteThrowable() throws IOException {
}
assertArrayEquals(deserialized.getStackTrace(), ex.getStackTrace());
assertTrue(deserialized.getStackTrace().length > 1);
assertVersionSerializable(VersionUtils.randomVersion(random()), cause);
assertVersionSerializable(VersionUtils.randomVersion(random()), ex);
assertVersionSerializable(VersionUtils.randomVersion(random()), deserialized);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,6 @@
import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertFirstHit;
import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertHitCount;
import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertNoFailures;
import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertSearchHit;
import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertSearchHits;
import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertSearchResponse;
import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertSecondHit;
Expand Down Expand Up @@ -191,7 +190,7 @@ public void testConstantScoreQuery() throws Exception {
SearchResponse searchResponse = client().prepareSearch().setQuery(constantScoreQuery(matchQuery("field1", "quick"))).get();
assertHitCount(searchResponse, 2L);
for (SearchHit searchHit : searchResponse.getHits().getHits()) {
assertSearchHit(searchHit, hasScore(1.0f));
assertThat(searchHit, hasScore(1.0f));
}

searchResponse = client().prepareSearch("test").setQuery(
Expand All @@ -210,7 +209,7 @@ public void testConstantScoreQuery() throws Exception {
assertHitCount(searchResponse, 2L);
assertFirstHit(searchResponse, hasScore(searchResponse.getHits().getAt(1).getScore()));
for (SearchHit searchHit : searchResponse.getHits().getHits()) {
assertSearchHit(searchHit, hasScore(1.0f));
assertThat(searchHit, hasScore(1.0f));
}

int num = scaledRandomIntBetween(100, 200);
Expand All @@ -228,7 +227,7 @@ public void testConstantScoreQuery() throws Exception {
long totalHits = searchResponse.getHits().getTotalHits();
SearchHits hits = searchResponse.getHits();
for (SearchHit searchHit : hits) {
assertSearchHit(searchHit, hasScore(1.0f));
assertThat(searchHit, hasScore(1.0f));
}
searchResponse = client().prepareSearch("test_1").setQuery(
boolQuery().must(matchAllQuery()).must(
Expand All @@ -238,7 +237,7 @@ public void testConstantScoreQuery() throws Exception {
if (totalHits > 1) {
float expected = hits.getAt(0).getScore();
for (SearchHit searchHit : hits) {
assertSearchHit(searchHit, hasScore(expected));
assertThat(searchHit, hasScore(expected));
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,6 @@
import static org.elasticsearch.common.xcontent.XContentFactory.jsonBuilder;
import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked;
import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAllSuccessful;
import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertSearchHit;
import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.hasId;
import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.hasScore;
import static org.hamcrest.Matchers.contains;
Expand Down Expand Up @@ -245,8 +244,8 @@ public void testSuggestDocument() throws Exception {
int id = numDocs;
for (CompletionSuggestion.Entry.Option option : options) {
assertThat(option.getText().toString(), equalTo("suggestion" + id));
assertSearchHit(option.getHit(), hasId("" + id));
assertSearchHit(option.getHit(), hasScore((id)));
assertThat(option.getHit(), hasId("" + id));
assertThat(option.getHit(), hasScore((id)));
assertNotNull(option.getHit().getSourceAsMap());
id--;
}
Expand Down Expand Up @@ -280,8 +279,8 @@ public void testSuggestDocumentNoSource() throws Exception {
int id = numDocs;
for (CompletionSuggestion.Entry.Option option : options) {
assertThat(option.getText().toString(), equalTo("suggestion" + id));
assertSearchHit(option.getHit(), hasId("" + id));
assertSearchHit(option.getHit(), hasScore((id)));
assertThat(option.getHit(), hasId("" + id));
assertThat(option.getHit(), hasScore((id)));
assertNull(option.getHit().getSourceAsMap());
id--;
}
Expand Down Expand Up @@ -317,8 +316,8 @@ public void testSuggestDocumentSourceFiltering() throws Exception {
int id = numDocs;
for (CompletionSuggestion.Entry.Option option : options) {
assertThat(option.getText().toString(), equalTo("suggestion" + id));
assertSearchHit(option.getHit(), hasId("" + id));
assertSearchHit(option.getHit(), hasScore((id)));
assertThat(option.getHit(), hasId("" + id));
assertThat(option.getHit(), hasScore((id)));
assertNotNull(option.getHit().getSourceAsMap());
Set<String> sourceFields = option.getHit().getSourceAsMap().keySet();
assertThat(sourceFields, contains("a"));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,6 @@
import org.elasticsearch.test.disruption.ServiceDisruptionScheme;
import org.elasticsearch.test.store.MockFSIndexStore;
import org.elasticsearch.test.transport.MockTransportService;
import org.elasticsearch.transport.AssertingTransportInterceptor;
import org.hamcrest.Matchers;
import org.junit.After;
import org.junit.AfterClass;
Expand Down Expand Up @@ -1921,9 +1920,6 @@ protected Collection<Class<? extends Plugin>> getMockPlugins() {
if (randomBoolean()) {
mocks.add(MockSearchService.TestPlugin.class);
}
if (randomBoolean()) {
mocks.add(AssertingTransportInterceptor.TestPlugin.class);
}
if (randomBoolean()) {
mocks.add(MockFieldFilterPlugin.class);
}
Expand Down
Loading

0 comments on commit b7e6fb9

Please sign in to comment.