Fix bug "Assertion framework(Yaml Rest test): numeric comparison fails when comparing Integer vs Long (or Float vs Double)"

CHANGELOG.md

@@ -94,6 +94,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 - Fix NPE of ScriptScoreQuery ([#19650](https://github.com/opensearch-project/OpenSearch/pull/19650))
 - Fix ClassCastException in FlightClientChannel for requests larger than 16KB ([#20010](https://github.com/opensearch-project/OpenSearch/pull/20010))
 - Fix GRPC Bulk ([#19937](https://github.com/opensearch-project/OpenSearch/pull/19937))
+- Fix bug in Assertion framework(Yaml Rest test): numeric comparison fails when comparing Integer vs Long (or Float vs Double) ([#19376](https://github.com/opensearch-project/OpenSearch/pull/19376))
 - Fix node bootstrap error when enable stream transport and remote cluster state ([#19948](https://github.com/opensearch-project/OpenSearch/pull/19948))
 - Keep track and release Reactor Netty 4 Transport accepted Http Channels during the Node shutdown ([#20106](https://github.com/opensearch-project/OpenSearch/pull/20106))
 - Fix deletion failure/error of unused index template; case when an index template matches a data stream but has a lower priority. ([#20102](https://github.com/opensearch-project/OpenSearch/pull/20102))

test/framework/src/main/java/org/opensearch/test/rest/yaml/section/Assertion.java

@@ -91,7 +91,11 @@ static Object convertActualValue(Object actualValue, Object expectedValue) {
             } else if (expectedValue instanceof Double) {
                 return Double.parseDouble(actualValue.toString());
             } else if (expectedValue instanceof Integer) {
-                return Integer.parseInt(actualValue.toString());
+                try {
+                    return Long.parseLong(actualValue.toString());
+                } catch (NumberFormatException e) {
+                    return Integer.parseInt(actualValue.toString());
+                }
             } else if (expectedValue instanceof Long) {
                 return Long.parseLong(actualValue.toString());
             }

test/framework/src/main/java/org/opensearch/test/rest/yaml/section/GreaterThanAssertion.java

@@ -31,65 +31,32 @@
 
 package org.opensearch.test.rest.yaml.section;
 
-import org.apache.logging.log4j.LogManager;
-import org.apache.logging.log4j.Logger;
-import org.opensearch.common.collect.Tuple;
 import org.opensearch.core.xcontent.XContentLocation;
 import org.opensearch.core.xcontent.XContentParser;
 
 import java.io.IOException;
 
-import static org.hamcrest.Matchers.greaterThan;
-import static org.hamcrest.Matchers.instanceOf;
-import static org.junit.Assert.assertThat;
-import static org.junit.Assert.fail;
-
 /**
  * Represents a gt assert section:
  * <p>
  * - gt:    { fields._ttl: 0}
  */
-public class GreaterThanAssertion extends Assertion {
+public class GreaterThanAssertion extends OrderingAssertion {
     public static GreaterThanAssertion parse(XContentParser parser) throws IOException {
-        XContentLocation location = parser.getTokenLocation();
-        Tuple<String, Object> stringObjectTuple = ParserUtils.parseTuple(parser);
-        if (!(stringObjectTuple.v2() instanceof Comparable)) {
-            throw new IllegalArgumentException(
-                "gt section can only be used with objects that support natural ordering, found "
-                    + stringObjectTuple.v2().getClass().getSimpleName()
-            );
-        }
-        return new GreaterThanAssertion(location, stringObjectTuple.v1(), stringObjectTuple.v2());
+        return parseOrderingAssertion(parser, Relation.GT, GreaterThanAssertion::new);
     }
 
-    private static final Logger logger = LogManager.getLogger(GreaterThanAssertion.class);
-
-    public GreaterThanAssertion(XContentLocation location, String field, Object expectedValue) {
-        super(location, field, expectedValue);
+    public GreaterThanAssertion(XContentLocation loc, String field, Object expected) {
+        super(loc, field, expected);
     }
 
     @Override
-    protected void doAssert(Object actualValue, Object expectedValue) {
-        logger.trace("assert that [{}] is greater than [{}] (field: [{}])", actualValue, expectedValue, getField());
-        actualValue = convertActualValue(actualValue, expectedValue);
-        assertThat(
-            "value of [" + getField() + "] is not comparable (got [" + safeClass(actualValue) + "])",
-            actualValue,
-            instanceOf(Comparable.class)
-        );
-        assertThat(
-            "expected value of [" + getField() + "] is not comparable (got [" + expectedValue.getClass() + "])",
-            expectedValue,
-            instanceOf(Comparable.class)
-        );
-        try {
-            assertThat(errorMessage(), (Comparable) actualValue, greaterThan((Comparable) expectedValue));
-        } catch (ClassCastException e) {
-            fail("cast error while checking (" + errorMessage() + "): " + e);
-        }
+    protected Relation relation() {
+        return Relation.GT;
     }
 
-    private String errorMessage() {
+    @Override
+    protected String errorMessage() {
         return "field [" + getField() + "] is not greater than [" + getExpectedValue() + "]";
     }
 }

test/framework/src/main/java/org/opensearch/test/rest/yaml/section/GreaterThanEqualToAssertion.java

@@ -32,65 +32,32 @@
 
 package org.opensearch.test.rest.yaml.section;
 
-import org.apache.logging.log4j.LogManager;
-import org.apache.logging.log4j.Logger;
-import org.opensearch.common.collect.Tuple;
 import org.opensearch.core.xcontent.XContentLocation;
 import org.opensearch.core.xcontent.XContentParser;
 
 import java.io.IOException;
 
-import static org.hamcrest.Matchers.greaterThanOrEqualTo;
-import static org.hamcrest.Matchers.instanceOf;
-import static org.junit.Assert.assertThat;
-import static org.junit.Assert.fail;
-
 /**
  * Represents a gte assert section:
  * <p>
  *   - gte:     { fields._ttl: 0 }
  */
-public class GreaterThanEqualToAssertion extends Assertion {
+public class GreaterThanEqualToAssertion extends OrderingAssertion {
     public static GreaterThanEqualToAssertion parse(XContentParser parser) throws IOException {
-        XContentLocation location = parser.getTokenLocation();
-        Tuple<String, Object> stringObjectTuple = ParserUtils.parseTuple(parser);
-        if (!(stringObjectTuple.v2() instanceof Comparable)) {
-            throw new IllegalArgumentException(
-                "gte section can only be used with objects that support natural ordering, found "
-                    + stringObjectTuple.v2().getClass().getSimpleName()
-            );
-        }
-        return new GreaterThanEqualToAssertion(location, stringObjectTuple.v1(), stringObjectTuple.v2());
+        return parseOrderingAssertion(parser, Relation.GTE, GreaterThanEqualToAssertion::new);
     }
 
-    private static final Logger logger = LogManager.getLogger(GreaterThanEqualToAssertion.class);
-
-    public GreaterThanEqualToAssertion(XContentLocation location, String field, Object expectedValue) {
-        super(location, field, expectedValue);
+    public GreaterThanEqualToAssertion(XContentLocation loc, String field, Object expected) {
+        super(loc, field, expected);
     }
 
     @Override
-    protected void doAssert(Object actualValue, Object expectedValue) {
-        logger.trace("assert that [{}] is greater than or equal to [{}] (field: [{}])", actualValue, expectedValue, getField());
-        actualValue = convertActualValue(actualValue, expectedValue);
-        assertThat(
-            "value of [" + getField() + "] is not comparable (got [" + safeClass(actualValue) + "])",
-            actualValue,
-            instanceOf(Comparable.class)
-        );
-        assertThat(
-            "expected value of [" + getField() + "] is not comparable (got [" + expectedValue.getClass() + "])",
-            expectedValue,
-            instanceOf(Comparable.class)
-        );
-        try {
-            assertThat(errorMessage(), (Comparable) actualValue, greaterThanOrEqualTo((Comparable) expectedValue));
-        } catch (ClassCastException e) {
-            fail("cast error while checking (" + errorMessage() + "): " + e);
-        }
+    protected Relation relation() {
+        return Relation.GTE;
     }
 
-    private String errorMessage() {
+    @Override
+    protected String errorMessage() {
         return "field [" + getField() + "] is not greater than or equal to [" + getExpectedValue() + "]";
     }
 }

test/framework/src/main/java/org/opensearch/test/rest/yaml/section/LessThanAssertion.java

@@ -31,66 +31,33 @@
 
 package org.opensearch.test.rest.yaml.section;
 
-import org.apache.logging.log4j.LogManager;
-import org.apache.logging.log4j.Logger;
-import org.opensearch.common.collect.Tuple;
 import org.opensearch.core.xcontent.XContentLocation;
 import org.opensearch.core.xcontent.XContentParser;
 
 import java.io.IOException;
 
-import static org.hamcrest.Matchers.instanceOf;
-import static org.hamcrest.Matchers.lessThan;
-import static org.junit.Assert.assertThat;
-import static org.junit.Assert.fail;
-
 /**
  * Represents a lt assert section:
  * <p>
  *  - lt:    { fields._ttl: 20000}
  *
  */
-public class LessThanAssertion extends Assertion {
+public class LessThanAssertion extends OrderingAssertion {
     public static LessThanAssertion parse(XContentParser parser) throws IOException {
-        XContentLocation location = parser.getTokenLocation();
-        Tuple<String, Object> stringObjectTuple = ParserUtils.parseTuple(parser);
-        if (false == stringObjectTuple.v2() instanceof Comparable) {
-            throw new IllegalArgumentException(
-                "lt section can only be used with objects that support natural ordering, found "
-                    + stringObjectTuple.v2().getClass().getSimpleName()
-            );
-        }
-        return new LessThanAssertion(location, stringObjectTuple.v1(), stringObjectTuple.v2());
+        return parseOrderingAssertion(parser, Relation.LT, LessThanAssertion::new);
     }
 
-    private static final Logger logger = LogManager.getLogger(LessThanAssertion.class);
-
-    public LessThanAssertion(XContentLocation location, String field, Object expectedValue) {
-        super(location, field, expectedValue);
+    public LessThanAssertion(XContentLocation loc, String field, Object expected) {
+        super(loc, field, expected);
     }
 
     @Override
-    protected void doAssert(Object actualValue, Object expectedValue) {
-        logger.trace("assert that [{}] is less than [{}] (field: [{}])", actualValue, expectedValue, getField());
-        actualValue = convertActualValue(actualValue, expectedValue);
-        assertThat(
-            "value of [" + getField() + "] is not comparable (got [" + safeClass(actualValue) + "])",
-            actualValue,
-            instanceOf(Comparable.class)
-        );
-        assertThat(
-            "expected value of [" + getField() + "] is not comparable (got [" + expectedValue.getClass() + "])",
-            expectedValue,
-            instanceOf(Comparable.class)
-        );
-        try {
-            assertThat(errorMessage(), (Comparable) actualValue, lessThan((Comparable) expectedValue));
-        } catch (ClassCastException e) {
-            fail("cast error while checking (" + errorMessage() + "): " + e);
-        }
+    protected Relation relation() {
+        return Relation.LT;
     }
 
-    private String errorMessage() {
+    @Override
+    protected String errorMessage() {
         return "field [" + getField() + "] is not less than [" + getExpectedValue() + "]";
     }
 }

test/framework/src/main/java/org/opensearch/test/rest/yaml/section/LessThanOrEqualToAssertion.java

@@ -32,65 +32,32 @@
 
 package org.opensearch.test.rest.yaml.section;
 
-import org.apache.logging.log4j.LogManager;
-import org.apache.logging.log4j.Logger;
-import org.opensearch.common.collect.Tuple;
 import org.opensearch.core.xcontent.XContentLocation;
 import org.opensearch.core.xcontent.XContentParser;
 
 import java.io.IOException;
 
-import static org.hamcrest.Matchers.instanceOf;
-import static org.hamcrest.Matchers.lessThanOrEqualTo;
-import static org.junit.Assert.assertThat;
-import static org.junit.Assert.fail;
-
 /**
  * Represents a lte assert section:
  * <p>
  *   - lte:     { fields._ttl: 0 }
  */
-public class LessThanOrEqualToAssertion extends Assertion {
+public class LessThanOrEqualToAssertion extends OrderingAssertion {
     public static LessThanOrEqualToAssertion parse(XContentParser parser) throws IOException {
-        XContentLocation location = parser.getTokenLocation();
-        Tuple<String, Object> stringObjectTuple = ParserUtils.parseTuple(parser);
-        if (false == stringObjectTuple.v2() instanceof Comparable) {
-            throw new IllegalArgumentException(
-                "lte section can only be used with objects that support natural ordering, found "
-                    + stringObjectTuple.v2().getClass().getSimpleName()
-            );
-        }
-        return new LessThanOrEqualToAssertion(location, stringObjectTuple.v1(), stringObjectTuple.v2());
+        return parseOrderingAssertion(parser, Relation.LTE, LessThanOrEqualToAssertion::new);
     }
 
-    private static final Logger logger = LogManager.getLogger(LessThanOrEqualToAssertion.class);
-
-    public LessThanOrEqualToAssertion(XContentLocation location, String field, Object expectedValue) {
-        super(location, field, expectedValue);
+    public LessThanOrEqualToAssertion(XContentLocation loc, String field, Object expected) {
+        super(loc, field, expected);
     }
 
     @Override
-    protected void doAssert(Object actualValue, Object expectedValue) {
-        logger.trace("assert that [{}] is less than or equal to [{}] (field: [{}])", actualValue, expectedValue, getField());
-        actualValue = convertActualValue(actualValue, expectedValue);
-        assertThat(
-            "value of [" + getField() + "] is not comparable (got [" + safeClass(actualValue) + "])",
-            actualValue,
-            instanceOf(Comparable.class)
-        );
-        assertThat(
-            "expected value of [" + getField() + "] is not comparable (got [" + expectedValue.getClass() + "])",
-            expectedValue,
-            instanceOf(Comparable.class)
-        );
-        try {
-            assertThat(errorMessage(), (Comparable) actualValue, lessThanOrEqualTo((Comparable) expectedValue));
-        } catch (ClassCastException e) {
-            fail("cast error while checking (" + errorMessage() + "): " + e);
-        }
+    protected Relation relation() {
+        return Relation.LTE;
     }
 
-    private String errorMessage() {
+    @Override
+    protected String errorMessage() {
         return "field [" + getField() + "] is not less than or equal to [" + getExpectedValue() + "]";
     }
 }