From a94ac303601de37a3e9b37cc8f4b3028c7fb766a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christoph=20B=C3=BCscher?= Date: Thu, 1 Jun 2017 14:12:26 +0200 Subject: [PATCH] [Tests] Improve error message for failed xContentEquivalent() tests (#24828) For comparing actual and parsed object equality for the response parsing we currently rely on comparing the original xContent and the output of the parsed object. Currently we only have cryptic error messages if this comparison fails which are hard to read also because we recursively compare lists and maps of the xContent structures we compare. This commits leverages the existing NotEqualMessageBuilder for providing error messages that are more detailed and useful for debugging if an error occurs. --- .../hamcrest/ElasticsearchAssertions.java | 16 +- .../ElasticsearchAssertionsTests.java | 159 ++++++++++++++++++ 2 files changed, 171 insertions(+), 4 deletions(-) diff --git a/test/framework/src/main/java/org/elasticsearch/test/hamcrest/ElasticsearchAssertions.java b/test/framework/src/main/java/org/elasticsearch/test/hamcrest/ElasticsearchAssertions.java index 94be3486244..9a36a0c3a3d 100644 --- a/test/framework/src/main/java/org/elasticsearch/test/hamcrest/ElasticsearchAssertions.java +++ b/test/framework/src/main/java/org/elasticsearch/test/hamcrest/ElasticsearchAssertions.java @@ -65,6 +65,7 @@ import org.elasticsearch.search.SearchHit; import org.elasticsearch.search.SearchModule; import org.elasticsearch.search.suggest.Suggest; import org.elasticsearch.test.ESIntegTestCase; +import org.elasticsearch.test.NotEqualMessageBuilder; import org.elasticsearch.test.VersionUtils; import org.hamcrest.CoreMatchers; import org.hamcrest.Matcher; @@ -790,11 +791,19 @@ public class ElasticsearchAssertions { //1) whenever anything goes through a map while parsing, ordering is not preserved, which is perfectly ok //2) Jackson SMILE parser parses floats as double, which then get printed out as double (with double precision) //Note that byte[] holding binary values need special treatment as they need to be properly compared item per item. + Map actualMap = null; + Map expectedMap = null; try (XContentParser actualParser = xContentType.xContent().createParser(NamedXContentRegistry.EMPTY, actual)) { - Map actualMap = actualParser.map(); + actualMap = actualParser.map(); try (XContentParser expectedParser = xContentType.xContent().createParser(NamedXContentRegistry.EMPTY, expected)) { - Map expectedMap = expectedParser.map(); - assertMapEquals(expectedMap, actualMap); + expectedMap = expectedParser.map(); + try { + assertMapEquals(expectedMap, actualMap); + } catch (AssertionError error) { + NotEqualMessageBuilder message = new NotEqualMessageBuilder(); + message.compareMaps(actualMap, expectedMap); + throw new AssertionError("Error when comparing xContent.\n" + message.toString(), error); + } } } } @@ -802,7 +811,6 @@ public class ElasticsearchAssertions { /** * Compares two maps recursively, using arrays comparisons for byte[] through Arrays.equals(byte[], byte[]) */ - @SuppressWarnings("unchecked") private static void assertMapEquals(Map expected, Map actual) { assertEquals(expected.size(), actual.size()); for (Map.Entry expectedEntry : expected.entrySet()) { diff --git a/test/framework/src/test/java/org/elasticsearch/test/hamcrest/ElasticsearchAssertionsTests.java b/test/framework/src/test/java/org/elasticsearch/test/hamcrest/ElasticsearchAssertionsTests.java index ef6d0265b59..b111024c77a 100644 --- a/test/framework/src/test/java/org/elasticsearch/test/hamcrest/ElasticsearchAssertionsTests.java +++ b/test/framework/src/test/java/org/elasticsearch/test/hamcrest/ElasticsearchAssertionsTests.java @@ -24,13 +24,20 @@ import org.elasticsearch.common.io.stream.NamedWriteableRegistry; import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; import org.elasticsearch.common.io.stream.Streamable; +import org.elasticsearch.common.xcontent.XContentBuilder; +import org.elasticsearch.common.xcontent.XContentHelper; +import org.elasticsearch.common.xcontent.XContentParser; +import org.elasticsearch.common.xcontent.json.JsonXContent; import org.elasticsearch.test.ESTestCase; +import org.elasticsearch.test.RandomObjects; import java.io.IOException; import static java.util.Collections.emptyList; import static org.elasticsearch.test.VersionUtils.randomVersion; +import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertToXContentEquivalent; import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertVersionSerializable; +import static org.hamcrest.Matchers.containsString; public class ElasticsearchAssertionsTests extends ESTestCase { public void testAssertVersionSerializableIsOkWithIllegalArgumentException() { @@ -52,4 +59,156 @@ public class ElasticsearchAssertionsTests extends ESTestCase { throw new IllegalArgumentException("Not supported."); } } + + public void testAssertXContentEquivalent() throws IOException { + try (XContentBuilder original = JsonXContent.contentBuilder()) { + original.startObject(); + for (Object value : RandomObjects.randomStoredFieldValues(random(), original.contentType()).v1()) { + original.field(randomAlphaOfLength(10), value); + } + { + original.startObject(randomAlphaOfLength(10)); + for (Object value : RandomObjects.randomStoredFieldValues(random(), original.contentType()).v1()) { + original.field(randomAlphaOfLength(10), value); + } + original.endObject(); + } + { + original.startArray(randomAlphaOfLength(10)); + for (Object value : RandomObjects.randomStoredFieldValues(random(), original.contentType()).v1()) { + original.value(value); + } + original.endArray(); + } + original.endObject(); + + try (XContentBuilder copy = JsonXContent.contentBuilder(); + XContentParser parser = createParser(original.contentType().xContent(), original.bytes())) { + parser.nextToken(); + XContentHelper.copyCurrentStructure(copy.generator(), parser); + try (XContentBuilder copyShuffled = shuffleXContent(copy) ) { + assertToXContentEquivalent(original.bytes(), copyShuffled.bytes(), original.contentType()); + } + } + } + } + + public void testAssertXContentEquivalentErrors() throws IOException { + { + XContentBuilder builder = JsonXContent.contentBuilder(); + builder.startObject(); + { + builder.startObject("foo"); + { + builder.field("f1", "value1"); + builder.field("f2", "value2"); + } + builder.endObject(); + } + builder.endObject(); + + XContentBuilder otherBuilder = JsonXContent.contentBuilder(); + otherBuilder.startObject(); + { + otherBuilder.startObject("foo"); + { + otherBuilder.field("f1", "value1"); + } + otherBuilder.endObject(); + } + otherBuilder.endObject(); + AssertionError error = expectThrows(AssertionError.class, + () -> assertToXContentEquivalent(builder.bytes(), otherBuilder.bytes(), builder.contentType())); + assertThat(error.getMessage(), containsString("f2: expected [value2] but not found")); + } + { + XContentBuilder builder = JsonXContent.contentBuilder(); + builder.startObject(); + { + builder.startObject("foo"); + { + builder.field("f1", "value1"); + builder.field("f2", "value2"); + } + builder.endObject(); + } + builder.endObject(); + + XContentBuilder otherBuilder = JsonXContent.contentBuilder(); + otherBuilder.startObject(); + { + otherBuilder.startObject("foo"); + { + otherBuilder.field("f1", "value1"); + otherBuilder.field("f2", "differentValue2"); + } + otherBuilder.endObject(); + } + otherBuilder.endObject(); + AssertionError error = expectThrows(AssertionError.class, + () -> assertToXContentEquivalent(builder.bytes(), otherBuilder.bytes(), builder.contentType())); + assertThat(error.getMessage(), containsString("f2: expected [value2] but was [differentValue2]")); + } + { + XContentBuilder builder = JsonXContent.contentBuilder(); + builder.startObject(); + { + builder.startArray("foo"); + { + builder.value("one"); + builder.value("two"); + builder.value("three"); + } + builder.endArray(); + } + builder.field("f1", "value"); + builder.endObject(); + + XContentBuilder otherBuilder = JsonXContent.contentBuilder(); + otherBuilder.startObject(); + { + otherBuilder.startArray("foo"); + { + otherBuilder.value("one"); + otherBuilder.value("two"); + otherBuilder.value("four"); + } + otherBuilder.endArray(); + } + otherBuilder.field("f1", "value"); + otherBuilder.endObject(); + AssertionError error = expectThrows(AssertionError.class, + () -> assertToXContentEquivalent(builder.bytes(), otherBuilder.bytes(), builder.contentType())); + assertThat(error.getMessage(), containsString("2: expected [three] but was [four]")); + } + { + XContentBuilder builder = JsonXContent.contentBuilder(); + builder.startObject(); + { + builder.startArray("foo"); + { + builder.value("one"); + builder.value("two"); + builder.value("three"); + } + builder.endArray(); + } + builder.endObject(); + + XContentBuilder otherBuilder = JsonXContent.contentBuilder(); + otherBuilder.startObject(); + { + otherBuilder.startArray("foo"); + { + otherBuilder.value("one"); + otherBuilder.value("two"); + } + otherBuilder.endArray(); + } + otherBuilder.endObject(); + AssertionError error = expectThrows(AssertionError.class, + () -> assertToXContentEquivalent(builder.bytes(), otherBuilder.bytes(), builder.contentType())); + assertThat(error.getMessage(), containsString("expected [1] more entries")); + } + } }