Skip to content

Commit ba9ec47

Browse files
laminelamLamine Idjeraoui
laminelam
and
Lamine Idjeraoui
authored
Fix bug "Assertion framework(Yaml Rest test): numeric comparison fails when comparing Integer vs Long (or Float vs Double)" (opensearch-project#19376)
* Fix bug: Assertion framework (Yaml Rest test): numeric comparison fails when comparing Integer vs Long (or Float vs Double) Signed-off-by: Lamine Idjeraoui <lidjeraoui@apple.com> * class rename Signed-off-by: Lamine Idjeraoui <lidjeraoui@apple.com> * some small changes based on the PR review Signed-off-by: Lamine Idjeraoui <lidjeraoui@apple.com> * update change log Signed-off-by: Lamine Idjeraoui <lidjeraoui@apple.com> --------- Signed-off-by: Lamine Idjeraoui <lidjeraoui@apple.com> Co-authored-by: Lamine Idjeraoui <lidjeraoui@apple.com>
1 parent 1c84b67 commit ba9ec47

File tree

8 files changed

+210
-165
lines changed

8 files changed

+210
-165
lines changed

‎CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
9797
- Fix NPE of ScriptScoreQuery ([#19650](https://github.com/opensearch-project/OpenSearch/pull/19650))
9898
- Fix ClassCastException in FlightClientChannel for requests larger than 16KB ([#20010](https://github.com/opensearch-project/OpenSearch/pull/20010))
9999
- Fix GRPC Bulk ([#19937](https://github.com/opensearch-project/OpenSearch/pull/19937))
100+
- 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))
100101
- Fix node bootstrap error when enable stream transport and remote cluster state ([#19948](https://github.com/opensearch-project/OpenSearch/pull/19948))
101102
- Keep track and release Reactor Netty 4 Transport accepted Http Channels during the Node shutdown ([#20106](https://github.com/opensearch-project/OpenSearch/pull/20106))
102103
- 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‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,11 @@ static Object convertActualValue(Object actualValue, Object expectedValue) {
9191
} else if (expectedValue instanceof Double) {
9292
return Double.parseDouble(actualValue.toString());
9393
} else if (expectedValue instanceof Integer) {
94-
return Integer.parseInt(actualValue.toString());
94+
try {
95+
return Long.parseLong(actualValue.toString());
96+
} catch (NumberFormatException e) {
97+
return Integer.parseInt(actualValue.toString());
98+
}
9599
} else if (expectedValue instanceof Long) {
96100
return Long.parseLong(actualValue.toString());
97101
}

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

Lines changed: 8 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -31,65 +31,32 @@
3131

3232
package org.opensearch.test.rest.yaml.section;
3333

34-
import org.apache.logging.log4j.LogManager;
35-
import org.apache.logging.log4j.Logger;
36-
import org.opensearch.common.collect.Tuple;
3734
import org.opensearch.core.xcontent.XContentLocation;
3835
import org.opensearch.core.xcontent.XContentParser;
3936

4037
import java.io.IOException;
4138

42-
import static org.hamcrest.Matchers.greaterThan;
43-
import static org.hamcrest.Matchers.instanceOf;
44-
import static org.junit.Assert.assertThat;
45-
import static org.junit.Assert.fail;
46-
4739
/**
4840
* Represents a gt assert section:
4941
* <p>
5042
* - gt: { fields._ttl: 0}
5143
*/
52-
public class GreaterThanAssertion extends Assertion {
44+
public class GreaterThanAssertion extends OrderingAssertion {
5345
public static GreaterThanAssertion parse(XContentParser parser) throws IOException {
54-
XContentLocation location = parser.getTokenLocation();
55-
Tuple<String, Object> stringObjectTuple = ParserUtils.parseTuple(parser);
56-
if (!(stringObjectTuple.v2() instanceof Comparable)) {
57-
throw new IllegalArgumentException(
58-
"gt section can only be used with objects that support natural ordering, found "
59-
+ stringObjectTuple.v2().getClass().getSimpleName()
60-
);
61-
}
62-
return new GreaterThanAssertion(location, stringObjectTuple.v1(), stringObjectTuple.v2());
46+
return parseOrderingAssertion(parser, Relation.GT, GreaterThanAssertion::new);
6347
}
6448

65-
private static final Logger logger = LogManager.getLogger(GreaterThanAssertion.class);
66-
67-
public GreaterThanAssertion(XContentLocation location, String field, Object expectedValue) {
68-
super(location, field, expectedValue);
49+
public GreaterThanAssertion(XContentLocation loc, String field, Object expected) {
50+
super(loc, field, expected);
6951
}
7052

7153
@Override
72-
protected void doAssert(Object actualValue, Object expectedValue) {
73-
logger.trace("assert that [{}] is greater than [{}] (field: [{}])", actualValue, expectedValue, getField());
74-
actualValue = convertActualValue(actualValue, expectedValue);
75-
assertThat(
76-
"value of [" + getField() + "] is not comparable (got [" + safeClass(actualValue) + "])",
77-
actualValue,
78-
instanceOf(Comparable.class)
79-
);
80-
assertThat(
81-
"expected value of [" + getField() + "] is not comparable (got [" + expectedValue.getClass() + "])",
82-
expectedValue,
83-
instanceOf(Comparable.class)
84-
);
85-
try {
86-
assertThat(errorMessage(), (Comparable) actualValue, greaterThan((Comparable) expectedValue));
87-
} catch (ClassCastException e) {
88-
fail("cast error while checking (" + errorMessage() + "): " + e);
89-
}
54+
protected Relation relation() {
55+
return Relation.GT;
9056
}
9157

92-
private String errorMessage() {
58+
@Override
59+
protected String errorMessage() {
9360
return "field [" + getField() + "] is not greater than [" + getExpectedValue() + "]";
9461
}
9562
}

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

Lines changed: 8 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -32,65 +32,32 @@
3232

3333
package org.opensearch.test.rest.yaml.section;
3434

35-
import org.apache.logging.log4j.LogManager;
36-
import org.apache.logging.log4j.Logger;
37-
import org.opensearch.common.collect.Tuple;
3835
import org.opensearch.core.xcontent.XContentLocation;
3936
import org.opensearch.core.xcontent.XContentParser;
4037

4138
import java.io.IOException;
4239

43-
import static org.hamcrest.Matchers.greaterThanOrEqualTo;
44-
import static org.hamcrest.Matchers.instanceOf;
45-
import static org.junit.Assert.assertThat;
46-
import static org.junit.Assert.fail;
47-
4840
/**
4941
* Represents a gte assert section:
5042
* <p>
5143
* - gte: { fields._ttl: 0 }
5244
*/
53-
public class GreaterThanEqualToAssertion extends Assertion {
45+
public class GreaterThanEqualToAssertion extends OrderingAssertion {
5446
public static GreaterThanEqualToAssertion parse(XContentParser parser) throws IOException {
55-
XContentLocation location = parser.getTokenLocation();
56-
Tuple<String, Object> stringObjectTuple = ParserUtils.parseTuple(parser);
57-
if (!(stringObjectTuple.v2() instanceof Comparable)) {
58-
throw new IllegalArgumentException(
59-
"gte section can only be used with objects that support natural ordering, found "
60-
+ stringObjectTuple.v2().getClass().getSimpleName()
61-
);
62-
}
63-
return new GreaterThanEqualToAssertion(location, stringObjectTuple.v1(), stringObjectTuple.v2());
47+
return parseOrderingAssertion(parser, Relation.GTE, GreaterThanEqualToAssertion::new);
6448
}
6549

66-
private static final Logger logger = LogManager.getLogger(GreaterThanEqualToAssertion.class);
67-
68-
public GreaterThanEqualToAssertion(XContentLocation location, String field, Object expectedValue) {
69-
super(location, field, expectedValue);
50+
public GreaterThanEqualToAssertion(XContentLocation loc, String field, Object expected) {
51+
super(loc, field, expected);
7052
}
7153

7254
@Override
73-
protected void doAssert(Object actualValue, Object expectedValue) {
74-
logger.trace("assert that [{}] is greater than or equal to [{}] (field: [{}])", actualValue, expectedValue, getField());
75-
actualValue = convertActualValue(actualValue, expectedValue);
76-
assertThat(
77-
"value of [" + getField() + "] is not comparable (got [" + safeClass(actualValue) + "])",
78-
actualValue,
79-
instanceOf(Comparable.class)
80-
);
81-
assertThat(
82-
"expected value of [" + getField() + "] is not comparable (got [" + expectedValue.getClass() + "])",
83-
expectedValue,
84-
instanceOf(Comparable.class)
85-
);
86-
try {
87-
assertThat(errorMessage(), (Comparable) actualValue, greaterThanOrEqualTo((Comparable) expectedValue));
88-
} catch (ClassCastException e) {
89-
fail("cast error while checking (" + errorMessage() + "): " + e);
90-
}
55+
protected Relation relation() {
56+
return Relation.GTE;
9157
}
9258

93-
private String errorMessage() {
59+
@Override
60+
protected String errorMessage() {
9461
return "field [" + getField() + "] is not greater than or equal to [" + getExpectedValue() + "]";
9562
}
9663
}

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

Lines changed: 8 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -31,66 +31,33 @@
3131

3232
package org.opensearch.test.rest.yaml.section;
3333

34-
import org.apache.logging.log4j.LogManager;
35-
import org.apache.logging.log4j.Logger;
36-
import org.opensearch.common.collect.Tuple;
3734
import org.opensearch.core.xcontent.XContentLocation;
3835
import org.opensearch.core.xcontent.XContentParser;
3936

4037
import java.io.IOException;
4138

42-
import static org.hamcrest.Matchers.instanceOf;
43-
import static org.hamcrest.Matchers.lessThan;
44-
import static org.junit.Assert.assertThat;
45-
import static org.junit.Assert.fail;
46-
4739
/**
4840
* Represents a lt assert section:
4941
* <p>
5042
* - lt: { fields._ttl: 20000}
5143
*
5244
*/
53-
public class LessThanAssertion extends Assertion {
45+
public class LessThanAssertion extends OrderingAssertion {
5446
public static LessThanAssertion parse(XContentParser parser) throws IOException {
55-
XContentLocation location = parser.getTokenLocation();
56-
Tuple<String, Object> stringObjectTuple = ParserUtils.parseTuple(parser);
57-
if (false == stringObjectTuple.v2() instanceof Comparable) {
58-
throw new IllegalArgumentException(
59-
"lt section can only be used with objects that support natural ordering, found "
60-
+ stringObjectTuple.v2().getClass().getSimpleName()
61-
);
62-
}
63-
return new LessThanAssertion(location, stringObjectTuple.v1(), stringObjectTuple.v2());
47+
return parseOrderingAssertion(parser, Relation.LT, LessThanAssertion::new);
6448
}
6549

66-
private static final Logger logger = LogManager.getLogger(LessThanAssertion.class);
67-
68-
public LessThanAssertion(XContentLocation location, String field, Object expectedValue) {
69-
super(location, field, expectedValue);
50+
public LessThanAssertion(XContentLocation loc, String field, Object expected) {
51+
super(loc, field, expected);
7052
}
7153

7254
@Override
73-
protected void doAssert(Object actualValue, Object expectedValue) {
74-
logger.trace("assert that [{}] is less than [{}] (field: [{}])", actualValue, expectedValue, getField());
75-
actualValue = convertActualValue(actualValue, expectedValue);
76-
assertThat(
77-
"value of [" + getField() + "] is not comparable (got [" + safeClass(actualValue) + "])",
78-
actualValue,
79-
instanceOf(Comparable.class)
80-
);
81-
assertThat(
82-
"expected value of [" + getField() + "] is not comparable (got [" + expectedValue.getClass() + "])",
83-
expectedValue,
84-
instanceOf(Comparable.class)
85-
);
86-
try {
87-
assertThat(errorMessage(), (Comparable) actualValue, lessThan((Comparable) expectedValue));
88-
} catch (ClassCastException e) {
89-
fail("cast error while checking (" + errorMessage() + "): " + e);
90-
}
55+
protected Relation relation() {
56+
return Relation.LT;
9157
}
9258

93-
private String errorMessage() {
59+
@Override
60+
protected String errorMessage() {
9461
return "field [" + getField() + "] is not less than [" + getExpectedValue() + "]";
9562
}
9663
}

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

Lines changed: 8 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -32,65 +32,32 @@
3232

3333
package org.opensearch.test.rest.yaml.section;
3434

35-
import org.apache.logging.log4j.LogManager;
36-
import org.apache.logging.log4j.Logger;
37-
import org.opensearch.common.collect.Tuple;
3835
import org.opensearch.core.xcontent.XContentLocation;
3936
import org.opensearch.core.xcontent.XContentParser;
4037

4138
import java.io.IOException;
4239

43-
import static org.hamcrest.Matchers.instanceOf;
44-
import static org.hamcrest.Matchers.lessThanOrEqualTo;
45-
import static org.junit.Assert.assertThat;
46-
import static org.junit.Assert.fail;
47-
4840
/**
4941
* Represents a lte assert section:
5042
* <p>
5143
* - lte: { fields._ttl: 0 }
5244
*/
53-
public class LessThanOrEqualToAssertion extends Assertion {
45+
public class LessThanOrEqualToAssertion extends OrderingAssertion {
5446
public static LessThanOrEqualToAssertion parse(XContentParser parser) throws IOException {
55-
XContentLocation location = parser.getTokenLocation();
56-
Tuple<String, Object> stringObjectTuple = ParserUtils.parseTuple(parser);
57-
if (false == stringObjectTuple.v2() instanceof Comparable) {
58-
throw new IllegalArgumentException(
59-
"lte section can only be used with objects that support natural ordering, found "
60-
+ stringObjectTuple.v2().getClass().getSimpleName()
61-
);
62-
}
63-
return new LessThanOrEqualToAssertion(location, stringObjectTuple.v1(), stringObjectTuple.v2());
47+
return parseOrderingAssertion(parser, Relation.LTE, LessThanOrEqualToAssertion::new);
6448
}
6549

66-
private static final Logger logger = LogManager.getLogger(LessThanOrEqualToAssertion.class);
67-
68-
public LessThanOrEqualToAssertion(XContentLocation location, String field, Object expectedValue) {
69-
super(location, field, expectedValue);
50+
public LessThanOrEqualToAssertion(XContentLocation loc, String field, Object expected) {
51+
super(loc, field, expected);
7052
}
7153

7254
@Override
73-
protected void doAssert(Object actualValue, Object expectedValue) {
74-
logger.trace("assert that [{}] is less than or equal to [{}] (field: [{}])", actualValue, expectedValue, getField());
75-
actualValue = convertActualValue(actualValue, expectedValue);
76-
assertThat(
77-
"value of [" + getField() + "] is not comparable (got [" + safeClass(actualValue) + "])",
78-
actualValue,
79-
instanceOf(Comparable.class)
80-
);
81-
assertThat(
82-
"expected value of [" + getField() + "] is not comparable (got [" + expectedValue.getClass() + "])",
83-
expectedValue,
84-
instanceOf(Comparable.class)
85-
);
86-
try {
87-
assertThat(errorMessage(), (Comparable) actualValue, lessThanOrEqualTo((Comparable) expectedValue));
88-
} catch (ClassCastException e) {
89-
fail("cast error while checking (" + errorMessage() + "): " + e);
90-
}
55+
protected Relation relation() {
56+
return Relation.LTE;
9157
}
9258

93-
private String errorMessage() {
59+
@Override
60+
protected String errorMessage() {
9461
return "field [" + getField() + "] is not less than or equal to [" + getExpectedValue() + "]";
9562
}
9663
}

0 commit comments

Comments
 (0)