Skip to content

Commit 86e17df

Browse files
committed
Do not poll 100 times when asserting a reference is not enqueued
LeakTests.checkRef() polls up to 100 times, calling System.gc() and blocking for up to 100ms on ReferenceQueue.remove() in each round. It breaks out as soon as the reference is enqueued, so the positive case is cheap. DynamicSupportTests.testConfigurationElementTracker4 asserted the opposite, that the object stays reachable because the tracker holds a strong reference, by wrapping checkRef() in assertThrows(). That case exhausts all 100 rounds by design, costing 10s of poll timeouts plus 100 full garbage collections on every run. Measured on a full CI run it is 20.33s of the class total of 20.4s, while its three sibling tests take 0.10s together. Add checkRefNotEnqueued() with a small poll count for this case. It is the only negative call site among the 23 checkRef() uses in the repository.
1 parent 51efa85 commit 86e17df

2 files changed

Lines changed: 19 additions & 8 deletions

File tree

tests/org.eclipse.ui.tests/Eclipse UI Tests/org/eclipse/ui/tests/dynamicplugins/DynamicSupportTests.java

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@
1616
import static org.junit.Assert.assertEquals;
1717
import static org.junit.Assert.assertFalse;
1818
import static org.junit.Assert.assertNotNull;
19-
import static org.junit.Assert.assertThrows;
2019

2120
import java.lang.ref.ReferenceQueue;
2221
import java.lang.ref.WeakReference;
@@ -93,7 +92,7 @@ public void testConfigurationElementTracker4() throws Exception {
9392
ReferenceQueue<Object> queue = new ReferenceQueue<>();
9493
WeakReference<Object> ref = new WeakReference<>(o1, queue);
9594
o1 = null;
96-
assertThrows(Throwable.class, () -> LeakTests.checkRef(queue, ref));
95+
LeakTests.checkRefNotEnqueued(queue, ref);
9796
Object [] results = tracker.getObjects(e1);
9897
assertNotNull(results);
9998
assertEquals(1, results.length);

tests/org.eclipse.ui.tests/Eclipse UI Tests/org/eclipse/ui/tests/leaks/LeakTests.java

Lines changed: 18 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717

1818
import static org.eclipse.ui.tests.harness.util.UITestUtil.openTestWindow;
1919
import static org.eclipse.ui.tests.harness.util.UITestUtil.processEvents;
20+
import static org.junit.jupiter.api.Assertions.assertFalse;
2021
import static org.junit.jupiter.api.Assertions.assertNotNull;
2122
import static org.junit.jupiter.api.Assertions.assertTrue;
2223

@@ -69,19 +70,30 @@ public class LeakTests {
6970

7071
public static void checkRef(ReferenceQueue<?> queue, Reference<?> ref)
7172
throws IllegalArgumentException, InterruptedException {
72-
boolean flag = false;
73-
for (int i = 0; i < 100; i++) {
73+
assertTrue(pollForRef(queue, ref, 100), "Reference not enqueued");
74+
}
75+
76+
/**
77+
* Asserts that the given reference is not enqueued. Polls far fewer times than
78+
* {@link #checkRef}, because this case always exhausts every attempt.
79+
*/
80+
public static void checkRefNotEnqueued(ReferenceQueue<?> queue, Reference<?> ref)
81+
throws IllegalArgumentException, InterruptedException {
82+
assertFalse(pollForRef(queue, ref, 5), "Reference enqueued");
83+
}
84+
85+
private static boolean pollForRef(ReferenceQueue<?> queue, Reference<?> ref, int attempts)
86+
throws InterruptedException {
87+
for (int i = 0; i < attempts; i++) {
7488
System.gc();
7589
Thread.yield();
7690
processEvents();
7791
Reference<?> checkRef = queue.remove(100);
7892
if (checkRef != null && checkRef.equals(ref)) {
79-
flag = true;
80-
break;
93+
return true;
8194
}
8295
}
83-
84-
assertTrue(flag, "Reference not enqueued");
96+
return false;
8597
}
8698

8799
@SuppressWarnings("unchecked")

0 commit comments

Comments
 (0)