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.

Sign up for GitHub

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

Jump to bottom

Flaky test fixed for testJvmMethodSorter #1765

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

Flaky test fixed for testJvmMethodSorter #1765

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f9567f3
flaky test fixed for testJvmMethodSorter
Sep 27, 2023
c35844b
Descriptive message about the changes you made
Oct 13, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 16 additions & 0 deletions src/test/java/org/junit/internal/MethodSorterTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
import java.lang.reflect.Method;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Comparator;
import java.util.List;

import org.junit.FixMethodOrder;
Expand Down Expand Up @@ -147,6 +148,21 @@ void epsilon() {
public void testJvmMethodSorter() {
Method[] fromJvmWithSynthetics = DummySortJvm.class.getDeclaredMethods();
Method[] sorted = MethodSorter.getDeclaredMethods(DummySortJvm.class);

Comparator<Method> methodComparator = new Comparator<Method>() {
@Override
public int compare(Method m1, Method m2) {
int nameComparison = m1.getName().compareTo(m2.getName());
if (nameComparison != 0) {
return nameComparison;
}
return m1.toString().compareTo(m2.toString());
}
};

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

@kcooney kcooney Nov 3, 2023 •
edited
Loading

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.


assertArrayEquals(fromJvmWithSynthetics, sorted);
}

Expand Down