Addressing review comments

core/src/test/java/org/elasticsearch/index/query/AbstractQueryTestCase.java

@@ -383,7 +383,7 @@ public abstract class AbstractQueryTestCase<QB extends AbstractQueryBuilder<QB>>
         for (int runs = 0; runs < NUMBER_OF_TESTQUERIES; runs++) {
             QB testQuery = createTestQueryBuilder();
             XContentBuilder builder = toXContent(testQuery, randomFrom(XContentType.values()));
-            XContentBuilder shuffled = shuffleXContent(builder, provideShuffleproofFields());
+            XContentBuilder shuffled = shuffleXContent(builder, shuffleProtectedFields());
             assertParsedQuery(shuffled.bytes(), testQuery);
             for (Map.Entry<String, QB> alternateVersion : getAlternateVersions().entrySet()) {
                 String queryAsString = alternateVersion.getKey();
@@ -393,10 +393,10 @@ public abstract class AbstractQueryTestCase<QB extends AbstractQueryBuilder<QB>>
     }
 
     /**
-     * subclasses should override this method in case some fields in xContent should be protected from random
+     * Subclasses can override this method and return a set of fields which should be protected from
-     * shuffling in the {@link #testFromXContent()} test case
+     * recursive random shuffling in the {@link #testFromXContent()} test case
      */
-    protected Set<String> provideShuffleproofFields() {
+    protected Set<String> shuffleProtectedFields() {
         return Collections.emptySet();
     }
 

core/src/test/java/org/elasticsearch/index/query/PercolatorQueryBuilderTests.java

@@ -38,7 +38,6 @@ import org.hamcrest.Matchers;
 
 import java.io.IOException;
 import java.util.Collections;
-import java.util.HashSet;
 import java.util.Set;
 
 import static org.hamcrest.Matchers.containsString;
@@ -46,6 +45,8 @@ import static org.hamcrest.Matchers.equalTo;
 
 public class PercolatorQueryBuilderTests extends AbstractQueryTestCase<PercolatorQueryBuilder> {
 
+    private static final Set<String> SHUFFLE_PROTECTED_FIELDS =
+            Collections.singleton(PercolatorQueryParser.DOCUMENT_FIELD.getPreferredName());
     private String indexedDocumentIndex;
     private String indexedDocumentType;
     private String indexedDocumentId;
@@ -84,10 +85,8 @@ public class PercolatorQueryBuilderTests extends AbstractQueryTestCase<Percolato
      * BytesReference
      */
     @Override
-    protected Set<String> provideShuffleproofFields() {
+    protected Set<String> shuffleProtectedFields() {
-        Set<String> fieldNames = new HashSet<>();
+        return SHUFFLE_PROTECTED_FIELDS;
-        fieldNames.add(PercolatorQueryParser.DOCUMENT_FIELD.getPreferredName());
-        return fieldNames;
     }
 
     @Override

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

@@ -78,7 +78,6 @@ import java.util.Collection;
 import java.util.Collections;
 import java.util.List;
 import java.util.Map;
-import java.util.Random;
 import java.util.Set;
 import java.util.TreeMap;
 import java.util.concurrent.Callable;
@@ -605,23 +604,31 @@ public abstract class ESTestCase extends LuceneTestCase {
         return tempList.subList(0, size);
     }
 
+    /**
+     * Randomly shuffles the fields inside objects in the {@link XContentBuilder} passed in.
+     * Recursively goes through inner objects and also shuffles them. Exceptions for this
+     * recursive shuffling behavior can be made by passing in the names of fields which
+     * internally should stay untouched.
+     */
     public static XContentBuilder shuffleXContent(XContentBuilder builder, Set<String> exceptFieldNames) throws IOException {
         BytesReference bytes = builder.bytes();
         XContentParser parser = XContentFactory.xContent(bytes).createParser(bytes);
         // use ordered maps for reproducibility
-        Map<String, Object> shuffledMap = shuffleMap(parser.mapOrdered(), exceptFieldNames, random());
+        Map<String, Object> shuffledMap = shuffleMap(parser.mapOrdered(), exceptFieldNames);
-        XContentBuilder jsonBuilder = XContentFactory.jsonBuilder();
+        XContentBuilder jsonBuilder = XContentFactory.contentBuilder(builder.contentType());
         return jsonBuilder.map(shuffledMap);
     }
 
-    private static Map<String, Object> shuffleMap(Map<String, Object> map, Set<String> exceptFieldNames, Random r) {
+    private static Map<String, Object> shuffleMap(Map<String, Object> map, Set<String> exceptFieldNames) {
         List<String> keys = new ArrayList<>(map.keySet());
+        // even though we shuffle later, we need this to make tests reproduce on different jvms
+        //Collections.sort(keys);
         Map<String, Object> targetMap = new TreeMap<>();
         Collections.shuffle(keys, random());
         for (String key : keys) {
             Object value = map.get(key);
             if (value instanceof Map && exceptFieldNames.contains(key) == false) {
-                targetMap.put(key, shuffleMap((Map) value, exceptFieldNames, r));
+                targetMap.put(key, shuffleMap((Map) value, exceptFieldNames));
             } else {
                 targetMap.put(key, value);
             }

test/framework/src/test/java/org/elasticsearch/test/test/ESTestCaseTests.java

@@ -24,6 +24,7 @@ import junit.framework.AssertionFailedError;
 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 java.io.IOException;
@@ -63,13 +64,13 @@ public class ESTestCaseTests extends ESTestCase {
 
     public void testShuffleXContent() throws IOException {
         Map<String, Object> randomStringObjectMap = randomStringObjectMap(5);
-        XContentBuilder builder = XContentFactory.jsonBuilder();
+        XContentBuilder builder = XContentFactory.contentBuilder(randomFrom(XContentType.values()));
         builder.map(randomStringObjectMap);
         XContentBuilder shuffleXContent = shuffleXContent(builder, Collections.emptySet());
         XContentParser parser = XContentFactory.xContent(shuffleXContent.bytes()).createParser(shuffleXContent.bytes());
         Map<String, Object> resultMap = parser.map();
         assertEquals("both maps should contain the same mappings", randomStringObjectMap, resultMap);
-        assertNotEquals("Both builders string representations should be different", builder.string(), shuffleXContent.string());
+        assertNotEquals("Both builders string representations should be different", builder.bytes(), shuffleXContent.bytes());
     }
 
     private static Map<String, Object> randomStringObjectMap(int depth) {