Fix random failing ServiceSupplierTestCase.testOptionalReferences #488

Fixes two issues: 1. The ServiceSupplierTestCase asserts several values that are assigned by an asynchronous task. To this end, it waits for the conditions to be fulfilled by the asynchronous task. However, the conditions are incomplete, leading to a race condition. 2. The ServiceSupplierTestCase has been duplicated for javax and jakarta and a fix for a random failure has only been applied to a single implementation in #829 This change corrects the conditions the test waits for and aligns both copies of the test class with each other. Fixes #488
eclipse-platform · Nov 29, 2023 · 9b0c7cc · 9b0c7cc
1 parent 2bcb23f
commit 9b0c7cc
Show file tree

Hide file tree

Showing 2 changed files with 14 additions and 10 deletions.
diff --git a/...x.tests/src/org/eclipse/e4/core/internal/tests/di/extensions/ServiceSupplierTestCase.java b/...x.tests/src/org/eclipse/e4/core/internal/tests/di/extensions/ServiceSupplierTestCase.java
@@ -73,11 +73,11 @@ public static class TestDisabledBean {
 		@Inject
 		@Optional
 		@Service(filterExpression = "(component=disabled)")
-		TestService disabledService;
+		volatile TestService disabledService;
 
 		@Inject
 		@Service(filterExpression = "(component=disabled)")
-		List<TestService> services;
+		volatile List<TestService> services;
 	}
 
 	private final List<ServiceRegistration<?>> registrations = new ArrayList<>();
@@ -250,36 +250,38 @@ public void testOptionalReferences() throws Exception {
 		try {
 			enabler.enableDisabledServiceA();
 			// wait for asynchronous service registry and injection to finish
-			waitForCondition(() -> bean.services.size() == 1, timeoutInMillis);
+			waitForCondition(() -> bean.services.size() == 1 && bean.disabledService != null, timeoutInMillis);
 			assertNotNull(bean.disabledService);
 			assertEquals(1, bean.services.size());
 			assertSame(DisabledServiceA.class, bean.disabledService.getClass());
 
 			enabler.enableDisabledServiceB();
 			// wait for asynchronous service registry and injection to finish
-			waitForCondition(() -> bean.services.size() == 2, timeoutInMillis);
+			waitForCondition(() -> bean.services.size() == 2 && bean.disabledService instanceof DisabledServiceB,
+					timeoutInMillis);
 			assertNotNull(bean.disabledService);
 			assertEquals(2, bean.services.size());
 			assertSame(DisabledServiceB.class, bean.disabledService.getClass());
 
 			enabler.disableDisabledServiceB();
 			// wait for asynchronous service registry and injection to finish
-			waitForCondition(() -> bean.services.size() == 1, timeoutInMillis);
+			waitForCondition(() -> bean.services.size() == 1 && bean.disabledService instanceof DisabledServiceA,
+					timeoutInMillis);
 			assertNotNull(bean.disabledService);
 			assertEquals(1, bean.services.size());
 			assertSame(DisabledServiceA.class, bean.disabledService.getClass());
 
 			enabler.disableDisabledServiceA();
 			// wait for asynchronous service registry and injection to finish
-			waitForCondition(() -> bean.services.size() == 0, timeoutInMillis);
+			waitForCondition(() -> bean.services.size() == 0 && bean.disabledService == null, timeoutInMillis);
 			assertNull(bean.disabledService);
 			assertEquals(0, bean.services.size());
 		} finally {
 			enabler.disableDisabledServiceA();
 			enabler.disableDisabledServiceB();
 			// wait for asynchronous service registry and injection to ensure
 			// clear state after this test
-			waitForCondition(() -> bean.services.size() == 0, timeoutInMillis);
+			waitForCondition(() -> bean.services.size() == 0 && bean.disabledService == null, timeoutInMillis);
 		}
 	}
 

diff --git a/...e.tests/src/org/eclipse/e4/core/internal/tests/di/extensions/ServiceSupplierTestCase.java b/...e.tests/src/org/eclipse/e4/core/internal/tests/di/extensions/ServiceSupplierTestCase.java
@@ -257,14 +257,16 @@ public void testOptionalReferences() throws Exception {
 
 			enabler.enableDisabledServiceB();
 			// wait for asynchronous service registry and injection to finish
-			waitForCondition(() -> bean.services.size() == 2, timeoutInMillis);
+			waitForCondition(() -> bean.services.size() == 2 && bean.disabledService instanceof DisabledServiceB,
+					timeoutInMillis);
 			assertNotNull(bean.disabledService);
 			assertEquals(2, bean.services.size());
 			assertSame(DisabledServiceB.class, bean.disabledService.getClass());
 
 			enabler.disableDisabledServiceB();
 			// wait for asynchronous service registry and injection to finish
-			waitForCondition(() -> bean.services.size() == 1, timeoutInMillis);
+			waitForCondition(() -> bean.services.size() == 1 && bean.disabledService instanceof DisabledServiceA,
+					timeoutInMillis);
 			assertNotNull(bean.disabledService);
 			assertEquals(1, bean.services.size());
 			assertSame(DisabledServiceA.class, bean.disabledService.getClass());
@@ -279,7 +281,7 @@ public void testOptionalReferences() throws Exception {
 			enabler.disableDisabledServiceB();
 			// wait for asynchronous service registry and injection to ensure
 			// clear state after this test
-			waitForCondition(() -> bean.services.size() == 0, timeoutInMillis);
+			waitForCondition(() -> bean.services.size() == 0 && bean.disabledService == null, timeoutInMillis);
 		}
 	}