[TEST] Remove duplicated main response unit test (#25855)

Also move MainResponseTets to extend AbstractStreamableXContentTestCase

core/src/main/java/org/elasticsearch/action/main/MainResponse.java

@@ -41,7 +41,7 @@ public class MainResponse extends ActionResponse implements ToXContentObject {
     private ClusterName clusterName;
     private String clusterUuid;
     private Build build;
-    private boolean available;
+    boolean available;
 
     MainResponse() {
     }
@@ -122,7 +122,7 @@ public class MainResponse extends ActionResponse implements ToXContentObject {
     }
 
     private static final ObjectParser<MainResponse, Void> PARSER = new ObjectParser<>(MainResponse.class.getName(), true,
-            () -> new MainResponse());
+            MainResponse::new);
 
     static {
         PARSER.declareString((response, value) -> response.nodeName = value, new ParseField("name"));

core/src/test/java/org/elasticsearch/action/main/MainActionTests.java

@@ -19,8 +19,6 @@
 
 package org.elasticsearch.action.main;
 
-import org.elasticsearch.Build;
-import org.elasticsearch.Version;
 import org.elasticsearch.action.ActionListener;
 import org.elasticsearch.action.support.ActionFilters;
 import org.elasticsearch.cluster.ClusterName;
@@ -30,20 +28,14 @@ import org.elasticsearch.cluster.block.ClusterBlockLevel;
 import org.elasticsearch.cluster.block.ClusterBlocks;
 import org.elasticsearch.cluster.metadata.IndexNameExpressionResolver;
 import org.elasticsearch.cluster.service.ClusterService;
-import org.elasticsearch.common.io.stream.BytesStreamOutput;
 import org.elasticsearch.common.settings.Settings;
-import org.elasticsearch.common.xcontent.ToXContent;
-import org.elasticsearch.common.xcontent.XContentBuilder;
-import org.elasticsearch.common.xcontent.XContentFactory;
 import org.elasticsearch.rest.RestStatus;
 import org.elasticsearch.test.ESTestCase;
 import org.elasticsearch.threadpool.ThreadPool;
 import org.elasticsearch.transport.TransportService;
 
-import java.io.IOException;
 import java.util.concurrent.atomic.AtomicReference;
 
-import static org.hamcrest.Matchers.equalTo;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.times;
 import static org.mockito.Mockito.verify;
@@ -51,53 +43,6 @@ import static org.mockito.Mockito.when;
 
 public class MainActionTests extends ESTestCase {
 
-    public void testMainResponseSerialization() throws IOException {
-        final String nodeName = "node1";
-        final ClusterName clusterName = new ClusterName("cluster1");
-        final String clusterUUID = randomAlphaOfLengthBetween(10, 20);
-        final boolean available = randomBoolean();
-        final Version version = Version.CURRENT;
-        final Build build = Build.CURRENT;
-
-        final MainResponse mainResponse = new MainResponse(nodeName, version, clusterName, clusterUUID, build, available);
-        BytesStreamOutput streamOutput = new BytesStreamOutput();
-        mainResponse.writeTo(streamOutput);
-        final MainResponse serialized = new MainResponse();
-        serialized.readFrom(streamOutput.bytes().streamInput());
-
-        assertThat(serialized.getNodeName(), equalTo(nodeName));
-        assertThat(serialized.getClusterName(), equalTo(clusterName));
-        assertThat(serialized.getBuild(), equalTo(build));
-        assertThat(serialized.isAvailable(), equalTo(available));
-        assertThat(serialized.getVersion(), equalTo(version));
-    }
-
-    public void testMainResponseXContent() throws IOException {
-        String clusterUUID = randomAlphaOfLengthBetween(10, 20);
-        final MainResponse mainResponse = new MainResponse("node1", Version.CURRENT, new ClusterName("cluster1"), clusterUUID,
-                Build.CURRENT, false);
-        final String expected = "{" +
-                "\"name\":\"node1\"," +
-                "\"cluster_name\":\"cluster1\"," +
-                "\"cluster_uuid\":\"" + clusterUUID + "\"," +
-                "\"version\":{" +
-                "\"number\":\"" + Version.CURRENT.toString() + "\"," +
-                "\"build_hash\":\"" + Build.CURRENT.shortHash() + "\"," +
-                "\"build_date\":\"" + Build.CURRENT.date() + "\"," +
-                "\"build_snapshot\":" + Build.CURRENT.isSnapshot() + "," +
-                "\"lucene_version\":\"" + Version.CURRENT.luceneVersion.toString() + "\"," +
-                "\"minimum_wire_compatibility_version\":\"" + Version.CURRENT.minimumCompatibilityVersion().toString() + "\"," +
-                "\"minimum_index_compatibility_version\":\"" + Version.CURRENT.minimumIndexCompatibilityVersion().toString() +
-                "\"}," +
-                "\"tagline\":\"You Know, for Search\"}";
-
-        XContentBuilder builder = XContentFactory.jsonBuilder();
-        mainResponse.toXContent(builder, ToXContent.EMPTY_PARAMS);
-        String xContent = builder.string();
-
-        assertEquals(expected, xContent);
-    }
-
     public void testMainActionClusterAvailable() {
         final ClusterService clusterService = mock(ClusterService.class);
         final ClusterName clusterName = new ClusterName("elasticsearch");

core/src/test/java/org/elasticsearch/action/main/MainResponseTests.java

@@ -22,26 +22,29 @@ package org.elasticsearch.action.main;
 import org.elasticsearch.Build;
 import org.elasticsearch.Version;
 import org.elasticsearch.cluster.ClusterName;
-import org.elasticsearch.common.bytes.BytesReference;
 import org.elasticsearch.common.xcontent.ToXContent;
 import org.elasticsearch.common.xcontent.XContentBuilder;
 import org.elasticsearch.common.xcontent.XContentFactory;
 import org.elasticsearch.common.xcontent.XContentParser;
-import org.elasticsearch.common.xcontent.XContentType;
-import org.elasticsearch.test.ESTestCase;
+import org.elasticsearch.test.AbstractStreamableXContentTestCase;
 import org.elasticsearch.test.VersionUtils;
 
 import java.io.IOException;
 import java.util.Date;
 
-import static org.elasticsearch.common.xcontent.XContentHelper.toXContent;
 import static org.elasticsearch.test.EqualsHashCodeTestUtils.checkEqualsAndHashCode;
-import static org.elasticsearch.test.XContentTestUtils.insertRandomFields;
-import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertToXContentEquivalent;
 
-public class MainResponseTests extends ESTestCase {
+public class MainResponseTests extends AbstractStreamableXContentTestCase<MainResponse> {
 
-    public static MainResponse createTestItem() {
+    @Override
+    protected MainResponse getExpectedFromXContent(MainResponse testInstance) {
+        // we cannot recreate the "available" flag from xContent, but should be "true" if request came through
+        testInstance.available = true;
+        return testInstance;
+    }
+
+    @Override
+    protected MainResponse createTestInstance() {
         String clusterUuid = randomAlphaOfLength(10);
         ClusterName clusterName = new ClusterName(randomAlphaOfLength(10));
         String nodeName = randomAlphaOfLength(10);
@@ -51,43 +54,32 @@ public class MainResponseTests extends ESTestCase {
         return new MainResponse(nodeName, version, clusterName, clusterUuid , build, available);
     }
 
-    public void testFromXContent() throws IOException {
-        MainResponse mainResponse = createTestItem();
-        XContentType xContentType = randomFrom(XContentType.values());
-        boolean humanReadable = randomBoolean();
-        BytesReference originalBytes = toShuffledXContent(mainResponse, xContentType, ToXContent.EMPTY_PARAMS, humanReadable);
-        // we add a few random fields to check that parser is lenient on new fields
-        BytesReference withRandomFields = insertRandomFields(xContentType, originalBytes, null, random());
-        MainResponse parsed;
-        try (XContentParser parser = createParser(xContentType.xContent(), withRandomFields)) {
-            parsed = MainResponse.fromXContent(parser);
-            assertNull(parser.nextToken());
-        }
-        assertEquals(mainResponse.getClusterUuid(), parsed.getClusterUuid());
-        assertEquals(mainResponse.getClusterName(), parsed.getClusterName());
-        assertEquals(mainResponse.getNodeName(), parsed.getNodeName());
-        assertEquals(mainResponse.getBuild(), parsed.getBuild());
-        assertEquals(mainResponse.getVersion(), parsed.getVersion());
-        // we cannot recreate the "available" flag from xContent, but should be "true" if request came through
-        assertEquals(true, parsed.isAvailable());
-        assertToXContentEquivalent(originalBytes, toXContent(parsed, xContentType, humanReadable), xContentType);
+    @Override
+    protected MainResponse createBlankInstance() {
+        return new MainResponse();
+    }
+
+    @Override
+    protected MainResponse doParseInstance(XContentParser parser) {
+        return MainResponse.fromXContent(parser);
     }
 
     public void testToXContent() throws IOException {
-        Build build = new Build("buildHash", "2016-11-15".toString(), true);
+        String clusterUUID = randomAlphaOfLengthBetween(10, 20);
+        Build build = new Build(Build.CURRENT.shortHash(), Build.CURRENT.date(), Build.CURRENT.isSnapshot());
         Version version = Version.CURRENT;
-        MainResponse response = new MainResponse("nodeName", version, new ClusterName("clusterName"), "clusterUuid", build, true);
+        MainResponse response = new MainResponse("nodeName", version, new ClusterName("clusterName"), clusterUUID, build, true);
         XContentBuilder builder = XContentFactory.jsonBuilder();
         response.toXContent(builder, ToXContent.EMPTY_PARAMS);
         assertEquals("{"
                 + "\"name\":\"nodeName\","
                 + "\"cluster_name\":\"clusterName\","
-                + "\"cluster_uuid\":\"clusterUuid\","
+                + "\"cluster_uuid\":\"" + clusterUUID + "\","
                 + "\"version\":{"
                     + "\"number\":\"" + version.toString() + "\","
-                    + "\"build_hash\":\"buildHash\","
-                    + "\"build_date\":\"2016-11-15\","
-                    + "\"build_snapshot\":true,"
+                    + "\"build_hash\":\"" + Build.CURRENT.shortHash() + "\","
+                    + "\"build_date\":\"" + Build.CURRENT.date() + "\","
+                    + "\"build_snapshot\":" + Build.CURRENT.isSnapshot() + ","
                     + "\"lucene_version\":\"" + version.luceneVersion.toString() + "\","
                     + "\"minimum_wire_compatibility_version\":\"" + version.minimumCompatibilityVersion().toString() + "\","
                     + "\"minimum_index_compatibility_version\":\"" + version.minimumIndexCompatibilityVersion().toString() + "\"},"
@@ -95,8 +87,10 @@ public class MainResponseTests extends ESTestCase {
           + "}", builder.string());
     }
 
+    //TODO this should be removed and the metehod from AbstractStreamableTestCase should be
+    //used instead once https://github.com/elastic/elasticsearch/pull/25910 goes in
     public void testEqualsAndHashcode() {
-        MainResponse original = createTestItem();
+        MainResponse original = createTestInstance();
         checkEqualsAndHashCode(original, MainResponseTests::copy, MainResponseTests::mutate);
     }
 
@@ -134,5 +128,4 @@ public class MainResponseTests extends ESTestCase {
         }
         return new MainResponse(nodeName, version, clusterName, clusterUuid, build, available);
     }
-
 }

core/src/test/java/org/elasticsearch/search/collapse/CollapseBuilderTests.java

@@ -26,14 +26,11 @@ import org.apache.lucene.index.IndexWriter;
 import org.apache.lucene.search.Query;
 import org.apache.lucene.store.Directory;
 import org.apache.lucene.store.RAMDirectory;
-import org.elasticsearch.common.bytes.BytesArray;
 import org.elasticsearch.common.io.stream.NamedWriteableRegistry;
 import org.elasticsearch.common.io.stream.Writeable;
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.common.xcontent.NamedXContentRegistry;
-import org.elasticsearch.common.xcontent.XContentBuilder;
 import org.elasticsearch.common.xcontent.XContentParser;
-import org.elasticsearch.common.xcontent.XContentType;
 import org.elasticsearch.index.mapper.KeywordFieldMapper;
 import org.elasticsearch.index.mapper.MappedFieldType;
 import org.elasticsearch.index.mapper.NumberFieldMapper;
@@ -50,7 +47,6 @@ import org.junit.BeforeClass;
 import java.io.IOException;
 import java.util.ArrayList;
 import java.util.List;
-import java.util.Map;
 
 import static java.util.Collections.emptyList;
 import static org.mockito.Mockito.mock;
@@ -203,21 +199,9 @@ public class CollapseBuilderTests extends AbstractSerializingTestCase<CollapseBu
         return CollapseBuilder.fromXContent(parser);
     }
 
-    /**
-     * Rewrite this test to disable xcontent shuffling on the highlight builder
-     */
     @Override
-    public void testFromXContent() throws IOException {
-        for (int runs = 0; runs < NUMBER_OF_TEST_RUNS; runs++) {
-            CollapseBuilder testInstance = createTestInstance();
-            XContentType xContentType = randomFrom(XContentType.values());
-            XContentBuilder builder = toXContent(testInstance, xContentType);
-            XContentBuilder shuffled = shuffleXContent(builder, "fields");
-            assertParsedInstance(xContentType, shuffled.bytes(), testInstance);
-            for (Map.Entry<String, CollapseBuilder> alternateVersion : getAlternateVersions().entrySet()) {
-                String instanceAsString = alternateVersion.getKey();
-                assertParsedInstance(XContentType.JSON, new BytesArray(instanceAsString), alternateVersion.getValue());
-            }
-        }
+    protected String[] getShuffleFieldsExceptions() {
+        //disable xcontent shuffling on the highlight builder
+        return new String[]{"fields"};
     }
 }

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

@@ -18,18 +18,15 @@
  */
 package org.elasticsearch.test;
 
-import org.elasticsearch.common.bytes.BytesArray;
+import org.elasticsearch.common.Strings;
 import org.elasticsearch.common.bytes.BytesReference;
 import org.elasticsearch.common.io.stream.Writeable;
 import org.elasticsearch.common.xcontent.ToXContent;
-import org.elasticsearch.common.xcontent.XContentBuilder;
 import org.elasticsearch.common.xcontent.XContentFactory;
 import org.elasticsearch.common.xcontent.XContentParser;
 import org.elasticsearch.common.xcontent.XContentType;
 
 import java.io.IOException;
-import java.util.Collections;
-import java.util.Map;
 
 public abstract class AbstractSerializingTestCase<T extends ToXContent & Writeable> extends AbstractWireSerializingTestCase<T> {
 
@@ -41,13 +38,9 @@ public abstract class AbstractSerializingTestCase<T extends ToXContent & Writeab
         for (int runs = 0; runs < NUMBER_OF_TEST_RUNS; runs++) {
             T testInstance = createTestInstance();
             XContentType xContentType = randomFrom(XContentType.values());
-            XContentBuilder builder = toXContent(testInstance, xContentType);
-            XContentBuilder shuffled = shuffleXContent(builder);
-            assertParsedInstance(xContentType, shuffled.bytes(), testInstance);
-            for (Map.Entry<String, T> alternateVersion : getAlternateVersions().entrySet()) {
-                String instanceAsString = alternateVersion.getKey();
-                assertParsedInstance(XContentType.JSON, new BytesArray(instanceAsString), alternateVersion.getValue());
-            }
+            BytesReference shuffled = toShuffledXContent(testInstance, xContentType, ToXContent.EMPTY_PARAMS,
+                    false, getShuffleFieldsExceptions());
+            assertParsedInstance(xContentType, shuffled, testInstance);
         }
     }
 
@@ -73,38 +66,9 @@ public abstract class AbstractSerializingTestCase<T extends ToXContent & Writeab
     protected abstract T doParseInstance(XContentParser parser) throws IOException;
 
     /**
-     * Renders the provided instance in XContent
-     *
-     * @param instance
-     *            the instance to render
-     * @param contentType
-     *            the content type to render to
+     * Fields that have to be ignored when shuffling as part of testFromXContent
      */
-    protected XContentBuilder toXContent(T instance, XContentType contentType)
-            throws IOException {
-        XContentBuilder builder = XContentFactory.contentBuilder(contentType);
-        if (randomBoolean()) {
-            builder.prettyPrint();
-        }
-        if (instance.isFragment()) {
-            builder.startObject();
-        }
-        instance.toXContent(builder, ToXContent.EMPTY_PARAMS);
-        if (instance.isFragment()) {
-            builder.endObject();
-        }
-        return builder;
+    protected String[] getShuffleFieldsExceptions() {
+        return Strings.EMPTY_ARRAY;
     }
-
-    /**
-     * Returns alternate string representation of the instance that need to be
-     * tested as they are never used as output of the test instance. By default
-     * there are no alternate versions.
-     *
-     * These alternatives must be JSON strings.
-     */
-    protected Map<String, T> getAlternateVersions() {
-        return Collections.emptyMap();
-    }
-
 }

test/framework/src/main/java/org/elasticsearch/test/AbstractStreamableTestCase.java

@@ -101,7 +101,7 @@ public abstract class AbstractStreamableTestCase<T extends Streamable> extends E
     /**
      * Round trip {@code instance} through binary serialization, setting the wire compatibility version to {@code version}.
      */
-    protected T copyInstance(T instance, Version version) throws IOException {
+    private T copyInstance(T instance, Version version) throws IOException {
         try (BytesStreamOutput output = new BytesStreamOutput()) {
             output.setVersion(version);
             instance.writeTo(output);

test/framework/src/main/java/org/elasticsearch/test/AbstractStreamableXContentTestCase.java

@@ -18,18 +18,17 @@
  */
 package org.elasticsearch.test;
 
-import org.elasticsearch.common.bytes.BytesArray;
 import org.elasticsearch.common.bytes.BytesReference;
 import org.elasticsearch.common.io.stream.Streamable;
 import org.elasticsearch.common.xcontent.ToXContent;
-import org.elasticsearch.common.xcontent.XContentBuilder;
 import org.elasticsearch.common.xcontent.XContentFactory;
+import org.elasticsearch.common.xcontent.XContentHelper;
 import org.elasticsearch.common.xcontent.XContentParser;
 import org.elasticsearch.common.xcontent.XContentType;
 
 import java.io.IOException;
-import java.util.Collections;
-import java.util.Map;
+
+import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertToXContentEquivalent;
 
 public abstract class AbstractStreamableXContentTestCase<T extends ToXContent & Streamable> extends AbstractStreamableTestCase<T> {
 
@@ -41,23 +40,38 @@ public abstract class AbstractStreamableXContentTestCase<T extends ToXContent &
         for (int runs = 0; runs < NUMBER_OF_TEST_RUNS; runs++) {
             T testInstance = createTestInstance();
             XContentType xContentType = randomFrom(XContentType.values());
-            XContentBuilder builder = toXContent(testInstance, xContentType);
-            XContentBuilder shuffled = shuffleXContent(builder);
-            assertParsedInstance(xContentType, shuffled.bytes(), testInstance);
-            for (Map.Entry<String, T> alternateVersion : getAlternateVersions().entrySet()) {
-                String instanceAsString = alternateVersion.getKey();
-                assertParsedInstance(XContentType.JSON, new BytesArray(instanceAsString), alternateVersion.getValue());
+            BytesReference shuffled = toShuffledXContent(testInstance, xContentType, ToXContent.EMPTY_PARAMS, false);
+            BytesReference withRandomFields;
+            if (supportsUnknownFields()) {
+                // we add a few random fields to check that parser is lenient on new fields
+                withRandomFields = XContentTestUtils.insertRandomFields(xContentType, shuffled, null, random());
+            } else {
+                withRandomFields = shuffled;
             }
+            XContentParser parser = createParser(XContentFactory.xContent(xContentType), withRandomFields);
+            T parsed = parseInstance(parser);
+            T expected = getExpectedFromXContent(testInstance);
+            assertNotSame(expected, parsed);
+            assertEquals(expected, parsed);
+            assertEquals(expected.hashCode(), parsed.hashCode());
+            assertToXContentEquivalent(shuffled, XContentHelper.toXContent(parsed, xContentType, false), xContentType);
         }
     }
 
-    private void assertParsedInstance(XContentType xContentType, BytesReference instanceAsBytes, T expectedInstance)
-            throws IOException {
-        XContentParser parser = createParser(XContentFactory.xContent(xContentType), instanceAsBytes);
-        T newInstance = parseInstance(parser);
-        assertNotSame(newInstance, expectedInstance);
-        assertEquals(expectedInstance, newInstance);
-        assertEquals(expectedInstance.hashCode(), newInstance.hashCode());
+    /**
+     * Returns the expected parsed object given the test object that the parser will be fed with.
+     * Useful in cases some fields are not written as part of toXContent, hence not parsed back.
+     */
+    protected T getExpectedFromXContent(T testInstance) {
+        return testInstance;
+    }
+
+    /**
+     * Indicates whether the parser supports unknown fields or not. In case it does, such behaviour will be tested by
+     * inserting random fields before parsing and checking that they don't make parsing fail.
+     */
+    protected boolean supportsUnknownFields() {
+        return true;
     }
 
     private T parseInstance(XContentParser parser) throws IOException {
@@ -70,33 +84,4 @@ public abstract class AbstractStreamableXContentTestCase<T extends ToXContent &
      * Parses to a new instance using the provided {@link XContentParser}
      */
     protected abstract T doParseInstance(XContentParser parser);
-
-    /**
-     * Renders the provided instance in XContent
-     * 
-     * @param instance
-     *            the instance to render
-     * @param contentType
-     *            the content type to render to
-     */
-    protected static <T extends ToXContent> XContentBuilder toXContent(T instance, XContentType contentType)
-            throws IOException {
-        XContentBuilder builder = XContentFactory.contentBuilder(contentType);
-        if (randomBoolean()) {
-            builder.prettyPrint();
-        }
-        instance.toXContent(builder, ToXContent.EMPTY_PARAMS);
-        return builder;
-    }
-
-    /**
-     * Returns alternate string representation of the instance that need to be
-     * tested as they are never used as output of the test instance. By default
-     * there are no alternate versions.
-     * 
-     * These alternatives must be JSON strings.
-     */
-    protected Map<String, T> getAlternateVersions() {
-        return Collections.emptyMap();
-    }
 }