Hussain Towaileb has uploaded a new change for review.
https://asterix-gerrit.ics.uci.edu/3009
Change subject: [ASTERIXDB-2469][TEST] Tests Can Result In False Positives For
Numericals
......................................................................
[ASTERIXDB-2469][TEST] Tests Can Result In False Positives For Numericals
- user model changes: no
- storage format changes: no
- interface changes: no
Details:
- The test framework doesn’t differentiate between numeric types in
expected results.
- With the current behavior, when comparing numerical cases,
as a last step, we try to convert both numbers to double and
do a comparison (in case the String camparison failed already),
this is good for the cases of having something like: expected 100.0
and acutal 10E1 (for whatever reason), in this case the String comparison
will fail, but the double conversion will produce the correct result.
- With this change, we ensure the above behavior is maintained,
however, we check that the types are compatible first, for example,
if the expected is 100.0 and the actual is 100.0, this should succeed,
but if the expected is 100.0 and the actual is 100, then this will fail,
this way we ensure correctness of both the numerical value as well as
correctness of data type check.
Change-Id: I918b7e5c3c39271f77a7d5a01ff634c2a0221ebc
---
M
asterixdb/asterix-app/src/test/java/org/apache/asterix/test/common/TestExecutor.java
A
asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/misc/ensure_result_numeric_type/ensure_result_numeric_type.3.query.sqlpp
A
asterixdb/asterix-app/src/test/resources/runtimets/results/misc/ensure_result_numeric_type/ensure_result_numeric_type.1.adm
M asterixdb/asterix-app/src/test/resources/runtimets/testsuite_sqlpp.xml
4 files changed, 82 insertions(+), 13 deletions(-)
git pull ssh://asterix-gerrit.ics.uci.edu:29418/asterixdb
refs/changes/09/3009/1
diff --git
a/asterixdb/asterix-app/src/test/java/org/apache/asterix/test/common/TestExecutor.java
b/asterixdb/asterix-app/src/test/java/org/apache/asterix/test/common/TestExecutor.java
index 98cd668..9f9b334 100644
---
a/asterixdb/asterix-app/src/test/java/org/apache/asterix/test/common/TestExecutor.java
+++
b/asterixdb/asterix-app/src/test/java/org/apache/asterix/test/common/TestExecutor.java
@@ -54,6 +54,8 @@
import java.util.concurrent.Semaphore;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.TimeoutException;
+import java.util.function.BiFunction;
+import java.util.function.Function;
import java.util.function.Predicate;
import java.util.regex.Matcher;
import java.util.regex.Pattern;
@@ -373,25 +375,54 @@
}
return false;
} else {
- // If the fields are floating-point numbers, test them
- // for equality safely
+ // Ensures the values to be compared are of matching types
(2 Ints or 2 Floats, but not a mix)
+ BiFunction<String, String, Boolean>
isCompatibleTypesComparator = (arg1, arg2) -> {
+ Function<String, Boolean> isIntegerStringComparator =
(arg) -> {
+ try {
+ Integer.parseInt(arg);
+ } catch (NumberFormatException ignored) {
+ return false; // String is not valid Int
+ }
+ return true;
+ };
+
+ // Step 1: Check that both are valid double numbers
first
+ try {
+ Double.parseDouble(arg1);
+ Double.parseDouble(arg2);
+ } catch (NumberFormatException ignored) {
+ return false; // One of the Strings isn't a valid
number.
+ }
+
+ // Step 2: If one of the values is Int and the other
isn't, then matching will give false
+ // positive, if both are Int or both are Floats, then
it's a safe equality
+ return isIntegerStringComparator.apply(arg1) ==
isIntegerStringComparator.apply(arg2);
+ };
+
+ // Get the String values
expectedFields[j] = expectedFields[j].split(",")[0];
actualFields[j] = actualFields[j].split(",")[0];
- try {
- Double double1 = Double.parseDouble(expectedFields[j]);
- Double double2 = Double.parseDouble(actualFields[j]);
- float float1 = (float) double1.doubleValue();
- float float2 = (float) double2.doubleValue();
- if (Math.abs(float1 - float2) == 0) {
- continue;
- } else {
+ // Ensure compatibility before conversion
+ if (isCompatibleTypesComparator.apply(expectedFields[j],
actualFields[j])) {
+ try {
+ Double double1 =
Double.parseDouble(expectedFields[j]);
+ Double double2 =
Double.parseDouble(actualFields[j]);
+ float float1 = (float) double1.doubleValue();
+ float float2 = (float) double2.doubleValue();
+
+ if (Math.abs(float1 - float2) == 0) {
+ continue;
+ } else {
+ return false;
+ }
+ } catch (NumberFormatException ignored) {
+ // Guess they weren't numbers - must simply not be
equal
return false;
}
- } catch (NumberFormatException ignored) {
- // Guess they weren't numbers - must simply not be
equal
- return false;
}
+
+ return false; // Not equal
}
}
}
diff --git
a/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/misc/ensure_result_numeric_type/ensure_result_numeric_type.3.query.sqlpp
b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/misc/ensure_result_numeric_type/ensure_result_numeric_type.3.query.sqlpp
new file mode 100644
index 0000000..3933e66
--- /dev/null
+++
b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/misc/ensure_result_numeric_type/ensure_result_numeric_type.3.query.sqlpp
@@ -0,0 +1,30 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF 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.
+ */
+
+ /*
+ * This test is expected to return a ComparisonException
+ * because the actual returned value is 3 (int) but the expected
+ * value is 3.0 (double)
+ */
+
+
+select element strict_count((
+ select element x
+ from [1,2,3] as x
+));
diff --git
a/asterixdb/asterix-app/src/test/resources/runtimets/results/misc/ensure_result_numeric_type/ensure_result_numeric_type.1.adm
b/asterixdb/asterix-app/src/test/resources/runtimets/results/misc/ensure_result_numeric_type/ensure_result_numeric_type.1.adm
new file mode 100644
index 0000000..9f55b2c
--- /dev/null
+++
b/asterixdb/asterix-app/src/test/resources/runtimets/results/misc/ensure_result_numeric_type/ensure_result_numeric_type.1.adm
@@ -0,0 +1 @@
+3.0
diff --git
a/asterixdb/asterix-app/src/test/resources/runtimets/testsuite_sqlpp.xml
b/asterixdb/asterix-app/src/test/resources/runtimets/testsuite_sqlpp.xml
index 1a9c7dd..3fc5be2 100644
--- a/asterixdb/asterix-app/src/test/resources/runtimets/testsuite_sqlpp.xml
+++ b/asterixdb/asterix-app/src/test/resources/runtimets/testsuite_sqlpp.xml
@@ -4582,6 +4582,13 @@
</compilation-unit>
</test-case>
<test-case FilePath="misc">
+ <compilation-unit name="ensure_result_numeric_type">
+ <output-dir compare="Text">ensure_result_numeric_type</output-dir>
+ <source-location>false</source-location>
+ <expected-error>expected < 3.0</expected-error>
+ </compilation-unit>
+ </test-case>
+ <test-case FilePath="misc">
<compilation-unit name="partition-by-nonexistent-field">
<output-dir compare="Text">partition-by-nonexistent-field</output-dir>
<expected-error>Field "id" is not found</expected-error>
--
To view, visit https://asterix-gerrit.ics.uci.edu/3009
To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I918b7e5c3c39271f77a7d5a01ff634c2a0221ebc
Gerrit-PatchSet: 1
Gerrit-Project: asterixdb
Gerrit-Branch: master
Gerrit-Owner: Hussain Towaileb <[email protected]>
