diff --git a/src/main/kotlin/com/coder/toolbox/diagnostics/CoderLogger.kt b/src/main/kotlin/com/coder/toolbox/diagnostics/CoderLogger.kt index 618e8881..d918dd4c 100644 --- a/src/main/kotlin/com/coder/toolbox/diagnostics/CoderLogger.kt +++ b/src/main/kotlin/com/coder/toolbox/diagnostics/CoderLogger.kt @@ -1,3 +1,5 @@ +@file:Suppress("NOTHING_TO_INLINE", "OVERRIDE_BY_INLINE") + package com.coder.toolbox.diagnostics import com.coder.toolbox.session.SessionId @@ -11,7 +13,8 @@ import kotlinx.coroutines.launch private const val CLIENT_SESSION_ID_LOG_KEY = "client_session_id" -private fun withSessionId(sessionId: SessionId?, message: String): String = +@PublishedApi +internal fun withSessionId(sessionId: SessionId?, message: String): String = sessionId?.let { "$CLIENT_SESSION_ID_LOG_KEY=$it $message" } ?: message /** @@ -19,86 +22,112 @@ private fun withSessionId(sessionId: SessionId?, message: String): String = * * A null [SessionId] leaves the message unchanged. A non-null ID adds the correlation field, while * a set of IDs emits one log for each session (or one unchanged log when the set is empty). + * Logging methods are inline so the Toolbox logger identifies the business call site rather than + * this wrapper as the source of each message. */ class CoderLogger( - private val delegate: Logger, + @PublishedApi internal val delegate: Logger, private val ui: ToolboxUi, private val cs: CoroutineScope, private val i18n: LocalizableStringFactory, ) : Logger by delegate { - fun error(sessionId: SessionId?, exception: Throwable, message: String) { + override inline fun error(message: String) { + delegate.error(message) + } + + override inline fun error(exception: Throwable, message: String) { + delegate.error(exception, message) + } + + inline fun error(sessionId: SessionId?, exception: Throwable, message: String) { delegate.error(exception, withSessionId(sessionId, message)) } - fun error(sessionIds: Set, exception: Throwable, message: String) { - sessionIds.onceOrForEach { error(it, exception, message) } + inline fun error(sessionIds: Set, exception: Throwable, message: String) { + sessionIds.onceOrForEach { delegate.error(exception, withSessionId(it, message)) } + } + + override inline fun warn(message: String) { + delegate.warn(message) } - fun warn(sessionId: SessionId?, message: String) { + override inline fun warn(exception: Throwable, message: String) { + delegate.warn(exception, message) + } + + inline fun warn(sessionId: SessionId?, message: String) { delegate.warn(withSessionId(sessionId, message)) } - fun warn(sessionIds: Set, exception: Throwable, message: String) { + inline fun warn(sessionIds: Set, exception: Throwable, message: String) { sessionIds.onceOrForEach { delegate.warn(exception, withSessionId(it, message)) } } - fun debug(sessionId: SessionId?, message: String) { + override inline fun debug(message: String) { + delegate.debug(message) + } + + inline fun debug(sessionId: SessionId?, message: String) { delegate.debug(withSessionId(sessionId, message)) } - fun debug(sessionIds: Set, message: String) { - sessionIds.onceOrForEach { debug(it, message) } + inline fun debug(sessionIds: Set, message: String) { + sessionIds.onceOrForEach { delegate.debug(withSessionId(it, message)) } + } + + override inline fun info(message: String) { + delegate.info(message) } - fun info(sessionId: SessionId?, message: String) { + inline fun info(sessionId: SessionId?, message: String) { delegate.info(withSessionId(sessionId, message)) } - fun info(sessionIds: Set, message: String) { - sessionIds.onceOrForEach { info(it, message) } + inline fun info(sessionIds: Set, message: String) { + sessionIds.onceOrForEach { delegate.info(withSessionId(it, message)) } } - fun logAndShowError(title: String, error: String) { - error(error) + inline fun logAndShowError(title: String, error: String) { + delegate.error(error) showInfoPopup(title, error) } - fun logAndShowError(sessionId: SessionId?, title: String, error: String) { + inline fun logAndShowError(sessionId: SessionId?, title: String, error: String) { delegate.error(withSessionId(sessionId, error)) showInfoPopup(title, error) } - fun logAndShowError(title: String, error: String, exception: Throwable) { - error(exception, error) + inline fun logAndShowError(title: String, error: String, exception: Throwable) { + delegate.error(exception, error) showInfoPopup(title, error) } - fun logAndShowError(sessionId: SessionId?, title: String, error: String, exception: Throwable) { - error(sessionId, exception, error) + inline fun logAndShowError(sessionId: SessionId?, title: String, error: String, exception: Throwable) { + delegate.error(exception, withSessionId(sessionId, error)) showInfoPopup(title, error) } - fun logAndShowError( + inline fun logAndShowError( sessionIds: Set, title: String, error: String, exception: Throwable, ) { - sessionIds.onceOrForEach { this.error(it, exception, error) } + sessionIds.onceOrForEach { delegate.error(exception, withSessionId(it, error)) } showInfoPopup(title, error) } - fun logAndShowWarning(title: String, warning: String) { - warn(warning) + inline fun logAndShowWarning(title: String, warning: String) { + delegate.warn(warning) showInfoPopup(title, warning) } - fun logAndShowWarning(sessionId: SessionId?, title: String, warning: String) { - warn(sessionId, warning) + inline fun logAndShowWarning(sessionId: SessionId?, title: String, warning: String) { + delegate.warn(withSessionId(sessionId, warning)) showInfoPopup(title, warning) } - fun logAndShowWarning( + inline fun logAndShowWarning( sessionIds: Set, title: String, warning: String, @@ -108,13 +137,13 @@ class CoderLogger( showInfoPopup(title, warning) } - fun logAndShowWarning(title: String, warning: String, exception: Throwable) { - warn(exception, warning) + inline fun logAndShowWarning(title: String, warning: String, exception: Throwable) { + delegate.warn(exception, warning) showInfoPopup(title, warning) } - fun logAndShowInfo(title: String, info: String) { - info(info) + inline fun logAndShowInfo(title: String, info: String) { + delegate.info(info) showInfoPopup(title, info) } @@ -130,7 +159,8 @@ class CoderLogger( * It is launched fire-and-forget so the caller is not suspended until the user closes the * popup. The caller can run any follow-up work immediately. */ - private fun showInfoPopup(title: String, text: String) { + @PublishedApi + internal fun showInfoPopup(title: String, text: String) { cs.launch(CoroutineName("popup")) { try { ui.showInfoPopup( @@ -141,12 +171,13 @@ class CoderLogger( } catch (_: CancellationException) { // Expected when the plugin scope shuts down while the popup is open. } catch (ex: Exception) { - error(ex, "Failed to display popup with title '$title'") + delegate.error(ex, "Failed to display popup with title '$title'") } } } - private inline fun Set.onceOrForEach(action: (SessionId?) -> Unit) { + @PublishedApi + internal inline fun Set.onceOrForEach(action: (SessionId?) -> Unit) { if (isEmpty()) { action(null) } else { diff --git a/src/test/kotlin/com/coder/toolbox/CoderRemoteEnvironmentTest.kt b/src/test/kotlin/com/coder/toolbox/CoderRemoteEnvironmentTest.kt index c44693df..9e186fac 100644 --- a/src/test/kotlin/com/coder/toolbox/CoderRemoteEnvironmentTest.kt +++ b/src/test/kotlin/com/coder/toolbox/CoderRemoteEnvironmentTest.kt @@ -12,9 +12,11 @@ import com.coder.toolbox.sdk.v2.models.WorkspaceStatus import com.coder.toolbox.session.SessionId import com.coder.toolbox.session.SessionIdRegistry import com.coder.toolbox.store.CoderSettingsStore +import com.jetbrains.toolbox.api.core.diagnostics.Logger import com.jetbrains.toolbox.api.localization.LocalizableStringFactory import com.jetbrains.toolbox.api.remoteDev.environments.SshEnvironmentContentsView import com.jetbrains.toolbox.api.remoteDev.states.EnvironmentStateColorPalette +import com.jetbrains.toolbox.api.ui.ToolboxUi import io.mockk.Called import io.mockk.clearMocks import io.mockk.every @@ -86,7 +88,9 @@ class CoderRemoteEnvironmentTest { connectionInfo.environment, ) verify(exactly = 1) { - fixture.logger.info(sessionId, match(::isSessionStartedMessage)) + fixture.logger.info(match { + hasSessionId(it, sessionId) && isSessionStartedMessage(it) + }) } } finally { fixture.environment.dispose() @@ -103,29 +107,31 @@ class CoderRemoteEnvironmentTest { val firstSessionId = assertNotNull(fixture.currentSessionId()) verify(exactly = 1) { - fixture.logger.info(firstSessionId, match(::isSessionStartedMessage)) + fixture.logger.info(match { + hasSessionId(it, firstSessionId) && isSessionStartedMessage(it) + }) } fixture.environment.afterDisconnect(isManual = false) assertEquals(firstSessionId, fixture.currentSessionId()) verify(exactly = 1) { - fixture.logger.info( - firstSessionId, - match { - it.contains("without an explicit user disconnect") && - it.contains("environment=Ready") && - it.contains("workspace=RUNNING") && - it.contains("agent=CONNECTED") && - it.contains("agentLifecycle=READY") && - !it.contains("may indicate a workspace or agent change") - }, - ) + fixture.logger.info(match { + hasSessionId(it, firstSessionId) && + it.contains("without an explicit user disconnect") && + it.contains("environment=Ready") && + it.contains("workspace=RUNNING") && + it.contains("agent=CONNECTED") && + it.contains("agentLifecycle=READY") && + !it.contains("may indicate a workspace or agent change") + }) } fixture.environment.beforeConnection() assertEquals(firstSessionId, fixture.currentSessionId()) verify(exactly = 1) { - fixture.logger.info(firstSessionId, match(::isSessionStartedMessage)) + fixture.logger.info(match { + hasSessionId(it, firstSessionId) && isSessionStartedMessage(it) + }) } } finally { fixture.environment.dispose() @@ -150,8 +156,7 @@ class CoderRemoteEnvironmentTest { ) verify(exactly = 1) { fixture.logger.info( - sessionId, - "Starting the network metrics poll job for ${fixture.environment.id}", + "client_session_id=$sessionId Starting the network metrics poll job for ${fixture.environment.id}", ) } } finally { @@ -180,24 +185,26 @@ class CoderRemoteEnvironmentTest { } verify(exactly = 1) { fixture.logger.info( - firstSessionId, - "Removed Toolbox SSH session for ${fixture.environment.id} after manual disconnect", + "client_session_id=$firstSessionId Removed Toolbox SSH session for " + + "${fixture.environment.id} after manual disconnect", ) } verify(exactly = 1) { - fixture.logger.info( - firstSessionId, - match { - it.contains("after an explicit user disconnect") && - it.contains("Latest known Coder state") - }, - ) + fixture.logger.info(match { + hasSessionId(it, firstSessionId) && + it.contains("after an explicit user disconnect") && + it.contains("Latest known Coder state") + }) } verify(exactly = 1) { - fixture.logger.info(firstSessionId, match(::isSessionStartedMessage)) + fixture.logger.info(match { + hasSessionId(it, firstSessionId) && isSessionStartedMessage(it) + }) } verify(exactly = 1) { - fixture.logger.info(secondSessionId, match(::isSessionStartedMessage)) + fixture.logger.info(match { + hasSessionId(it, secondSessionId) && isSessionStartedMessage(it) + }) } } finally { fixture.environment.dispose() @@ -225,17 +232,15 @@ class CoderRemoteEnvironmentTest { assertEquals(sessionId, fixture.currentSessionId()) verify(exactly = 1) { - fixture.logger.info( - sessionId, - match { - it.contains("without an explicit user disconnect") && - it.contains("may indicate a workspace or agent change") && - it.contains("environment=Stopping") && - it.contains("workspace=STOPPING") && - it.contains("agent=DISCONNECTED") && - it.contains("agentLifecycle=SHUTTING_DOWN") - }, - ) + fixture.logger.info(match { + hasSessionId(it, sessionId) && + it.contains("without an explicit user disconnect") && + it.contains("may indicate a workspace or agent change") && + it.contains("environment=Stopping") && + it.contains("workspace=STOPPING") && + it.contains("agent=DISCONNECTED") && + it.contains("agentLifecycle=SHUTTING_DOWN") + }) } } finally { fixture.environment.dispose() @@ -255,13 +260,17 @@ class CoderRemoteEnvironmentTest { assertNull(fixture.currentSessionId()) verify(exactly = 1) { - fixture.logger.info(sessionId, match(::isSessionDisposedMessage)) + fixture.logger.info(match { + hasSessionId(it, sessionId) && isSessionDisposedMessage(it) + }) } fixture.environment.dispose() assertNull(fixture.currentSessionId()) verify(exactly = 1) { - fixture.logger.info(sessionId, match(::isSessionDisposedMessage)) + fixture.logger.info(match { + hasSessionId(it, sessionId) && isSessionDisposedMessage(it) + }) } } finally { fixture.removeSession() @@ -286,15 +295,13 @@ class CoderRemoteEnvironmentTest { fixture.environment.update(updatedWorkspace, updatedAgent) verify(exactly = 1) { - fixture.logger.info( - sessionId, - match { - it.contains("changed from Ready to Stopping") && - it.contains("Workspace status: RUNNING -> STOPPING") && - it.contains("agent status: CONNECTED -> DISCONNECTED") && - it.contains("agent lifecycle state: READY -> SHUTTING_DOWN") - }, - ) + fixture.logger.info(match { + hasSessionId(it, sessionId) && + it.contains("changed from Ready to Stopping") && + it.contains("Workspace status: RUNNING -> STOPPING") && + it.contains("agent status: CONNECTED -> DISCONNECTED") && + it.contains("agent lifecycle state: READY -> SHUTTING_DOWN") + }) } } finally { fixture.environment.dispose() @@ -324,13 +331,14 @@ class CoderRemoteEnvironmentTest { ) val agent = requireNotNull(workspace.latestBuild.resources.single().agents).single() val context = mockk(relaxed = true) - val logger = mockk(relaxed = true) + val logger = mockk(relaxed = true) val settingsStore = mockk(relaxed = true) - + val i18n = mockk(relaxed = true) + val coderLogger = CoderLogger(logger, mockk(relaxed = true), scope, i18n) every { context.cs } returns scope - every { context.logger } returns logger + every { context.logger } returns coderLogger every { context.settingsStore } returns settingsStore - every { context.i18n } returns mockk(relaxed = true) + every { context.i18n } returns i18n every { context.envStateColorPalette } returns mockk(relaxed = true) every { settingsStore.shouldAutoConnect(any()) } returns autoConnect @@ -342,7 +350,15 @@ class CoderRemoteEnvironmentTest { workspace = workspace, agent = agent, ) - return Fixture(environment, logger, settingsStore, workspace, agent, workspaceName, agentName) + return Fixture( + environment, + logger, + settingsStore, + workspace, + agent, + workspaceName, + agentName, + ) } private fun isSessionStartedMessage(message: String): Boolean = @@ -355,9 +371,12 @@ class CoderRemoteEnvironmentTest { message.contains("remov", ignoreCase = true) || message.contains("end", ignoreCase = true)) + private fun hasSessionId(message: String, sessionId: SessionId): Boolean = + message.startsWith("client_session_id=$sessionId ") + private data class Fixture( val environment: CoderRemoteEnvironment, - val logger: CoderLogger, + val logger: Logger, val settingsStore: CoderSettingsStore, val workspace: Workspace, val agent: WorkspaceAgent, diff --git a/src/test/kotlin/com/coder/toolbox/CoderRemoteProviderTest.kt b/src/test/kotlin/com/coder/toolbox/CoderRemoteProviderTest.kt index a0bc6626..717ea565 100644 --- a/src/test/kotlin/com/coder/toolbox/CoderRemoteProviderTest.kt +++ b/src/test/kotlin/com/coder/toolbox/CoderRemoteProviderTest.kt @@ -17,7 +17,10 @@ import com.coder.toolbox.store.CoderSettingsStore import com.coder.toolbox.views.CoderSetupWizardPage import com.coder.toolbox.views.state.StoredOAuthSession import com.coder.toolbox.views.state.WizardStep +import com.jetbrains.toolbox.api.core.diagnostics.Logger import com.jetbrains.toolbox.api.core.util.LoadableState +import com.jetbrains.toolbox.api.localization.LocalizableStringFactory +import com.jetbrains.toolbox.api.ui.ToolboxUi import io.mockk.clearAllMocks import io.mockk.coEvery import io.mockk.coVerify @@ -26,6 +29,7 @@ import io.mockk.mockk import io.mockk.slot import io.mockk.verify import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.test.StandardTestDispatcher import kotlinx.coroutines.test.runCurrent @@ -49,6 +53,7 @@ class CoderRemoteProviderTest { private lateinit var mockCli: CoderCLIManager private lateinit var mockContext: CoderToolboxContext private lateinit var mockLogger: CoderLogger + private lateinit var underlyingLogger: Logger private lateinit var remoteProvider: CoderRemoteProvider @BeforeTest @@ -56,7 +61,13 @@ class CoderRemoteProviderTest { mockClient = mockk(relaxed = true) mockCli = mockk(relaxed = true) mockContext = mockk(relaxed = true) - mockLogger = mockk(relaxed = true) + underlyingLogger = mockk(relaxed = true) + mockLogger = CoderLogger( + underlyingLogger, + mockk(relaxed = true), + CoroutineScope(Dispatchers.Unconfined), + mockk(relaxed = true), + ) val settingsStore = mockk(relaxed = true) every { mockContext.settingsStore } returns settingsStore every { mockContext.logger } returns mockLogger @@ -88,7 +99,8 @@ class CoderRemoteProviderTest { val agent = mockAgent("agent1") val workspace = mockWorkspace("ws1", WorkspaceStatus.RUNNING, listOf(mockResource(listOf(agent)))) coEvery { mockClient.workspaces(any()) } returns listOf(workspace) - every { mockCli.configSsh(any(), any(), any(), any()) } throws FileNotFoundException("Permission denied") + val failure = FileNotFoundException("Permission denied") + every { mockCli.configSsh(any(), any(), any(), any()) } throws failure // when val pollJob = remoteProvider.poll(mockClient, mockCli) @@ -102,12 +114,7 @@ class CoderRemoteProviderTest { } val warningText = slot() verify(exactly = 1) { - mockLogger.logAndShowWarning( - emptySet(), - "SSH configuration could not be updated", - capture(warningText), - any(), - ) + underlyingLogger.warn(failure, capture(warningText)) } assertTrue(warningText.captured.contains("Permission denied")) @@ -144,10 +151,14 @@ class CoderRemoteProviderTest { runCurrent() verify(exactly = 1) { - mockLogger.info( - setOf(firstSessionId, secondSessionId), - match { it.startsWith("Workspaces have changed, reconfiguring CLI:") }, - ) + underlyingLogger.info(match { + it.startsWith("client_session_id=$firstSessionId Workspaces have changed, reconfiguring CLI:") + }) + } + verify(exactly = 1) { + underlyingLogger.info(match { + it.startsWith("client_session_id=$secondSessionId Workspaces have changed, reconfiguring CLI:") + }) } pollJob.cancel() @@ -176,11 +187,10 @@ class CoderRemoteProviderTest { runCurrent() verify(exactly = 1) { - mockLogger.error( - setOf(firstSessionId, secondSessionId), - failure, - "workspace polling error encountered", - ) + underlyingLogger.error(failure, "client_session_id=$firstSessionId workspace polling error encountered") + } + verify(exactly = 1) { + underlyingLogger.error(failure, "client_session_id=$secondSessionId workspace polling error encountered") } pollJob.cancel() @@ -206,7 +216,9 @@ class CoderRemoteProviderTest { verify(exactly = 1) { existingEnvironment.dispose() } verify(exactly = 1) { - mockLogger.info(setOf(sessionId), match { it.startsWith("Workspaces have changed, reconfiguring CLI:") }) + underlyingLogger.info(match { + it.startsWith("client_session_id=$sessionId Workspaces have changed, reconfiguring CLI:") + }) } verify(exactly = 1) { mockCli.configSsh(any(), setOf(sessionId), any(), any()) @@ -230,11 +242,7 @@ class CoderRemoteProviderTest { assertTrue(remoteProvider.environments.value is LoadableState.Loading) verify(exactly = 0) { - mockLogger.logAndShowWarning( - "SSH configuration could not be updated", - any(), - any(), - ) + underlyingLogger.warn(any(), match { it.startsWith("Workspaces remain available") }) } pollJob.cancel() diff --git a/src/test/kotlin/com/coder/toolbox/diagnostics/CoderLoggerTest.kt b/src/test/kotlin/com/coder/toolbox/diagnostics/CoderLoggerTest.kt index af155ef4..4b0a626a 100644 --- a/src/test/kotlin/com/coder/toolbox/diagnostics/CoderLoggerTest.kt +++ b/src/test/kotlin/com/coder/toolbox/diagnostics/CoderLoggerTest.kt @@ -6,6 +6,7 @@ import com.jetbrains.toolbox.api.localization.LocalizableString import com.jetbrains.toolbox.api.localization.LocalizableStringFactory import com.jetbrains.toolbox.api.ui.ToolboxUi import io.mockk.coVerify +import io.mockk.every import io.mockk.mockk import io.mockk.verify import kotlinx.coroutines.CoroutineScope @@ -84,17 +85,25 @@ class CoderLoggerTest { } @Test - fun `log and show logs and displays the same user message`() { + fun `log and show logs and displays the same message`() { + val localizedTitle = mockk() + val localizedMessage = mockk() + val localizedOk = mockk() + every { i18n.pnotr("Connection ready") } returns localizedTitle + every { i18n.pnotr("Connected to the workspace") } returns localizedMessage + every { i18n.ptrl("OK") } returns localizedOk + logger.logAndShowInfo("Connection ready", "Connected to the workspace") verify(exactly = 1) { delegate.info("Connected to the workspace") } verify(exactly = 1) { i18n.pnotr("Connection ready") } verify(exactly = 1) { i18n.pnotr("Connected to the workspace") } + verify(exactly = 1) { i18n.ptrl("OK") } coVerify(exactly = 1) { ui.showInfoPopup( - any(), - any(), - any(), + localizedTitle, + localizedMessage, + localizedOk, ) } } diff --git a/src/test/kotlin/com/coder/toolbox/feed/IdeFeedManagerOfflineTest.kt b/src/test/kotlin/com/coder/toolbox/feed/IdeFeedManagerOfflineTest.kt index bd13b0dc..36e8cb4c 100644 --- a/src/test/kotlin/com/coder/toolbox/feed/IdeFeedManagerOfflineTest.kt +++ b/src/test/kotlin/com/coder/toolbox/feed/IdeFeedManagerOfflineTest.kt @@ -3,10 +3,14 @@ package com.coder.toolbox.feed import com.coder.toolbox.CoderToolboxContext import com.coder.toolbox.diagnostics.CoderLogger import com.coder.toolbox.store.CoderSettingsStore +import com.jetbrains.toolbox.api.core.diagnostics.Logger +import com.jetbrains.toolbox.api.localization.LocalizableStringFactory +import com.jetbrains.toolbox.api.ui.ToolboxUi import com.squareup.moshi.Moshi import com.squareup.moshi.Types import io.mockk.every import io.mockk.mockk +import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.test.runTest import org.junit.jupiter.api.AfterEach import org.junit.jupiter.api.Assertions.assertEquals @@ -86,7 +90,12 @@ class IdeFeedManagerOfflineTest { System.setProperty("user.home", tempDir.toAbsolutePath().toString()) context = mockk() - logger = mockk(relaxed = true) + logger = CoderLogger( + mockk(relaxed = true), + mockk(relaxed = true), + mockk(relaxed = true), + mockk(relaxed = true), + ) settingsStore = mockk(relaxed = true) every { context.logger } returns logger diff --git a/src/test/kotlin/com/coder/toolbox/feed/IdeFeedManagerTest.kt b/src/test/kotlin/com/coder/toolbox/feed/IdeFeedManagerTest.kt index 94fe5a1f..46244e9d 100644 --- a/src/test/kotlin/com/coder/toolbox/feed/IdeFeedManagerTest.kt +++ b/src/test/kotlin/com/coder/toolbox/feed/IdeFeedManagerTest.kt @@ -2,9 +2,13 @@ package com.coder.toolbox.feed import com.coder.toolbox.CoderToolboxContext import com.coder.toolbox.diagnostics.CoderLogger +import com.jetbrains.toolbox.api.core.diagnostics.Logger +import com.jetbrains.toolbox.api.localization.LocalizableStringFactory +import com.jetbrains.toolbox.api.ui.ToolboxUi import io.mockk.coEvery import io.mockk.every import io.mockk.mockk +import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.test.runTest import org.junit.jupiter.api.AfterEach import org.junit.jupiter.api.Assertions.assertEquals @@ -79,7 +83,12 @@ class IdeFeedManagerTest { System.setProperty("user.home", tempDir.toAbsolutePath().toString()) context = mockk() - logger = mockk(relaxed = true) + logger = CoderLogger( + mockk(relaxed = true), + mockk(relaxed = true), + mockk(relaxed = true), + mockk(relaxed = true), + ) every { context.logger } returns logger feedService = mockk() diff --git a/src/test/kotlin/com/coder/toolbox/session/SessionIdRegistryTest.kt b/src/test/kotlin/com/coder/toolbox/session/SessionIdRegistryTest.kt index 970bb1cc..2c8655bf 100644 --- a/src/test/kotlin/com/coder/toolbox/session/SessionIdRegistryTest.kt +++ b/src/test/kotlin/com/coder/toolbox/session/SessionIdRegistryTest.kt @@ -2,9 +2,13 @@ package com.coder.toolbox.session import com.coder.toolbox.CoderToolboxContext import com.coder.toolbox.diagnostics.CoderLogger +import com.jetbrains.toolbox.api.core.diagnostics.Logger +import com.jetbrains.toolbox.api.localization.LocalizableStringFactory +import com.jetbrains.toolbox.api.ui.ToolboxUi import io.mockk.every import io.mockk.mockk import io.mockk.verify +import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.async import kotlinx.coroutines.awaitAll @@ -17,11 +21,17 @@ import kotlin.test.assertNull import kotlin.test.assertTrue class SessionIdRegistryTest { - private val logger = mockk(relaxed = true) + private val logger = mockk(relaxed = true) + private val coderLogger = CoderLogger( + logger, + mockk(relaxed = true), + mockk(relaxed = true), + mockk(relaxed = true), + ) private val context = mockk(relaxed = true) init { - every { context.logger } returns logger + every { context.logger } returns coderLogger } @Test @@ -41,7 +51,9 @@ class SessionIdRegistryTest { assertEquals(first, second) assertEquals(first, SessionIdRegistry.findSession(key.workspaceName, key.agentName)) verify(exactly = 1) { - logger.info(first, "Created Toolbox SSH session for ${key.workspaceName}.${key.agentName}") + logger.info( + "client_session_id=$first Created Toolbox SSH session for ${key.workspaceName}.${key.agentName}" + ) } } @@ -89,7 +101,10 @@ class SessionIdRegistryTest { assertEquals(1, results.toSet().size) verify(exactly = 1) { - logger.info(results.first(), "Created Toolbox SSH session for ${key.workspaceName}.${key.agentName}") + logger.info( + "client_session_id=${results.first()} Created Toolbox SSH session for " + + "${key.workspaceName}.${key.agentName}" + ) } } diff --git a/src/test/kotlin/com/coder/toolbox/util/CoderProtocolHandlerTest.kt b/src/test/kotlin/com/coder/toolbox/util/CoderProtocolHandlerTest.kt index 3a2d57e3..7b768d22 100644 --- a/src/test/kotlin/com/coder/toolbox/util/CoderProtocolHandlerTest.kt +++ b/src/test/kotlin/com/coder/toolbox/util/CoderProtocolHandlerTest.kt @@ -12,8 +12,11 @@ import com.coder.toolbox.sdk.CoderRestClient import com.coder.toolbox.sdk.DataGen import com.coder.toolbox.session.SessionId import com.coder.toolbox.session.SessionIdRegistry +import com.jetbrains.toolbox.api.core.diagnostics.Logger import com.jetbrains.toolbox.api.core.util.LoadableState +import com.jetbrains.toolbox.api.localization.LocalizableStringFactory import com.jetbrains.toolbox.api.remoteDev.connection.RemoteToolsHelper +import com.jetbrains.toolbox.api.ui.ToolboxUi import io.mockk.coEvery import io.mockk.coVerify import io.mockk.every @@ -42,6 +45,7 @@ class CoderProtocolHandlerTest { private lateinit var handler: CoderProtocolHandler private lateinit var remoteToolsHelper: RemoteToolsHelper private lateinit var logger: CoderLogger + private lateinit var underlyingLogger: Logger private lateinit var environment: CoderRemoteEnvironment // Test Coroutine Scope @@ -68,7 +72,13 @@ class CoderProtocolHandlerTest { feedService = mockk(relaxed = true) ideFeedManager = IdeFeedManager(context, feedService) remoteToolsHelper = mockk(relaxed = true) - logger = mockk(relaxed = true) + underlyingLogger = mockk(relaxed = true) + logger = CoderLogger( + underlyingLogger, + mockk(relaxed = true), + CoroutineScope(dispatcher), + mockk(relaxed = true), + ) environment = mockk { every { id } returns "env-1" every { currentSessionId() } returns null @@ -437,10 +447,10 @@ class CoderProtocolHandlerTest { assertEquals("RR-241.1", handler.resolveIdeIdentifier(environment, "RR", "241.1")) - verify(exactly = 1) { logger.info(sessionId, "Available RR IDEs: [241.1]") } - verify(exactly = 1) { logger.info(sessionId, "Installed RR IDEs: []") } - verify(exactly = 0) { logger.info("Available RR IDEs: [241.1]") } - verify(exactly = 0) { logger.info("Installed RR IDEs: []") } + verify(exactly = 1) { underlyingLogger.info("client_session_id=$sessionId Available RR IDEs: [241.1]") } + verify(exactly = 1) { underlyingLogger.info("client_session_id=$sessionId Installed RR IDEs: []") } + verify(exactly = 0) { underlyingLogger.info("Available RR IDEs: [241.1]") } + verify(exactly = 0) { underlyingLogger.info("Installed RR IDEs: []") } } @Test @@ -456,8 +466,8 @@ class CoderProtocolHandlerTest { assertNull(handler.resolveIdeIdentifier(environment, "RR", "latest_eap")) - verify(exactly = 1) { logger.logAndShowError(sessionId, "Can't handle URI", message) } - verify(exactly = 0) { logger.logAndShowError("Can't handle URI", message) } + verify(exactly = 1) { underlyingLogger.error("client_session_id=$sessionId $message") } + verify(exactly = 0) { underlyingLogger.error(message) } } @Test @@ -499,10 +509,12 @@ class CoderProtocolHandlerTest { verify(exactly = 1) { environment.startSshConnection() } verify(exactly = 1) { - logger.info(sessionId, "Selected IDE RR-241.1 for RR with hint 241.1") + underlyingLogger.info("client_session_id=$sessionId Selected IDE RR-241.1 for RR with hint 241.1") + } + verify(exactly = 1) { + underlyingLogger.info("client_session_id=$sessionId Launching RR-241.1 on $environmentId") } - verify(exactly = 1) { logger.info(sessionId, "Launching RR-241.1 on $environmentId") } - verify(exactly = 0) { logger.info("Launching RR-241.1 on $environmentId") } + verify(exactly = 0) { underlyingLogger.info("Launching RR-241.1 on $environmentId") } } finally { SessionIdRegistry.removeSession(workspace.name, AGENT_BOB.name) } diff --git a/src/test/kotlin/com/coder/toolbox/util/ConnectionMonitoringServiceTest.kt b/src/test/kotlin/com/coder/toolbox/util/ConnectionMonitoringServiceTest.kt index 0222e420..3f57b1ce 100644 --- a/src/test/kotlin/com/coder/toolbox/util/ConnectionMonitoringServiceTest.kt +++ b/src/test/kotlin/com/coder/toolbox/util/ConnectionMonitoringServiceTest.kt @@ -9,20 +9,34 @@ import com.coder.toolbox.sdk.v2.models.WorkspaceAgentStatus import com.coder.toolbox.sdk.v2.models.WorkspaceBuild import com.coder.toolbox.sdk.v2.models.WorkspaceStatus import com.coder.toolbox.session.SessionIdRegistry +import com.jetbrains.toolbox.api.core.diagnostics.Logger +import com.jetbrains.toolbox.api.localization.LocalizableStringFactory +import com.jetbrains.toolbox.api.ui.ToolboxUi import io.mockk.clearMocks import io.mockk.every import io.mockk.mockk import io.mockk.verify +import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.Dispatchers import java.util.UUID import kotlin.test.Test +private const val CONNECTION_WARNING = + "Unstable connection between Coder server and workspace detected. Your active sessions may disconnect" + class ConnectionMonitoringServiceTest { private val context = mockk(relaxed = true) - private val logger = mockk(relaxed = true) + private val logger = mockk(relaxed = true) + private val coderLogger = CoderLogger( + logger, + mockk(relaxed = true), + CoroutineScope(Dispatchers.Unconfined), + mockk(relaxed = true), + ) init { - every { context.logger } returns logger + every { context.logger } returns coderLogger } @Test @@ -33,7 +47,7 @@ class ConnectionMonitoringServiceTest { service.checkConnectionStatus(workspace, agent) - verify(exactly = 1) { logger.logAndShowWarning(null, any(), any()) } + verify(exactly = 1) { logger.warn(CONNECTION_WARNING) } } @Test @@ -44,7 +58,7 @@ class ConnectionMonitoringServiceTest { service.checkConnectionStatus(workspace, agent) - verify(exactly = 1) { logger.logAndShowWarning(null, any(), any()) } + verify(exactly = 1) { logger.warn(CONNECTION_WARNING) } } @Test @@ -58,11 +72,7 @@ class ConnectionMonitoringServiceTest { service.checkConnectionStatus(workspace, agent) verify(exactly = 1) { - logger.logAndShowWarning( - sessionId, - "Unstable connection detected", - any(), - ) + logger.warn("client_session_id=$sessionId $CONNECTION_WARNING") } } finally { SessionIdRegistry.removeSession(workspace.name, agent.name) @@ -77,7 +87,7 @@ class ConnectionMonitoringServiceTest { service.checkConnectionStatus(workspace, agent) - verify(exactly = 0) { logger.logAndShowWarning(null, any(), any()) } + verify(exactly = 0) { logger.warn(any()) } } @Test @@ -88,7 +98,7 @@ class ConnectionMonitoringServiceTest { service.checkConnectionStatus(workspace, agent) - verify(exactly = 0) { logger.logAndShowWarning(null, any(), any()) } + verify(exactly = 0) { logger.warn(any()) } } @Test @@ -106,7 +116,7 @@ class ConnectionMonitoringServiceTest { // Second call should not trigger notification service.checkConnectionStatus(workspace, agent) - verify(exactly = 0) { logger.logAndShowWarning(null, any(), any()) } + verify(exactly = 0) { logger.warn(any()) } } @Test @@ -121,7 +131,7 @@ class ConnectionMonitoringServiceTest { // Second call should not trigger notification service.checkConnectionStatus(workspace, agent) - verify(exactly = 1) { logger.logAndShowWarning(null, any(), any()) } + verify(exactly = 1) { logger.warn(CONNECTION_WARNING) } } @Test @@ -138,7 +148,7 @@ class ConnectionMonitoringServiceTest { // Second call should not trigger notification service.checkConnectionStatus(ws2, agent2) - verify(exactly = 1) { logger.logAndShowWarning(null, any(), any()) } + verify(exactly = 1) { logger.warn(CONNECTION_WARNING) } } @Test @@ -155,7 +165,7 @@ class ConnectionMonitoringServiceTest { // Second call should not trigger notification service.checkConnectionStatus(ws2, agent2) - verify(exactly = 1) { logger.logAndShowWarning(null, any(), any()) } + verify(exactly = 1) { logger.warn(CONNECTION_WARNING) } } diff --git a/src/test/kotlin/com/coder/toolbox/views/ActionTest.kt b/src/test/kotlin/com/coder/toolbox/views/ActionTest.kt index 41eff3e4..d0e5ec5b 100644 --- a/src/test/kotlin/com/coder/toolbox/views/ActionTest.kt +++ b/src/test/kotlin/com/coder/toolbox/views/ActionTest.kt @@ -3,6 +3,9 @@ package com.coder.toolbox.views import com.coder.toolbox.CoderToolboxContext import com.coder.toolbox.diagnostics.CoderLogger import com.coder.toolbox.session.SessionId +import com.jetbrains.toolbox.api.core.diagnostics.Logger +import com.jetbrains.toolbox.api.localization.LocalizableStringFactory +import com.jetbrains.toolbox.api.ui.ToolboxUi import io.mockk.every import io.mockk.mockk import io.mockk.verify @@ -16,11 +19,17 @@ class ActionTest { @OptIn(ExperimentalCoroutinesApi::class) fun `action failure uses the current session`() = runTest { val context = mockk(relaxed = true) - val logger = mockk(relaxed = true) + val logger = mockk(relaxed = true) val sessionId = SessionId.generate() val testScope = this + val coderLogger = CoderLogger( + logger, + mockk(relaxed = true), + testScope, + mockk(relaxed = true), + ) every { context.cs } returns testScope - every { context.logger } returns logger + every { context.logger } returns coderLogger val action = Action(context, "Stop workspace") { error("stop failed") }.withCurrentSessionId { sessionId } @@ -29,12 +38,7 @@ class ActionTest { advanceUntilIdle() verify(exactly = 1) { - logger.logAndShowError( - sessionId, - "Error while running `Stop workspace`", - "stop failed", - any(), - ) + logger.error(any(), "client_session_id=$sessionId stop failed") } } @@ -42,12 +46,18 @@ class ActionTest { @OptIn(ExperimentalCoroutinesApi::class) fun `action failure resolves the session when the error is logged`() = runTest { val context = mockk(relaxed = true) - val logger = mockk(relaxed = true) + val logger = mockk(relaxed = true) val sessionId = SessionId.generate() var currentSessionId: SessionId? = sessionId val testScope = this + val coderLogger = CoderLogger( + logger, + mockk(relaxed = true), + testScope, + mockk(relaxed = true), + ) every { context.cs } returns testScope - every { context.logger } returns logger + every { context.logger } returns coderLogger val action = Action(context, "Stop workspace") { currentSessionId = null error("stop failed") @@ -57,12 +67,7 @@ class ActionTest { advanceUntilIdle() verify(exactly = 1) { - logger.logAndShowError( - null, - "Error while running `Stop workspace`", - "stop failed", - any(), - ) + logger.error(any(), "stop failed") } } }