Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix SetOfSuperFloatPropertyParameterTest for Super classes of Type Float #521

Merged
merged 1 commit into from
Oct 28, 2023

Conversation

219sansim
Copy link
Contributor

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
Copy link
Owner

pholser commented Oct 26, 2023

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
Copy link
Contributor Author

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:

Object[] array = items.toArray(new Object[0]);
return (T) array[random.nextInt(array.length)];
, I observe that items.toArray(new Object[0]) relies on the fact that the base type (value with variable bottom -
public static Set<Type<?>> supertypes(Type<?> bottom) {
Set<Type<?>> supertypes = new HashSet<>();
supertypes.add(bottom);
supertypes.addAll(bottom.getAllTypesAssignableFromThis());
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
Copy link
Owner

pholser commented Oct 28, 2023

LGTM. Thanks for looking into the flaky test.

@pholser pholser merged commit 7242993 into pholser:master Oct 28, 2023
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants