This is an automated email from the ASF dual-hosted git repository.
chaokunyang pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/fury.git
The following commit(s) were added to refs/heads/main by this push:
new 9be8c958 fix(java): Fix flakiness in
ExpressionVisitorTest#testTraverseExpression (#1968)
9be8c958 is described below
commit 9be8c958c7db0f8f3db017cfa734ea60ff36c3a0
Author: Amit Prasad <[email protected]>
AuthorDate: Thu Dec 12 20:51:49 2024 -0600
fix(java): Fix flakiness in ExpressionVisitorTest#testTraverseExpression
(#1968)
## What does this PR do?
`ExpressionVisitorTest#testTraverseExpression` relies on ordered
traversal. However, the traversal depends on the order of
`getDeclaredFields()` to be deterministic (see below). The Java
specification explicitly marks `getDeclaredFields` as a function that
can return field names in any order. In the wild, this is most likely to
manifest on different architectures with different JVM versions.
https://github.com/apache/fury/blob/54b62fb6ab5d7e557131efe07c7402c885f6e7c4/java/fury-core/src/main/java/org/apache/fury/reflect/ReflectionUtils.java#L368-L381
Using [NonDex](https://github.com/TestingResearchIllinois/NonDex), we
can replicate the flaky behavior with the following command:
```
mvn edu.illinois:nondex-maven-plugin:2.1.7:nondex -pl fury-core/
-Dcheckstyle.skip -Dmaven.javadoc.skip
-Dtest=org.apache.fury.codegen.ExpressionVisitorTest
```
The fix, in this case, is to simply use HashSet equality instead of an
ordered List equality. The above NonDex command should succeed after
applying this patch.
I do, however, note that there are other flaky tests (found by running
NonDex on the entire `fury-core` project) that fail due to reliance on
e.g. `PriorityQueue#toArray`, which is also explicity marked to return
nondeterministically ordered arrays.
## Does this PR introduce any user-facing change?
No
---------
Co-authored-by: Shawn Yang <[email protected]>
---
.../java/org/apache/fury/codegen/ExpressionVisitorTest.java | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git
a/java/fury-core/src/test/java/org/apache/fury/codegen/ExpressionVisitorTest.java
b/java/fury-core/src/test/java/org/apache/fury/codegen/ExpressionVisitorTest.java
index 5404aba3..3d6e9ef4 100644
---
a/java/fury-core/src/test/java/org/apache/fury/codegen/ExpressionVisitorTest.java
+++
b/java/fury-core/src/test/java/org/apache/fury/codegen/ExpressionVisitorTest.java
@@ -26,7 +26,9 @@ import java.lang.reflect.InvocationTargetException;
import java.lang.reflect.Method;
import java.util.ArrayList;
import java.util.Arrays;
+import java.util.HashSet;
import java.util.List;
+import java.util.Set;
import org.apache.fury.codegen.Expression.Literal;
import org.apache.fury.reflect.ReflectionUtils;
import org.apache.fury.reflect.TypeRef;
@@ -63,7 +65,11 @@ public class ExpressionVisitorTest {
assertEquals(serializedLambda.getCapturedArgCount(), 1);
ExpressionVisitor.ExprHolder exprHolder =
(ExpressionVisitor.ExprHolder) (serializedLambda.getCapturedArg(0));
- assertEquals(
- expressions, Arrays.asList(forLoop, start, end, step, e1, ref,
exprHolder.get("e2")));
+
+ // Traversal relies on getDeclaredFields(), nondeterministic order.
+ Set<Expression> expressionsSet = new HashSet<>(expressions);
+ Set<Expression> expressionsSet2 =
+ new HashSet<>(Arrays.asList(forLoop, e1, ref, exprHolder.get("e2"),
end, start, step));
+ assertEquals(expressionsSet, expressionsSet2);
}
}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
