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

Flaky test fixed for testJvmMethodSorter #1765

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

Doom-Prophet
Copy link

Description

Flaky tests are common occurrences in open-source projects, yielding inconsistent results—sometimes passing and sometimes failing—without code changes. NonDex is a tool for detecting and debugging wrong assumptions on under-determined Java APIs. I have resolved a flaky test issue using NonDex tool, specifically in the MethodSorterTest class located at org.junit.internal.MethodSorterTest.testJvmMethodSorter

Root cause

The flakiness comes from the original codes which tends to sort the list of methods of a class, and use this test to test whether the declared methods of a class is gotten in a predictable order after going through his method sorter. However, the codes here firstly gets the first list of methods using java.lang.Class.getDeclaredMethods(), then gets the second one by making the class of methods going through the sorter, notice that the sorter is written based on a comparator that compare the 'methods' words by getting their hashcode. The fact is, java.lang.Class.getDeclaredMethods() method in Java does not guarantee a specific order for the returned methods. The order in which the methods are returned is not specified and may vary between different Java implementations or versions.

Fix

The logical of comparator is correct, the test is made flaky only because of java.lang.Class.getDeclaredMethods(), thus remove that function to get the methods' list, instead directly apply the comparator's logic in the test to make it work normally.

Setup

Java: openjdk version "11.0.20.1"
Maven: Apache Maven 3.6.3

How to test

  1. Compile the module
    mvn install -pl . -am -DskipTests
  2. Run regular tests
    mvn -pl . test -Dtest=org.junit.internal.MethodSorterTest#testJvmMethodSorter
  3. Run tests with NonDex tool
    mvn -pl . edu.illinois:nondex-maven-plugin:2.1.1:nondex -Dtest=org.junit.internal.MethodSorterTest#testJvmMethodSorter

After fix, all tests pass

};

Arrays.sort(fromJvmWithSynthetics, methodComparator);
Arrays.sort(sorted, methodComparator);
Copy link
Member

@kcooney kcooney Nov 3, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test now simply verifies that the two methods return the same values, ignoring order, so with these changes it's not a particularly good test.

The ordering of methods returned by getDeclaredMethods() is no longer guaranteed as of JDK 7. I think the only way to make this test useful but not flaky would be to skip the test if the JDK is 7 or greater.

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