Skip to content

Conversation

@lbh930
Copy link

@lbh930 lbh930 commented Dec 12, 2025

Description

Modified TestParameterMethodProcessor.getFieldValueHolders to sort the fields returned by Class.getDeclaredFields() alphabetically. This ensures that the order in which @TestParameter annotated fields are processed is deterministic.

Motivation

NonDex detected test flakiness in:

  • com.google.testing.junit.testparameterinjector.TestParameterInjectorKotlinTest#test_success[TestParameter_PrimaryConstructorParamMixedWithField]
  • com.google.testing.junit.testparameterinjector.TestParameterMethodProcessorTest#test_success[MultipleAnnotatedFieldsAndParameters]

This is due to the non-deterministic order of fields returned by Class.getDeclaredFields(). The TestParameterInjector uses this order to generate test names (for example test[param1=val1, param2=val2]). When the order of fields changes the generated test names change, causing assertions that expect a specific order to fail. Sorting the fields ensures a consistent order regardless of JVM implementation or runtime conditions. NonDex output with the failure attached.
nondex_output__.txt

@nymanjens
Copy link
Collaborator

Hi Benhao,

Thanks for letting us know.

I don't think it would be good if we'd sort the fields alphabetically because:

  • The declared order (which it usually seems to use) is the desired order in which to display the fields to the user. Any other order would just be weird IMO
  • It changes the test name, which may break some clients (we can't always avoid it, but I try when I can)

However, I don't want to cause flaky tests in other people's projects, so I propose I hide the two failing tests (TestParameter_PrimaryConstructorParamMixedWithField and MultipleAnnotatedFieldsAndParameters) when exporting them from Google to Github. That way, they are still run on our end, but avoid causing trouble for you and others.

WDYT?

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