Skip to content

Commit e51586e

Browse files
committed
Fix and reenable ExpressionTests.testSubTypeTiming
Replaces the flaky timing-based assertion with a functional validation of the caching behaviour: uses Mockito.mockStatic to verify that Expressions.uncachedIsSubtype is invoked exactly once for a given (Class, type) pair across two consecutive isInstanceOf calls, proving that the second call is served from the cache. Fixes #894
1 parent 2883155 commit e51586e

2 files changed

Lines changed: 36 additions & 25 deletions

File tree

runtime/tests/org.eclipse.core.expressions.tests/META-INF/MANIFEST.MF

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,8 @@ Require-Bundle:
1313
Import-Package: org.assertj.core.api,
1414
org.junit.jupiter.api;version="[5.14.0,6.0.0)",
1515
org.junit.jupiter.api.function;version="[5.14.0,6.0.0)",
16-
org.junit.platform.suite.api;version="[1.14.0,2.0.0)"
16+
org.junit.platform.suite.api;version="[1.14.0,2.0.0)",
17+
org.mockito
1718
Bundle-RequiredExecutionEnvironment: JavaSE-17
1819
Eclipse-BundleShape: dir
1920
Bundle-ActivationPolicy: lazy

runtime/tests/org.eclipse.core.expressions.tests/src/org/eclipse/core/internal/expressions/tests/ExpressionTests.java

Lines changed: 34 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -18,10 +18,9 @@
1818
import static org.assertj.core.api.Assertions.assertThatThrownBy;
1919
import static org.assertj.core.api.Assertions.fail;
2020
import static org.junit.jupiter.api.Assertions.assertEquals;
21-
import static org.junit.jupiter.api.Assertions.assertFalse;
2221
import static org.junit.jupiter.api.Assertions.assertNotNull;
2322
import static org.junit.jupiter.api.Assertions.assertThrows;
24-
import static org.junit.jupiter.api.Assertions.assertTrue;
23+
import static org.mockito.Mockito.times;
2524

2625
import java.net.URL;
2726
import java.util.AbstractCollection;
@@ -33,8 +32,9 @@
3332
import java.util.LinkedList;
3433
import java.util.List;
3534

36-
import org.junit.jupiter.api.Disabled;
3735
import org.junit.jupiter.api.Test;
36+
import org.mockito.MockedStatic;
37+
import org.mockito.Mockito;
3838
import org.osgi.framework.FrameworkUtil;
3939

4040
import org.w3c.dom.Document;
@@ -72,7 +72,13 @@
7272
@SuppressWarnings("restriction")
7373
public class ExpressionTests {
7474

75-
private static final int TYPE_ITERATIONS = 100000;
75+
/**
76+
* Used exclusively by {@link #testSubTypeTiming()} to provide a cache key type
77+
* unique to this test, avoiding interference with other tests.
78+
*/
79+
private static class CachingTestSet extends HashSet<Object> {
80+
private static final long serialVersionUID = 1L;
81+
}
7682

7783
public static class CollectionWrapper {
7884
public Collection<String> collection;
@@ -1014,28 +1020,32 @@ public void testSubType() throws Exception {
10141020
}
10151021

10161022
@Test
1017-
@Disabled("CI test environment too unstable for performance tests")
10181023
public void testSubTypeTiming() throws Exception {
1019-
HashSet<?> o1 = new HashSet<>();
1020-
1021-
System.gc();
1022-
long cachedStart= System.currentTimeMillis();
1023-
for (int i= 0; i < TYPE_ITERATIONS; i++) {
1024-
assertTrue(Expressions.isInstanceOf(o1, "java.util.Set"));
1025-
assertFalse(Expressions.isInstanceOf(o1, "java.util.List"));
1024+
CachingTestSet o = new CachingTestSet();
1025+
1026+
try (MockedStatic<Expressions> expressionsMock = Mockito.mockStatic(Expressions.class,
1027+
Mockito.CALLS_REAL_METHODS)) {
1028+
// First call (positive): cache miss — isSubtype must traverse the class
1029+
// hierarchy
1030+
assertThat(Expressions.isInstanceOf(o, "java.util.Set")).isTrue();
1031+
expressionsMock.verify(() -> Expressions.uncachedIsSubtype(CachingTestSet.class, "java.util.Set"),
1032+
times(1));
1033+
// Second call (positive): cache hit — uncachedIsSubtype must NOT be invoked
1034+
// again
1035+
assertThat(Expressions.isInstanceOf(o, "java.util.Set")).isTrue();
1036+
expressionsMock.verify(() -> Expressions.uncachedIsSubtype(CachingTestSet.class, "java.util.Set"),
1037+
times(1));
1038+
// First call (negative): cache miss — isSubtype must traverse the class
1039+
// hierarchy
1040+
assertThat(Expressions.isInstanceOf(o, "java.util.List")).isFalse();
1041+
expressionsMock.verify(() -> Expressions.uncachedIsSubtype(CachingTestSet.class, "java.util.List"),
1042+
times(1));
1043+
// Second call (negative): cache hit — uncachedIsSubtype must NOT be invoked
1044+
// again
1045+
assertThat(Expressions.isInstanceOf(o, "java.util.List")).isFalse();
1046+
expressionsMock.verify(() -> Expressions.uncachedIsSubtype(CachingTestSet.class, "java.util.List"),
1047+
times(1));
10261048
}
1027-
long cachedDelta= System.currentTimeMillis() - cachedStart;
1028-
1029-
System.gc();
1030-
long instanceStart= System.currentTimeMillis();
1031-
for (int i= 0; i < TYPE_ITERATIONS; i++) {
1032-
assertTrue(Expressions.uncachedIsSubtype(o1.getClass(), "java.util.Set"));
1033-
assertFalse(Expressions.uncachedIsSubtype(o1.getClass(), "java.util.List"));
1034-
}
1035-
long instanceDelta= System.currentTimeMillis() - instanceStart;
1036-
1037-
assertThat(cachedDelta).as("assert cachedDelta is less than instanceDelta" + instanceDelta)
1038-
.isLessThan(instanceDelta);
10391049
}
10401050

10411051
}

0 commit comments

Comments
 (0)