From bbb6d91147676d03c9b904a6bf1d09f4e4b00096 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christoph=20B=C3=BCscher?= Date: Thu, 31 Mar 2016 17:09:35 +0200 Subject: [PATCH] Add randomization of XContentBuilder output to query tests Currently our testing of parsing query builders is limited to the default order of the parameters that each builders toXContent() method produces. To better test real queries where the order of parameters can be different, this change adds a helper method to ESTestCase that takes a XContentBuilder and randomly shuffles the order of the fields inside an object. This is used in AbstractQueryTestCase, but it can be used in other similar places in the future. --- .../index/query/AbstractQueryTestCase.java | 12 +++- .../query/PercolatorQueryBuilderTests.java | 16 +++++ .../org/elasticsearch/test/ESTestCase.java | 33 +++++++++- .../test/test/ESTestCaseTests.java | 60 +++++++++++++++++++ 4 files changed, 119 insertions(+), 2 deletions(-) diff --git a/core/src/test/java/org/elasticsearch/index/query/AbstractQueryTestCase.java b/core/src/test/java/org/elasticsearch/index/query/AbstractQueryTestCase.java index bc45ffc7df5..0129fe1cc57 100644 --- a/core/src/test/java/org/elasticsearch/index/query/AbstractQueryTestCase.java +++ b/core/src/test/java/org/elasticsearch/index/query/AbstractQueryTestCase.java @@ -22,6 +22,7 @@ package org.elasticsearch.index.query; import com.carrotsearch.randomizedtesting.generators.CodepointSetGenerator; import com.fasterxml.jackson.core.JsonParseException; import com.fasterxml.jackson.core.io.JsonStringEncoder; + import org.apache.lucene.search.BoostQuery; import org.apache.lucene.search.Query; import org.apache.lucene.search.TermQuery; @@ -382,7 +383,8 @@ public abstract class AbstractQueryTestCase> for (int runs = 0; runs < NUMBER_OF_TESTQUERIES; runs++) { QB testQuery = createTestQueryBuilder(); XContentBuilder builder = toXContent(testQuery, randomFrom(XContentType.values())); - assertParsedQuery(builder.bytes(), testQuery); + XContentBuilder shuffled = shuffleXContent(builder, provideShuffleproofFields()); + assertParsedQuery(shuffled.bytes(), testQuery); for (Map.Entry alternateVersion : getAlternateVersions().entrySet()) { String queryAsString = alternateVersion.getKey(); assertParsedQuery(new BytesArray(queryAsString), alternateVersion.getValue(), ParseFieldMatcher.EMPTY); @@ -390,6 +392,14 @@ public abstract class AbstractQueryTestCase> } } + /** + * subclasses should override this method in case some fields in xContent should be protected from random + * shuffling in the {@link #testFromXContent()} test case + */ + protected Set provideShuffleproofFields() { + return Collections.emptySet(); + } + protected static XContentBuilder toXContent(QueryBuilder query, XContentType contentType) throws IOException { XContentBuilder builder = XContentFactory.contentBuilder(contentType); if (randomBoolean()) { diff --git a/core/src/test/java/org/elasticsearch/index/query/PercolatorQueryBuilderTests.java b/core/src/test/java/org/elasticsearch/index/query/PercolatorQueryBuilderTests.java index 30d4ec908b8..864be91a401 100644 --- a/core/src/test/java/org/elasticsearch/index/query/PercolatorQueryBuilderTests.java +++ b/core/src/test/java/org/elasticsearch/index/query/PercolatorQueryBuilderTests.java @@ -20,6 +20,7 @@ package org.elasticsearch.index.query; import com.fasterxml.jackson.core.JsonParseException; + import org.apache.lucene.search.Query; import org.elasticsearch.ElasticsearchParseException; import org.elasticsearch.ResourceNotFoundException; @@ -37,6 +38,8 @@ 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; import static org.hamcrest.Matchers.equalTo; @@ -75,6 +78,18 @@ public class PercolatorQueryBuilderTests extends AbstractQueryTestCase provideShuffleproofFields() { + Set fieldNames = new HashSet<>(); + fieldNames.add(PercolatorQueryParser.DOCUMENT_FIELD.getPreferredName()); + return fieldNames; + } + @Override protected GetResponse executeGet(GetRequest getRequest) { assertThat(getRequest.index(), Matchers.equalTo(indexedDocumentIndex)); @@ -132,6 +147,7 @@ public class PercolatorQueryBuilderTests extends AbstractQueryTestCase exceptFieldNames) throws IOException { + BytesReference bytes = builder.bytes(); + XContentParser parser = XContentFactory.xContent(bytes).createParser(bytes); + // use ordered maps for reproducibility + Map shuffledMap = shuffleMap(parser.mapOrdered(), exceptFieldNames, random()); + XContentBuilder jsonBuilder = XContentFactory.jsonBuilder(); + return jsonBuilder.map(shuffledMap); + } + + private static Map shuffleMap(Map map, Set exceptFieldNames, Random r) { + List keys = new ArrayList<>(map.keySet()); + Map 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)); + } else { + targetMap.put(key, value); + } + } + return targetMap; + } + /** * Returns true iff assertions for elasticsearch packages are enabled */ diff --git a/test/framework/src/test/java/org/elasticsearch/test/test/ESTestCaseTests.java b/test/framework/src/test/java/org/elasticsearch/test/test/ESTestCaseTests.java index 0d44fc4abcd..2f62790d95b 100644 --- a/test/framework/src/test/java/org/elasticsearch/test/test/ESTestCaseTests.java +++ b/test/framework/src/test/java/org/elasticsearch/test/test/ESTestCaseTests.java @@ -20,9 +20,21 @@ package org.elasticsearch.test.test; 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.test.ESTestCase; +import java.io.IOException; +import java.util.ArrayList; +import java.util.Collections; +import java.util.HashMap; +import java.util.List; +import java.util.Map; + public class ESTestCaseTests extends ESTestCase { + public void testExpectThrows() { IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> { throw new IllegalArgumentException("bad arg"); @@ -48,4 +60,52 @@ public class ESTestCaseTests extends ESTestCase { assertEquals("Expected exception IllegalArgumentException", assertFailed.getMessage()); } } + + public void testShuffleXContent() throws IOException { + Map randomStringObjectMap = randomStringObjectMap(5); + XContentBuilder builder = XContentFactory.jsonBuilder(); + builder.map(randomStringObjectMap); + XContentBuilder shuffleXContent = shuffleXContent(builder, Collections.emptySet()); + XContentParser parser = XContentFactory.xContent(shuffleXContent.bytes()).createParser(shuffleXContent.bytes()); + Map 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()); + } + + private static Map randomStringObjectMap(int depth) { + Map result = new HashMap<>(); + int entries = randomInt(10); + for (int i = 0; i < entries; i++) { + String key = randomAsciiOfLengthBetween(5, 15); + int suprise = randomIntBetween(0, 4); + switch (suprise) { + case 0: + result.put(key, randomUnicodeOfCodepointLength(20)); + break; + case 1: + result.put(key, randomInt(100)); + break; + case 2: + result.put(key, randomDoubleBetween(-100.0, 100.0, true)); + break; + case 3: + result.put(key, randomBoolean()); + break; + case 4: + List stringList = new ArrayList<>(); + int size = randomInt(5); + for (int s = 0; s < size; s++) { + stringList.add(randomUnicodeOfCodepointLength(20)); + } + result.put(key, stringList); + break; + default: + throw new IllegalArgumentException("unexpected random option: " + suprise); + } + } + if (depth > 0) { + result.put(randomAsciiOfLengthBetween(5, 15), randomStringObjectMap(depth - 1)); + } + return result; + } }