review: test: Fixed nondeterministic failures in ContractOnSettersParametrizedTest.data()#6499
Conversation
…RT_REFERENCE and pattern setters
| @@ -220,6 +223,35 @@ private void testContract(CtType<?> toTest) throws Throwable { | |||
| int nBefore = changeListener.nbCallsToOnAction; | |||
| changeListener.changedElements = new ArrayList<>(); | |||
|
|
|||
There was a problem hiding this comment.
Could you add a fat comment of what you do here and why we need it?
It's unclear how this relates to the problem described in the PR description.
Thanks!
There was a problem hiding this comment.
Thank you for your review! The code below filters out invalid (receiver, method, argument) combinations before invoking setters via reflection in ContractOnSettersParametrizedTest. This test generates candidate combinations, and the order of these candidates depends on iteration over unordered collections. When NonDex randomizes iteration order, some seeds produce combinations that violate Spoon's API contracts, which leads to nondeterministic test failures.
For IMPORT_REFERENCE, the parameter type is a CtReference, but Spoon only accepts four specific subtypes: CtTypeReference, CtExecutableReference, CtFieldReference, and CtPackageReference. When createCompatibleObject() produced other CtReference subtypes, the reflective call triggered contract violations. The code below explicitly checks for these four allowed types and skip all other arguments.
Class<?> ctImport = Class.forName("spoon.reflect.declaration.CtImport");
if (ctImport.isInstance(receiver) && "setReference".equals(actualMethod.getName())) {
Class<?> typeRef = Class.forName("spoon.reflect.reference.CtTypeReference");
Class<?> execRef = Class.forName("spoon.reflect.reference.CtExecutableReference");
Class<?> fieldRef = Class.forName("spoon.reflect.reference.CtFieldReference");
Class<?> pkgRef = Class.forName("spoon.reflect.reference.CtPackageReference");
boolean ok = typeRef.isInstance(argument)
|| execRef.isInstance(argument)
|| fieldRef.isInstance(argument)
|| pkgRef.isInstance(argument);
if (!ok) continue;
}
For pattern arguments, the generator sometimes produces a CtUnnamedPattern as the value for this parameter. However, CtCasePattern cannot directly take a CtUnnamedPattern as its pattern. It is only valid when nested inside a record pattern. When this invalid combination appears under certain NonDex seeds, the reflective call fails. The code below therefore skips this specific combination.
Class<?> casePattern = Class.forName("spoon.reflect.code.CtCasePattern");
Class<?> unnamedPattern = Class.forName("spoon.reflect.code.CtUnnamedPattern");
if (casePattern.isInstance(receiver)
&& "setPattern".equals(actualMethod.getName())
&& unnamedPattern.isInstance(argument)) {
continue;
}
Please let me know if you need any more details. Thank you again for pointing out the unclear part.
What does this PR do?
This PR fixes a nondeterministic test failure in
ContractOnSettersParametrizedTest.data()when running with NonDex.Problem
ContractOnSettersParametrizedTestbuilds a list of (receiver, setter, argument) combinations by iterating over unordered collections. When NonDex randomizes the iteration order, some seeds produce combinations where a setter is invoked with an argument that is type-compatible at the Java level but invalid according to Spoon’s API. When those combinations are reached, the reflective invocation fails and the test becomes nondeterministicIn these cases:
IMPORT_REFERENCE:
CtImport.setReference()has a parameter of typeCtReference, but Spoon only accepts four specific subtypes:CtTypeReference,CtExecutableReference,CtFieldReference, andCtPackageReference. The argument generator can produce otherCtReferencesubtypes, which are assignable toCtReferencebut not accepted by Spoon when invoked. When NonDex orders the candidates such thatCtImport.setReference()is called with one of these unsupported subtypes, the reflective call violates the contract and the test fails.Pattern arguments: For
CtCasePattern.setPattern(), the parameter type allows patterns in general, so the generator may produce aCtUnnamedPatternas the argument. However,CtCasePatterncannot directly take aCtUnnamedPattern; such patterns are only valid when nested inside a record pattern. When NonDex produces an ordering whereCtCasePattern.setPattern()is invoked with aCtUnnamedPattern, the reflective call is invalid and throws during execution, causing the test to fail for some seeds.Reproduce Test
To reproduce the failure, run NonDex on
.module using the following commands:The Fix
Refined type handling for CtReference: Replaced the broad check
isAssignableFrom(CtReference.class)with an exact-type matchequals(CtReference.class)increateCompatibleObject(). In addition, added precise constructors forCtReferencesubtypes.Validated IMPORT_REFERENCE arguments: Before invoking
CtImport.setReference(), ensured the argument is one of the four valid reference types.Validated Pattern arguments: Skipped invalid invocations of
CtCasePattern.setPattern()withCtUnnamedPattern.Stabilized Set iteration: Replaced
HashSetcreation withLinkedHashSetto preserve element order and remove nondeterminism during argument generation.