pholser / junit-quickcheck

Property-based testing, JUnit-style
MIT License
954 stars 121 forks source link

Fix SetOfSuperFloatPropertyParameterTest for Super classes of Type Float #521

Closed 219sansim closed 10 months ago

219sansim commented 10 months ago

Overview: There is an error detected in the test com.pholser.junit.quickcheck.generator.java.util.SetOfSuperFloatPropertyParameterTest.verifyInteractionWithRandomness

The wildcard ? super Float helps to generate all Types which Float is a Superclass of, however, in this context, we require to generate all Types which are Superclass of Float (like Integer, Decimal).

The fix:

Use the wildcard ? extends Float instead of ? super Float. All test cases pass after making the change.

Reference : Stackoverflow question

Steps to detect and reproduce the error: On running the command

mvn -pl generators test -Dtest=com.pholser.junit.quickcheck.generator.java.util.SetOfSuperFloatPropertyParameterTest#verifyInteractionWithRandomness

the test passes However, when I run the command with NonDex tool

mvn -pl generators edu.illinois:nondex-maven-plugin:2.1.1:nondex -Dtest=com.pholser.junit.quickcheck.generator.java.util.SetOfSuperFloatPropertyParameterTest#verifyInteractionWithRandomness

I get the following errors

[ERROR]   SetOfSuperFloatPropertyParameterTest.verifyInteractionWithRandomness:78 
Wanted but not invoked:
randomForParameterGenerator.nextFloat(
    0.0f,
    1.0f
);
-> at com.pholser.junit.quickcheck.Generating.verifyFloats(Generating.java:102)

However, there were exactly 3 interactions with this mock:
randomForParameterGenerator.nextByte(
    (byte) 0x80,
    (byte) 0x7F
);
-> at com.pholser.junit.quickcheck.generator.java.lang.ByteGenerator.generate(ByteGenerator.java:76)

randomForParameterGenerator.nextByte(
    (byte) 0x80,
    (byte) 0x7F
);
-> at com.pholser.junit.quickcheck.generator.java.lang.ByteGenerator.generate(ByteGenerator.java:76)

randomForParameterGenerator.nextByte(
    (byte) 0x80,
    (byte) 0x7F
);
-> at com.pholser.junit.quickcheck.generator.java.lang.ByteGenerator.generate(ByteGenerator.java:76)
[ERROR]   SetOfSuperFloatPropertyParameterTest.verifyInteractionWithRandomness:78 
Wanted but not invoked:
randomForParameterGenerator.nextFloat(
    0.0f,
    1.0f
);
-> at com.pholser.junit.quickcheck.Generating.verifyFloats(Generating.java:102)

However, there were exactly 3 interactions with this mock:
randomForParameterGenerator.nextBoolean();
-> at com.pholser.junit.quickcheck.generator.java.lang.BooleanGenerator.generate(BooleanGenerator.java:52)

randomForParameterGenerator.nextBoolean();
-> at com.pholser.junit.quickcheck.generator.java.lang.BooleanGenerator.generate(BooleanGenerator.java:52)

randomForParameterGenerator.nextBoolean();
-> at com.pholser.junit.quickcheck.generator.java.lang.BooleanGenerator.generate(BooleanGenerator.java:52)
pholser commented 10 months ago

Thanks for this.

I would correct your statement above, though: ? super Float means "some definite type that is either Float or some type that a variable of type Floatcould be assigned to (e.g. Number, 'Object, 'Serializable). ? extends Float means "some definitely type that is either Float or a type whose values can be assigned to a variable of type Float (in this case, there are no such types other than Float itself).

Now, the test as written was intended to produce lists of either Float or some other supertype of Float that we have a generator for. It turns out that there are no such other generators, and so the expectation is that we have some list type (array list, linked list, whatever) of element type Float. The fact that it's triggering Byte generations for you is troublesome.

Does NonDex instrument bytecode in any particular way? Could it be doing so in a way that affects random generation?

I don't think changing the test in the way you did is the correct course of action -- the wildcard type matches the intent of the test. We'd need to understand why under NonDex the expectations of the test aren't met.

219sansim commented 10 months ago

Thanks for you clarification about wildcard ? super Float and I have fixed the error correctly this time. I dug down deeper to see why the NonDex tool is triggering Byte generations.

In this section of code: https://github.com/pholser/junit-quickcheck/blob/bcec1ae07d45e731c681c4b0b6884794b5898316/core/src/main/java/com/pholser/junit/quickcheck/internal/Items.java#L57-L58, I observe that items.toArray(new Object[0]) relies on the fact that the base type (value with variable bottom - https://github.com/pholser/junit-quickcheck/blob/bcec1ae07d45e731c681c4b0b6884794b5898316/core/src/main/java/com/pholser/junit/quickcheck/internal/Reflection.java#L137-L140 is always the first index of HashSet items for type-defining purpose.

This is why the Byte generations were being trigerred, since NonDex was changing the first element of the supertypes HashSet to one of Float's Super Types (java.lang.Object, java.lang.Number, java.io.Serializable, java.lang.Comparable<java.lang.Float>

NonDex simulates the behaviour of the 'non constant order over time' of HashSet to detect flaky tests. According to HashSet documentation : HashSet 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 fix: The solution is to change the data structure for supertypes from HashSet to a LinkedHashSet so that the order of elements of the supertypes HashSet do not change and test is not flaky.

After making the change from HashSet to LinkedHashSet, the test passes with the NonDex tool.

pholser commented 10 months ago

LGTM. Thanks for looking into the flaky test.