This is an automated email from the ASF dual-hosted git repository. chaokunyang pushed a commit to branch releases-0.10 in repository https://gitbox.apache.org/repos/asf/fury.git
commit 8a8bf1e1cc90a196d6917bdd72671ae34c271092 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]
