FasterXML / jackson-datatypes-collections

Jackson project that contains various collection-oriented datatype libraries: Eclipse Collections, Guava, HPPC, PCollections
Apache License 2.0
79 stars 53 forks source link

fix two flaky tests #93

Closed arianacai1997 closed 2 years ago

arianacai1997 commented 3 years ago

There are two flaky tests. They are because of the order of twin and samplePair. By running nondex, the order of one and two is nondeterministic. Therefore, I fixed them by adding another possibility

yawkat commented 3 years ago

Why do you think this test is flaky? The serialization order for Pair should be deterministic. If it's not, that seems like a bug.

arianacai1997 commented 3 years ago

When I ran nondex, I got information like \ DeserializerTest.primitivePairs:659 expected:<{"[one":"f","two":"e]"}> but was:<{"[two":"e","one":"f]"}> \ DeserializerTest.twin:673 expected:<{"[one":"n","two":"h]"}> but was:<{"[two":"h","one":"n]"}>\ By looking at the debug file, it seems that mapper is an ObjectMapper with nondeterministic order.\ Here is a part of the traceback"":

java.lang.Class.getDeclaredFields(Class.java:1916)
com.fasterxml.jackson.databind.introspect.AnnotatedFieldCollector._findFields(AnnotatedFieldCollector.java:73)
com.fasterxml.jackson.databind.introspect.AnnotatedFieldCollector._findFields(AnnotatedFieldCollector.java:71)
com.fasterxml.jackson.databind.introspect.AnnotatedFieldCollector.collect(AnnotatedFieldCollector.java:48)
com.fasterxml.jackson.databind.introspect.AnnotatedFieldCollector.collectFields(AnnotatedFieldCollector.java:43)
com.fasterxml.jackson.databind.introspect.AnnotatedClass._fields(AnnotatedClass.java:371)
com.fasterxml.jackson.databind.introspect.AnnotatedClass.fields(AnnotatedClass.java:343)
com.fasterxml.jackson.databind.introspect.POJOPropertiesCollector._addFields(POJOPropertiesCollector.java:493)
com.fasterxml.jackson.databind.introspect.POJOPropertiesCollector.collectAll(POJOPropertiesCollector.java:421)
com.fasterxml.jackson.databind.introspect.POJOPropertiesCollector.getJsonValueAccessor(POJOPropertiesCollector.java:270)
com.fasterxml.jackson.databind.introspect.BasicBeanDescription.findJsonValueAccessor(BasicBeanDescription.java:258)
com.fasterxml.jackson.databind.ser.BasicSerializerFactory.findSerializerByAnnotations(BasicSerializerFactory.java:391)
com.fasterxml.jackson.databind.ser.BeanSerializerFactory._createSerializer2(BeanSerializerFactory.java:224)
com.fasterxml.jackson.databind.ser.BeanSerializerFactory.createSerializer(BeanSerializerFactory.java:173)
com.fasterxml.jackson.databind.SerializerProvider._createUntypedSerializer(SerializerProvider.java:1495)
com.fasterxml.jackson.databind.SerializerProvider._createAndCacheUntypedSerializer(SerializerProvider.java:1443)
com.fasterxml.jackson.databind.SerializerProvider.findValueSerializer(SerializerProvider.java:544)
com.fasterxml.jackson.databind.SerializerProvider.findTypedValueSerializer(SerializerProvider.java:822)
com.fasterxml.jackson.databind.ser.DefaultSerializerProvider.serializeValue(DefaultSerializerProvider.java:308)
com.fasterxml.jackson.databind.ObjectMapper._writeValueAndClose(ObjectMapper.java:4568)
com.fasterxml.jackson.databind.ObjectMapper.writeValueAsString(ObjectMapper.java:3821)
com.fasterxml.jackson.datatype.eclipsecollections.DeserializerTest.twin(DeserializerTest.java:673)
yawkat commented 3 years ago

Do you have a setup where this reproduces without nondex, whatever that is? Because the test cases aren't flaky for me locally.

arianacai1997 commented 3 years ago

Do you have a setup where this reproduces without nondex, whatever that is? Because the test cases aren't flaky for me locally. I used these commands. But you might need to adapt the server to your VM.

mvn install -pl eclipse-collections -am -DskipTests | tee mvn-install.log
mvn -pl eclipse-collections test -Dtest=com.fasterxml.jackson.datatype.eclipsecollections.DeserializerTest#primitivePairs
mvn -pl eclipse-collections edu.illinois:nondex-maven-plugin:1.1.2:nondex -DnondexRuns=10 -Dtest=com.fasterxml.jackson.datatype.eclipsecollections.DeserializerTest#primitivePairs
yawkat commented 3 years ago

Look I get that it's erroring with nondex, but is it actually breaking in the real world? Outside of nondex?

arianacai1997 commented 3 years ago

Outside

Sorry I didn't get what you mean. Nondex is a tool for order-related flaky test detection. Maybe you can refer to the GitHub for installation. https://github.com/TestingResearchIllinois/NonDex.

yawkat commented 3 years ago

Look. I understand that nondex reports this as an issue. However I will not consider this a bug unless you can show a case where the test fails without nondex being involved.

If I understand the issue correctly, then I wouldn't rule it out that this can happen in the real world. But I will not accept a bug that apparently exclusively appears in a runtime that was specifically designed to trigger that bug, and will never happen in the real world.

arianacai1997 commented 2 years ago

Thanks for the explanation. I'll look at the issue and update it accordingly.

cowtowncoder commented 2 years ago

There seems to be a class of reported potential (but not necessarily actual, although could be) issues. I'll close this until such time as there's something to actually reproduce an issue, or at least verify fix we might try.