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/fory.git
The following commit(s) were added to refs/heads/main by this push:
new 4f2ec817c fix(java): Fix flakiness in
CollectionSerializersTest#testCollectionFieldSerializers,
CollectionSerializersTest#testCollectionFieldSerializersCopy and
ProtocolInteroperabilityTest#testComplexCollection (#2790)
4f2ec817c is described below
commit 4f2ec817c9efee2a7e8b1180c5f9300b0d3f8e41
Author: JACKDABOSS <[email protected]>
AuthorDate: Mon Oct 20 21:38:40 2025 -0500
fix(java): Fix flakiness in
CollectionSerializersTest#testCollectionFieldSerializers,
CollectionSerializersTest#testCollectionFieldSerializersCopy and
ProtocolInteroperabilityTest#testComplexCollection (#2790)
## What does this PR do?
These tests create a `CollectionFieldsClass` that contains HashSet and
PriorityQueue fields, which do not guarantee deterministic ordering. So
`toString()` comparisons could lead to non-deterministic failures.
Quotes from Java's summary of HashSet and PriorityQueue:
> It makes no guarantees as to the iteration order of the set; in
particular, it does not guarantee that the order will remain constant
over time.
> The Iterator provided in method
[iterator()](https://docs.oracle.com/javase/8/docs/api/java/util/PriorityQueue.html#iterator--)
is not guaranteed to traverse the elements of the priority queue in any
particular order. If you need ordered traversal, consider using
Arrays.sort(pq.toArray()).
We can reproduce the flakiness with
[NonDex](https://github.com/TestingResearchIllinois/NonDex):
```
mvn -pl fory-core edu.illinois:nondex-maven-plugin:2.1.7:nondex
-Dtest=org.apache.fory.serializer.collection.CollectionSerializersTest#testCollectionFieldSerializers
mvn -pl fory-core edu.illinois:nondex-maven-plugin:2.1.7:nondex
-Dtest=org.apache.fory.serializer.collection.CollectionSerializersTest#testCollectionFieldSerializersCopy
mvn -pl fory-core edu.illinois:nondex-maven-plugin:2.1.7:nondex
-Dtest=org.apache.fory.serializer.ProtocolInteroperabilityTest#testComplexCollection
```
Since `toString()` just dumps all the fields, we can solve the problem
by iteratively comparing fields with deterministic ordering, and
manually compare everything else after sorting.
In particular,
`CollectionSerializersTest.testCollectionFieldsObjectEqual` is made
public, so that `ProtocolInteroperabilityTest` can reuse the solution.
## Does this PR introduce any user-facing change?
No
---
.../serializer/ProtocolInteroperabilityTest.java | 14 +++++++-
.../collection/CollectionSerializersTest.java | 40 +++++++++++++++++++---
2 files changed, 49 insertions(+), 5 deletions(-)
diff --git
a/java/fory-core/src/test/java/org/apache/fory/serializer/ProtocolInteroperabilityTest.java
b/java/fory-core/src/test/java/org/apache/fory/serializer/ProtocolInteroperabilityTest.java
index 91c707a6e..6901fe450 100644
---
a/java/fory-core/src/test/java/org/apache/fory/serializer/ProtocolInteroperabilityTest.java
+++
b/java/fory-core/src/test/java/org/apache/fory/serializer/ProtocolInteroperabilityTest.java
@@ -217,7 +217,19 @@ public class ProtocolInteroperabilityTest extends
ForyTestBase {
public void testComplexCollection(Fory fory, Fory foryJIT) {
CollectionSerializersTest.CollectionFieldsClass o =
CollectionSerializersTest.createCollectionFieldsObject();
- roundCheck(fory, foryJIT, o, Object::toString);
+ roundCheck(
+ fory,
+ foryJIT,
+ o,
+ obj -> {
+ try {
+ CollectionSerializersTest.testCollectionFieldsObjectEqual(
+ (CollectionSerializersTest.CollectionFieldsClass) obj, o);
+ return "OK";
+ } catch (IllegalAccessException e) {
+ throw new RuntimeException(e);
+ }
+ });
}
@Test(dataProvider = "fory")
diff --git
a/java/fory-core/src/test/java/org/apache/fory/serializer/collection/CollectionSerializersTest.java
b/java/fory-core/src/test/java/org/apache/fory/serializer/collection/CollectionSerializersTest.java
index 731dbf859..fd517e22d 100644
---
a/java/fory-core/src/test/java/org/apache/fory/serializer/collection/CollectionSerializersTest.java
+++
b/java/fory-core/src/test/java/org/apache/fory/serializer/collection/CollectionSerializersTest.java
@@ -29,6 +29,7 @@ import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.ImmutableSortedSet;
import java.io.Serializable;
+import java.lang.reflect.Field;
import java.util.AbstractCollection;
import java.util.ArrayDeque;
import java.util.ArrayList;
@@ -662,10 +663,41 @@ public class CollectionSerializersTest extends
ForyTestBase {
return obj;
}
+ public static void testCollectionFieldsObjectEqual(
+ CollectionFieldsClass got, CollectionFieldsClass obj) throws
IllegalAccessException {
+ // Fields that can cause flakiness need to be compared separately
+ Set<String> special = ImmutableSet.of("hashSet", "hashSet2",
"priorityQueue", "priorityQueue2");
+ // Compare fields with deterministic ordering
+ for (Field f :
CollectionSerializersTest.CollectionFieldsClass.class.getDeclaredFields()) {
+ if (special.contains(f.getName())) continue;
+ f.setAccessible(true);
+ Object gv = f.get(got);
+ Object ov = f.get(obj);
+ Assert.assertEquals(String.valueOf(gv), String.valueOf(ov));
+ }
+ // Compare everything else
+ String hsGot = new TreeSet<>(got.hashSet).toString();
+ String hsObj = new TreeSet<>(obj.hashSet).toString();
+ Assert.assertEquals(hsGot, hsObj);
+ hsGot = new TreeSet<>(got.hashSet2).toString();
+ hsObj = new TreeSet<>(obj.hashSet2).toString();
+ Assert.assertEquals(hsGot, hsObj);
+ List<String> pqGot = new ArrayList<>(got.priorityQueue);
+ List<String> pqExp = new ArrayList<>(obj.priorityQueue);
+ Collections.sort(pqGot);
+ Collections.sort(pqExp);
+ Assert.assertEquals(pqGot.toString(), pqExp.toString());
+ pqGot = new ArrayList<>(got.priorityQueue2);
+ pqExp = new ArrayList<>(obj.priorityQueue2);
+ Collections.sort(pqGot);
+ Collections.sort(pqExp);
+ Assert.assertEquals(pqGot.toString(), pqExp.toString());
+ }
+
@Test(dataProvider = "javaFory")
- public void testCollectionFieldSerializers(Fory fory) {
+ public void testCollectionFieldSerializers(Fory fory) throws Exception {
CollectionFieldsClass obj = createCollectionFieldsObject();
- Assert.assertEquals(serDe(fory, obj).toString(), obj.toString());
+ testCollectionFieldsObjectEqual(serDe(fory, obj), obj);
if (fory.getConfig().isCodeGenEnabled()) {
Assert.assertTrue(
fory.getClassResolver()
@@ -676,9 +708,9 @@ public class CollectionSerializersTest extends ForyTestBase
{
}
@Test(dataProvider = "foryCopyConfig")
- public void testCollectionFieldSerializersCopy(Fory fory) {
+ public void testCollectionFieldSerializersCopy(Fory fory) throws Exception {
CollectionFieldsClass obj = createCollectionFieldsObject();
- Assert.assertEquals(fory.copy(obj).toString(), obj.toString());
+ testCollectionFieldsObjectEqual(fory.copy(obj), obj);
}
@Data
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
