From b2be6afc78ed226a698a965fb134a8eaec0ddd71 Mon Sep 17 00:00:00 2001 From: Michael Froh Date: Wed, 29 May 2024 17:57:25 +0000 Subject: [PATCH] Add basic type conversion for YAML tests Our yaml rest tests parse expected values based on basic yaml parsing rules, which don't draw a distinction between float and double values. If there's a mismatch between the types of the expected and actual values in an assertion, we should try parsing the actual value to the expected type. For now, I just added support for the 32- and 64-bit integer and floating point types. Signed-off-by: Michael Froh --- .../test/rest/yaml/section/Assertion.java | 45 ++++++++++++++++++- .../rest/yaml/section/ContainsAssertion.java | 5 +++ .../rest/yaml/section/IsFalseAssertion.java | 5 +++ .../rest/yaml/section/IsTrueAssertion.java | 5 +++ .../rest/yaml/section/LengthAssertion.java | 5 +++ 5 files changed, 63 insertions(+), 2 deletions(-) diff --git a/test/framework/src/main/java/org/opensearch/test/rest/yaml/section/Assertion.java b/test/framework/src/main/java/org/opensearch/test/rest/yaml/section/Assertion.java index b9cbaacdf8873..bf8f41dca33fd 100644 --- a/test/framework/src/main/java/org/opensearch/test/rest/yaml/section/Assertion.java +++ b/test/framework/src/main/java/org/opensearch/test/rest/yaml/section/Assertion.java @@ -37,6 +37,8 @@ import java.io.IOException; import java.util.Map; +import static org.junit.Assert.fail; + /** * Base class for executable sections that hold assertions */ @@ -73,10 +75,49 @@ protected final Object resolveExpectedValue(ClientYamlTestExecutionContext execu } protected final Object getActualValue(ClientYamlTestExecutionContext executionContext) throws IOException { + Object actualValue; if (executionContext.stash().containsStashedValue(field)) { - return executionContext.stash().getValue(field); + actualValue = executionContext.stash().getValue(field); + } else { + actualValue = executionContext.response(field); + } + // Expected/actual values have compatible types, or they're not expected to have the same. Can just return. + if (actualValue == null || expectedValue.getClass().isAssignableFrom(actualValue.getClass()) || !expectedAndActualHaveSameType()) { + return actualValue; + } + if (actualValue instanceof Number && expectedValue instanceof Number) { + if (expectedValue instanceof Float) { + return Float.parseFloat(actualValue.toString()); + } else if (expectedValue instanceof Double) { + return Double.parseDouble(actualValue.toString()); + } else if (expectedValue instanceof Integer) { + return Integer.parseInt(actualValue.toString()); + } else if (expectedValue instanceof Long) { + return Long.parseLong(actualValue.toString()); + } } - return executionContext.response(field); + // Force a class cast exception here, so developers can flesh out the above logic as needed. + try { + expectedValue.getClass().cast(actualValue); + } catch (ClassCastException e) { + fail( + "Type mismatch: Expected value (" + + expectedValue + + ") has type " + + expectedValue.getClass() + + ". " + + "Actual value (" + + actualValue + + ") has type " + + actualValue.getClass() + + "." + ); + } + return actualValue; + } + + protected boolean expectedAndActualHaveSameType() { + return true; } @Override diff --git a/test/framework/src/main/java/org/opensearch/test/rest/yaml/section/ContainsAssertion.java b/test/framework/src/main/java/org/opensearch/test/rest/yaml/section/ContainsAssertion.java index 8012b90b1b04f..e1ea8313d9546 100644 --- a/test/framework/src/main/java/org/opensearch/test/rest/yaml/section/ContainsAssertion.java +++ b/test/framework/src/main/java/org/opensearch/test/rest/yaml/section/ContainsAssertion.java @@ -91,4 +91,9 @@ protected void doAssert(Object actualValue, Object expectedValue) { fail("'contains' only supports checking an object against a list of objects"); } } + + @Override + protected boolean expectedAndActualHaveSameType() { + return false; + } } diff --git a/test/framework/src/main/java/org/opensearch/test/rest/yaml/section/IsFalseAssertion.java b/test/framework/src/main/java/org/opensearch/test/rest/yaml/section/IsFalseAssertion.java index 999486ad04455..0908a8d3c13d3 100644 --- a/test/framework/src/main/java/org/opensearch/test/rest/yaml/section/IsFalseAssertion.java +++ b/test/framework/src/main/java/org/opensearch/test/rest/yaml/section/IsFalseAssertion.java @@ -75,4 +75,9 @@ protected void doAssert(Object actualValue, Object expectedValue) { private String errorMessage() { return "field [" + getField() + "] has a true value but it shouldn't"; } + + @Override + protected boolean expectedAndActualHaveSameType() { + return false; + } } diff --git a/test/framework/src/main/java/org/opensearch/test/rest/yaml/section/IsTrueAssertion.java b/test/framework/src/main/java/org/opensearch/test/rest/yaml/section/IsTrueAssertion.java index bf5822406f014..68e154e8db778 100644 --- a/test/framework/src/main/java/org/opensearch/test/rest/yaml/section/IsTrueAssertion.java +++ b/test/framework/src/main/java/org/opensearch/test/rest/yaml/section/IsTrueAssertion.java @@ -75,4 +75,9 @@ protected void doAssert(Object actualValue, Object expectedValue) { private String errorMessage() { return "field [" + getField() + "] doesn't have a true value"; } + + @Override + protected boolean expectedAndActualHaveSameType() { + return false; + } } diff --git a/test/framework/src/main/java/org/opensearch/test/rest/yaml/section/LengthAssertion.java b/test/framework/src/main/java/org/opensearch/test/rest/yaml/section/LengthAssertion.java index fd7d8d632cc1e..dfbd6580f4403 100644 --- a/test/framework/src/main/java/org/opensearch/test/rest/yaml/section/LengthAssertion.java +++ b/test/framework/src/main/java/org/opensearch/test/rest/yaml/section/LengthAssertion.java @@ -97,4 +97,9 @@ protected void doAssert(Object actualValue, Object expectedValue) { private String errorMessage() { return "field [" + getField() + "] doesn't have length [" + getExpectedValue() + "]"; } + + @Override + protected boolean expectedAndActualHaveSameType() { + return false; // Expected value is a number, while actual will some type with a length. + } }