From 62462f5d9b579c11144f5d402128776ab70d9be0 Mon Sep 17 00:00:00 2001 From: javanna Date: Fri, 24 Jun 2016 12:11:48 +0200 Subject: [PATCH] [TEST] replace ResponseBodyAssertion with existing MatchAssertion We introduced a special response_body assertion to test our docs snippets. The match assertion does the same job though and can be reused and adapted where needed. ResponseBodyAssertion contains provides much better and accurate errors though, which can be now utilized in MatchAssertion so that many more REST tests can benefit from readable error messages. Each response body gets always stashed and can be retrieved for later evaluations already. Instead of providing the response body as strings that get parsed to json objects separately, then converted to maps as ResponseBodyAssertion did, we parse everything once, the json is part of the yaml test, which is supported. The only downside is that json comments cannot be used, rather yaml comments should be used (// C style vs # ). There were only two docs tests that were using comments in ingest-node.asciidoc where I went ahead and remove the comments which didn't seem that useful anyways. --- .../doc/RestTestsFromSnippetsTask.groovy | 5 +- docs/reference/ingest/ingest-node.asciidoc | 2 - .../parser/RestTestSuiteParseContext.java | 10 +- .../test/rest/section/MatchAssertion.java | 132 +++++++++++++- .../rest/section/ResponseBodyAssertion.java | 170 ------------------ 5 files changed, 131 insertions(+), 188 deletions(-) delete mode 100644 test/framework/src/main/java/org/elasticsearch/test/rest/section/ResponseBodyAssertion.java diff --git a/buildSrc/src/main/groovy/org/elasticsearch/gradle/doc/RestTestsFromSnippetsTask.groovy b/buildSrc/src/main/groovy/org/elasticsearch/gradle/doc/RestTestsFromSnippetsTask.groovy index ba7311fee6f..c7f4316ee04 100644 --- a/buildSrc/src/main/groovy/org/elasticsearch/gradle/doc/RestTestsFromSnippetsTask.groovy +++ b/buildSrc/src/main/groovy/org/elasticsearch/gradle/doc/RestTestsFromSnippetsTask.groovy @@ -131,8 +131,9 @@ public class RestTestsFromSnippetsTask extends SnippetsTask { } private void response(Snippet response) { - current.println(" - response_body: |") - response.contents.eachLine { current.println(" $it") } + current.println(" - match: ") + current.println(" \$body: ") + response.contents.eachLine { current.println(" $it") } } void emitDo(String method, String pathAndQuery, diff --git a/docs/reference/ingest/ingest-node.asciidoc b/docs/reference/ingest/ingest-node.asciidoc index b03ed641de7..ec4f9c30e66 100644 --- a/docs/reference/ingest/ingest-node.asciidoc +++ b/docs/reference/ingest/ingest-node.asciidoc @@ -46,7 +46,6 @@ PUT _ingest/pipeline/my-pipeline-id "value": "bar" } } - // other processors ] } -------------------------------------------------- @@ -83,7 +82,6 @@ Example response: "value": "bar" } } - // other processors ] } } ] diff --git a/test/framework/src/main/java/org/elasticsearch/test/rest/parser/RestTestSuiteParseContext.java b/test/framework/src/main/java/org/elasticsearch/test/rest/parser/RestTestSuiteParseContext.java index e972aea641a..7edad9995bb 100644 --- a/test/framework/src/main/java/org/elasticsearch/test/rest/parser/RestTestSuiteParseContext.java +++ b/test/framework/src/main/java/org/elasticsearch/test/rest/parser/RestTestSuiteParseContext.java @@ -18,22 +18,21 @@ */ package org.elasticsearch.test.rest.parser; -import java.io.IOException; -import java.util.HashMap; -import java.util.Map; - import org.elasticsearch.common.collect.Tuple; import org.elasticsearch.common.xcontent.XContentLocation; import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.common.xcontent.XContentParser.Token; import org.elasticsearch.test.rest.section.DoSection; import org.elasticsearch.test.rest.section.ExecutableSection; -import org.elasticsearch.test.rest.section.ResponseBodyAssertion; import org.elasticsearch.test.rest.section.SetupSection; import org.elasticsearch.test.rest.section.SkipSection; import org.elasticsearch.test.rest.section.TeardownSection; import org.elasticsearch.test.rest.section.TestSection; +import java.io.IOException; +import java.util.HashMap; +import java.util.Map; + /** * Context shared across the whole tests parse phase. * Provides shared parse methods and holds information needed to parse the test sections (e.g. es version) @@ -57,7 +56,6 @@ public class RestTestSuiteParseContext { EXECUTABLE_SECTIONS_PARSERS.put("lt", new LessThanParser()); EXECUTABLE_SECTIONS_PARSERS.put("lte", new LessThanOrEqualToParser()); EXECUTABLE_SECTIONS_PARSERS.put("length", new LengthParser()); - EXECUTABLE_SECTIONS_PARSERS.put("response_body", ResponseBodyAssertion.PARSER); } private final String api; diff --git a/test/framework/src/main/java/org/elasticsearch/test/rest/section/MatchAssertion.java b/test/framework/src/main/java/org/elasticsearch/test/rest/section/MatchAssertion.java index e00fbbea01c..49b82dfafac 100644 --- a/test/framework/src/main/java/org/elasticsearch/test/rest/section/MatchAssertion.java +++ b/test/framework/src/main/java/org/elasticsearch/test/rest/section/MatchAssertion.java @@ -18,15 +18,21 @@ */ package org.elasticsearch.test.rest.section; +import org.elasticsearch.common.Nullable; import org.elasticsearch.common.logging.ESLogger; import org.elasticsearch.common.logging.Loggers; +import java.util.List; +import java.util.Locale; +import java.util.Map; +import java.util.Objects; +import java.util.TreeMap; import java.util.regex.Pattern; import static org.elasticsearch.test.hamcrest.RegexMatcher.matches; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.instanceOf; -import static org.hamcrest.Matchers.notNullValue; +import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertThat; /** @@ -45,7 +51,6 @@ public class MatchAssertion extends Assertion { @Override protected void doAssert(Object actualValue, Object expectedValue) { - //if the value is wrapped into / it is a regexp (e.g. /s+d+/) if (expectedValue instanceof String) { String expValue = ((String) expectedValue).trim(); @@ -60,20 +65,131 @@ public class MatchAssertion extends Assertion { } } - assertThat(errorMessage(), actualValue, notNullValue()); + assertNotNull("field [" + getField() + "] is null", actualValue); logger.trace("assert that [{}] matches [{}] (field [{}])", actualValue, expectedValue, getField()); - if (!actualValue.getClass().equals(safeClass(expectedValue))) { + if (actualValue.getClass().equals(safeClass(expectedValue)) == false) { if (actualValue instanceof Number && expectedValue instanceof Number) { //Double 1.0 is equal to Integer 1 - assertThat(errorMessage(), ((Number) actualValue).doubleValue(), equalTo(((Number) expectedValue).doubleValue())); + assertThat("field [" + getField() + "] doesn't match the expected value", + ((Number) actualValue).doubleValue(), equalTo(((Number) expectedValue).doubleValue())); return; } } - assertThat(errorMessage(), actualValue, equalTo(expectedValue)); + if (expectedValue.equals(actualValue) == false) { + FailureMessage message = new FailureMessage(getField()); + message.compare(getField(), actualValue, expectedValue); + throw new AssertionError(message.message); + } } - private String errorMessage() { - return "field [" + getField() + "] doesn't match the expected value"; + private static class FailureMessage { + private final StringBuilder message; + private int indent = 0; + + private FailureMessage(String field) { + this.message = new StringBuilder(field + " didn't match the expected value:\n"); + } + + private void compareMaps(Map actual, Map expected) { + actual = new TreeMap<>(actual); + expected = new TreeMap<>(expected); + for (Map.Entry expectedEntry : expected.entrySet()) { + compare(expectedEntry.getKey(), actual.remove(expectedEntry.getKey()), expectedEntry.getValue()); + } + for (Map.Entry unmatchedEntry : actual.entrySet()) { + field(unmatchedEntry.getKey(), "unexpected but found [" + unmatchedEntry.getValue() + "]"); + } + } + + private void compareLists(List actual, List expected) { + int i = 0; + while (i < actual.size() && i < expected.size()) { + compare(Integer.toString(i), actual.get(i), expected.get(i)); + i++; + } + if (actual.size() == expected.size()) { + return; + } + indent(); + if (actual.size() < expected.size()) { + message.append("expected [").append(expected.size() - i).append("] more entries\n"); + return; + } + message.append("received [").append(actual.size() - i).append("] more entries than expected\n"); + } + + private void compare(String field, @Nullable Object actual, Object expected) { + if (expected instanceof Map) { + if (actual == null) { + field(field, "expected map but not found"); + return; + } + if (false == actual instanceof Map) { + field(field, "expected map but found [" + actual + "]"); + return; + } + @SuppressWarnings("unchecked") + Map expectedMap = (Map) expected; + @SuppressWarnings("unchecked") + Map actualMap = (Map) actual; + if (expectedMap.isEmpty() && actualMap.isEmpty()) { + field(field, "same [empty map]"); + return; + } + field(field, null); + indent += 1; + compareMaps(actualMap, expectedMap); + indent -= 1; + return; + } + if (expected instanceof List) { + if (actual == null) { + field(field, "expected list but not found"); + return; + } + if (false == actual instanceof List) { + field(field, "expected list but found [" + actual + "]"); + return; + } + @SuppressWarnings("unchecked") + List expectedList = (List) expected; + @SuppressWarnings("unchecked") + List actualList = (List) actual; + if (expectedList.isEmpty() && actualList.isEmpty()) { + field(field, "same [empty list]"); + return; + } + field(field, null); + indent += 1; + compareLists(actualList, expectedList); + indent -= 1; + return; + } + if (actual == null) { + field(field, "expected [" + expected + "] but not found"); + return; + } + if (Objects.equals(expected, actual)) { + field(field, "same [" + expected + "]"); + return; + } + field(field, "expected [" + expected + "] but was [" + actual + "]"); + } + + private void indent() { + for (int i = 0; i < indent; i++) { + message.append(" "); + } + } + + private void field(Object name, String info) { + indent(); + message.append(String.format(Locale.ROOT, "%30s: ", name)); + if (info != null) { + message.append(info); + } + message.append('\n'); + } } } diff --git a/test/framework/src/main/java/org/elasticsearch/test/rest/section/ResponseBodyAssertion.java b/test/framework/src/main/java/org/elasticsearch/test/rest/section/ResponseBodyAssertion.java deleted file mode 100644 index 3ead65a2111..00000000000 --- a/test/framework/src/main/java/org/elasticsearch/test/rest/section/ResponseBodyAssertion.java +++ /dev/null @@ -1,170 +0,0 @@ -/* - * Licensed to Elasticsearch under one or more contributor - * license agreements. See the NOTICE file distributed with - * this work for additional information regarding copyright - * ownership. Elasticsearch licenses this file to you under - * the Apache License, Version 2.0 (the "License"); you may - * not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ -package org.elasticsearch.test.rest.section; - -import java.io.IOException; -import java.util.List; -import java.util.Locale; -import java.util.Map; -import java.util.Objects; -import java.util.TreeMap; - -import org.elasticsearch.common.Nullable; -import org.elasticsearch.common.xcontent.XContentParser; -import org.elasticsearch.common.xcontent.json.JsonXContent; -import org.elasticsearch.test.rest.parser.RestTestFragmentParser; -import org.elasticsearch.test.rest.parser.RestTestParseException; -import org.elasticsearch.test.rest.parser.RestTestSuiteParseContext; - -/** - * Checks that the response body matches some text. - */ -public class ResponseBodyAssertion extends Assertion { - public static final RestTestFragmentParser PARSER = new RestTestFragmentParser() { - @Override - public ResponseBodyAssertion parse(RestTestSuiteParseContext parseContext) throws IOException, RestTestParseException { - try (XContentParser parser = JsonXContent.jsonXContent.createParser(parseContext.parseField())) { - return new ResponseBodyAssertion("$body", parser.map()); - } - } - }; - - private ResponseBodyAssertion(String field, Map expectedValue) { - super(field, expectedValue); - } - - @Override - protected void doAssert(Object actualValue, Object expectedValue) { - if (false == expectedValue.equals(actualValue)) { - @SuppressWarnings("unchecked") - Map actual = (Map) actualValue; - @SuppressWarnings("unchecked") - Map expected = (Map) expectedValue; - FailureMessage message = new FailureMessage(); - message.compareMaps(actual, expected); - throw new AssertionError(message.message); - } - } - - private class FailureMessage { - private final StringBuilder message = new StringBuilder("body didn't match the expected value:\n"); - private int indent = 0; - - private void compareMaps(Map actual, Map expected) { - actual = new TreeMap<>(actual); - expected = new TreeMap<>(expected); - for (Map.Entry expectedEntry : expected.entrySet()) { - compare(expectedEntry.getKey(), actual.remove(expectedEntry.getKey()), expectedEntry.getValue()); - } - for (Map.Entry unmatchedEntry : actual.entrySet()) { - field(unmatchedEntry.getKey(), "unexpected but found [" + unmatchedEntry.getValue() + "]"); - } - } - - private void compareLists(List actual, List expected) { - int i = 0; - while (i < actual.size() && i < expected.size()) { - compare(i, actual.get(i), expected.get(i)); - i++; - } - if (actual.size() == expected.size()) { - return; - } - indent(); - if (actual.size() < expected.size()) { - message.append("expected [").append(expected.size() - i).append("] more entries\n"); - return; - } - message.append("received [").append(actual.size() - i).append("] more entries than expected\n"); - } - - private void compare(Object field, @Nullable Object actual, Object expected) { - if (expected instanceof Map) { - if (actual == null) { - field(field, "expected map but not found"); - return; - } - if (false == actual instanceof Map) { - field(field, "expected map but found [" + actual + "]"); - return; - } - @SuppressWarnings("unchecked") - Map expectedMap = (Map) expected; - @SuppressWarnings("unchecked") - Map actualMap = (Map) actual; - if (expectedMap.isEmpty() && actualMap.isEmpty()) { - field(field, "same [empty map]"); - return; - } - field(field, null); - indent += 1; - compareMaps(actualMap, expectedMap); - indent -= 1; - return; - } - if (expected instanceof List) { - if (actual == null) { - field(field, "expected list but not found"); - return; - } - if (false == actual instanceof List) { - field(field, "expected list but found [" + actual + "]"); - return; - } - @SuppressWarnings("unchecked") - List expectedList = (List) expected; - @SuppressWarnings("unchecked") - List actualList = (List) actual; - if (expectedList.isEmpty() && actualList.isEmpty()) { - field(field, "same [empty list]"); - return; - } - field(field, null); - indent += 1; - compareLists(actualList, expectedList); - indent -= 1; - return; - } - if (actual == null) { - field(field, "expected [" + expected + "] but not found"); - return; - } - if (Objects.equals(expected, actual)) { - field(field, "same [" + expected + "]"); - return; - } - field(field, "expected [" + expected + "] but was [" + actual + "]"); - } - - private void indent() { - for (int i = 0; i < indent; i++) { - message.append(" "); - } - } - - private void field(Object name, String info) { - indent(); - message.append(String.format(Locale.ROOT, "%30s: ", name)); - if (info != null) { - message.append(info); - } - message.append('\n'); - } - } -}