From 411bee5819d66bf9ec423b4565c5f4e57c246849 Mon Sep 17 00:00:00 2001 From: Ethan Hou Date: Tue, 4 Aug 2026 16:10:38 +0800 Subject: [PATCH 1/2] test: Refactor ByokServiceTests to use assertions instead of verifications --- .../ui/chat/services/ByokServiceTests.java | 37 ++++++++++++++----- 1 file changed, 28 insertions(+), 9 deletions(-) diff --git a/com.microsoft.copilot.eclipse.ui.test/src/com/microsoft/copilot/eclipse/ui/chat/services/ByokServiceTests.java b/com.microsoft.copilot.eclipse.ui.test/src/com/microsoft/copilot/eclipse/ui/chat/services/ByokServiceTests.java index a21e7116..1c70f99a 100644 --- a/com.microsoft.copilot.eclipse.ui.test/src/com/microsoft/copilot/eclipse/ui/chat/services/ByokServiceTests.java +++ b/com.microsoft.copilot.eclipse.ui.test/src/com/microsoft/copilot/eclipse/ui/chat/services/ByokServiceTests.java @@ -3,11 +3,12 @@ package com.microsoft.copilot.eclipse.ui.chat.services; +import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.argThat; -import static org.mockito.Mockito.clearInvocations; +import static org.mockito.Mockito.atLeastOnce; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; @@ -25,6 +26,8 @@ import org.mockito.Mock; import org.mockito.junit.jupiter.MockitoExtension; +import org.eclipse.swt.widgets.Display; + import com.microsoft.copilot.eclipse.core.lsp.CopilotLanguageServerConnection; import com.microsoft.copilot.eclipse.core.lsp.protocol.byok.ByokApiKey; import com.microsoft.copilot.eclipse.core.lsp.protocol.byok.ByokListApiKeyResponse; @@ -55,7 +58,6 @@ class ByokServiceTests { void setUp() { byokService = new ByokService(lsConnection); byokService.bindByokPreferencePage(preferencePage); - clearInvocations(preferencePage); } @AfterEach @@ -71,8 +73,7 @@ void testConfigureOllama_discoveryFailureKeepsSavedEndpointVisible() { assertThrows(CompletionException.class, () -> byokService.configureOllama(OLLAMA_ENDPOINT).join()); - verify(preferencePage).updateProviderUrlsDisplay(argThat( - providerUrls -> OLLAMA_ENDPOINT.equals(providerUrls.get(OLLAMA_PROVIDER)))); + assertEquals(OLLAMA_ENDPOINT, awaitProviderUrlsDisplay().get(OLLAMA_PROVIDER)); } @Test @@ -86,7 +87,7 @@ void testLoadProviderUrls_ignoresBlankUrlsAndKeepsFirstDuplicate() { byokService.loadProviderUrls().join(); - verify(preferencePage).updateProviderUrlsDisplay(Map.of(OLLAMA_PROVIDER, OLLAMA_ENDPOINT)); + assertEquals(Map.of(OLLAMA_PROVIDER, OLLAMA_ENDPOINT), awaitProviderUrlsDisplay()); } @Test @@ -118,15 +119,23 @@ void testConfigureOllama_discoveredModelsAreRegistered() { } @Test - void testDeleteOllamaConfig_removesEndpointBeforeRefresh() { + void testDeleteOllamaConfig_removesEndpoint() { + ByokListModelResponse emptyModels = new ByokListModelResponse(); + emptyModels.setModels(List.of()); when(lsConnection.deleteByokProviderConfig(any())).thenReturn(completedStatus()); - configureRefreshResponses(List.of()); + when(lsConnection.listByokModels(any())).thenReturn(CompletableFuture.completedFuture(emptyModels)); + when(lsConnection.listByokApiKeys(any(ByokApiKey.class))) + .thenReturn(CompletableFuture.completedFuture(new ByokListApiKeyResponse(List.of()))); + when(lsConnection.listByokProviderConfigs(any(ByokListProviderConfigParams.class))).thenReturn( + CompletableFuture.completedFuture(new ByokListProviderConfigResponse( + List.of(new ByokProviderConfig(OLLAMA_PROVIDER, OLLAMA_ENDPOINT)))), + CompletableFuture.completedFuture(new ByokListProviderConfigResponse(List.of()))); + byokService.loadProviderUrls().join(); - clearInvocations(preferencePage); byokService.deleteOllamaConfig().join(); - verify(preferencePage).updateProviderUrlsDisplay(argThat(Map::isEmpty)); + assertTrue(awaitProviderUrlsDisplay().isEmpty()); } private void configureRefreshResponses(List discoveredModels) { @@ -147,4 +156,14 @@ private CompletableFuture completedStatus() { response.setSuccess(true); return CompletableFuture.completedFuture(response); } + + // Flushes the UI Realm so the pending ISideEffect callback runs, then returns the last pushed value. + @SuppressWarnings("unchecked") + private Map awaitProviderUrlsDisplay() { + Display.getDefault().syncExec(() -> { }); + ArgumentCaptor> providerUrls = ArgumentCaptor.forClass(Map.class); + verify(preferencePage, atLeastOnce()).updateProviderUrlsDisplay(providerUrls.capture()); + return providerUrls.getValue(); + } + } From f4c4bd9b4c2a1af98f616a111e10a88186bc597a Mon Sep 17 00:00:00 2001 From: Ethan Hou Date: Tue, 4 Aug 2026 16:21:28 +0800 Subject: [PATCH 2/2] test: drain UI events on UI thread in ByokServiceTests flush helper Make awaitProviderUrlsDisplay robust regardless of whether the test runs on or off the SWT UI thread: pump readAndDispatch when on the UI thread, otherwise use a syncExec ordering barrier. --- .../eclipse/ui/chat/services/ByokServiceTests.java | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/com.microsoft.copilot.eclipse.ui.test/src/com/microsoft/copilot/eclipse/ui/chat/services/ByokServiceTests.java b/com.microsoft.copilot.eclipse.ui.test/src/com/microsoft/copilot/eclipse/ui/chat/services/ByokServiceTests.java index 1c70f99a..00e88013 100644 --- a/com.microsoft.copilot.eclipse.ui.test/src/com/microsoft/copilot/eclipse/ui/chat/services/ByokServiceTests.java +++ b/com.microsoft.copilot.eclipse.ui.test/src/com/microsoft/copilot/eclipse/ui/chat/services/ByokServiceTests.java @@ -157,10 +157,17 @@ private CompletableFuture completedStatus() { return CompletableFuture.completedFuture(response); } - // Flushes the UI Realm so the pending ISideEffect callback runs, then returns the last pushed value. + // Drains pending UI Realm callbacks so the ISideEffect has pushed the latest value, then returns it. @SuppressWarnings("unchecked") private Map awaitProviderUrlsDisplay() { - Display.getDefault().syncExec(() -> { }); + Display display = Display.getDefault(); + if (display.getThread() == Thread.currentThread()) { + while (display.readAndDispatch()) { + // drain queued asyncExec callbacks while on the UI thread + } + } else { + display.syncExec(() -> { }); + } ArgumentCaptor> providerUrls = ArgumentCaptor.forClass(Map.class); verify(preferencePage, atLeastOnce()).updateProviderUrlsDisplay(providerUrls.capture()); return providerUrls.getValue();