From 50100581c7209b751c7df2d194e06df20329ad2d Mon Sep 17 00:00:00 2001 From: Sergey Pak Date: Tue, 9 Jul 2024 13:46:12 +0200 Subject: [PATCH] Don't use test scope in tests (use timeoutRunBlocking instead). Related to IJPL-158038 GitOrigin-RevId: d9d4c92e77c7f28a80c05d173cdcd08d59e9c139 --- .../settingsSync/SettingsSyncBridge.kt | 111 +++++++++++------- .../intellij/settingsSync/SettingsSyncMain.kt | 2 +- .../settingsSync/SettingsProviderTest.kt | 36 +++--- .../settingsSync/SettingsSyncFlowTest.kt | 90 ++++++++------ .../settingsSync/SettingsSyncRealIdeTest.kt | 32 ++--- .../SettingsSyncRealIdeTestBase.kt | 20 ++-- .../settingsSync/SettingsSyncTestBase.kt | 30 +++-- 7 files changed, 191 insertions(+), 130 deletions(-) diff --git a/plugins/settings-sync/src/com/intellij/settingsSync/SettingsSyncBridge.kt b/plugins/settings-sync/src/com/intellij/settingsSync/SettingsSyncBridge.kt index e40370d42332..a154d149d59d 100644 --- a/plugins/settings-sync/src/com/intellij/settingsSync/SettingsSyncBridge.kt +++ b/plugins/settings-sync/src/com/intellij/settingsSync/SettingsSyncBridge.kt @@ -2,10 +2,8 @@ package com.intellij.settingsSync import com.intellij.codeInsight.template.impl.TemplateSettings import com.intellij.configurationStore.saveSettings -import com.intellij.openapi.Disposable import com.intellij.openapi.application.ApplicationManager import com.intellij.openapi.diagnostic.logger -import com.intellij.openapi.progress.runBlockingCancellable import com.intellij.platform.util.progress.withProgressText import com.intellij.settingsSync.SettingsSyncBridge.PushRequestMode.* import com.intellij.settingsSync.statistics.SettingsSyncEventsStatistics @@ -13,10 +11,11 @@ import com.intellij.util.containers.ContainerUtil import kotlinx.coroutines.* import org.jetbrains.annotations.ApiStatus import org.jetbrains.annotations.TestOnly -import org.jetbrains.annotations.VisibleForTesting import java.nio.file.Path import java.time.Instant import java.util.concurrent.TimeUnit +import java.util.concurrent.atomic.AtomicBoolean +import java.util.concurrent.atomic.AtomicLong import java.util.concurrent.locks.ReentrantLock /** @@ -26,7 +25,6 @@ import java.util.concurrent.locks.ReentrantLock @ApiStatus.Internal class SettingsSyncBridge( private val coroutineScope: CoroutineScope, - parentDisposable: Disposable, private val appConfigPath: Path, private val settingsLog: SettingsLog, private val ideMediator: SettingsSyncIdeMediator, @@ -40,6 +38,16 @@ class SettingsSyncBridge( @Volatile private var queueJob: Job? = null + @TestOnly + internal val eventsProcessed = AtomicLong() + + // used in tests only + private val eventProcessingFlag = AtomicBoolean() + + // used in tests only + internal val queueSize: Int + get() = pendingEvents.size + pendingExclusiveEvents.size + (if (eventProcessingFlag.get()) 1 else 0) + val isInitialized get() = queueJob != null @@ -70,6 +78,7 @@ class SettingsSyncBridge( if (locked) { eventsLock.unlock() } + pendingExclusiveEvents.remove(event) } } } @@ -103,7 +112,7 @@ class SettingsSyncBridge( private fun startQueue() { LOG.info("Starting settings sync queue") queueJob = coroutineScope.launch { - while (true) { + while (SettingsSyncSettings.getInstance().syncEnabled) { processPendingEvents() try { delay(1000) @@ -114,13 +123,12 @@ class SettingsSyncBridge( break; } } + LOG.info("Queue processing stopped") } } - private fun saveIdeSettings() { - runBlockingCancellable { - saveSettings(ApplicationManager.getApplication(), forceSavingAllSettings = true) - } + private suspend fun saveIdeSettings() { + saveSettings(ApplicationManager.getApplication()) } private suspend fun applyInitialChanges(initMode: InitMode) { @@ -244,50 +252,66 @@ class SettingsSyncBridge( } } - private suspend fun processPendingEvents() { - val locked = eventsLock.tryLock() + private suspend fun processPendingEvents(force: Boolean = false) { + var locked = eventsLock.tryLock() try { - if (locked) { + while (!locked) { + if (force) { + locked = eventsLock.tryLock() + } + else { + LOG.debug("Couldn't obtain event lock. Will retry later") + return + } + } + if (pendingEvents.isEmpty()) { + LOG.debug("Pending events is empty") + return + } + while (pendingEvents.isNotEmpty()) { + var pushRequestMode: PushRequestMode = PUSH_IF_NEEDED + var mergeAndPushAfterProcessingEvents = true val previousState = collectCurrentState() try { - var pushRequestMode: PushRequestMode = PUSH_IF_NEEDED - var mergeAndPushAfterProcessingEvents = true - while (pendingEvents.isNotEmpty()) { - val event = pendingEvents.removeAt(0) - LOG.info("Processing event $event") - when (event) { - is SyncSettingsEvent.IdeChange -> { - settingsLog.applyIdeState(event.snapshot, "Local changes made in the IDE") - } - is SyncSettingsEvent.CloudChange -> { - settingsLog.applyCloudState(event.snapshot, "Remote changes") - SettingsSyncLocalSettings.getInstance().knownAndAppliedServerId = event.serverVersionId - } - is SyncSettingsEvent.LogCurrentSettings -> { - settingsLog.logExistingSettings() - } - is SyncSettingsEvent.MustPushRequest -> { - pushRequestMode = MUST_PUSH - } - is SyncSettingsEvent.DeleteServerData -> { - mergeAndPushAfterProcessingEvents = false - stopSyncingAndRollback(previousState) - deleteServerData(event.afterDeleting) - } - SyncSettingsEvent.DeletedOnCloud -> { - mergeAndPushAfterProcessingEvents = false - stopSyncingAndRollback(previousState) - } + val event = pendingEvents.removeAt(0) + eventProcessingFlag.set(true) + LOG.info("Processing event $event") + when (event) { + is SyncSettingsEvent.IdeChange -> { + settingsLog.applyIdeState(event.snapshot, "Local changes made in the IDE") + } + is SyncSettingsEvent.CloudChange -> { + settingsLog.applyCloudState(event.snapshot, "Remote changes") + SettingsSyncLocalSettings.getInstance().knownAndAppliedServerId = event.serverVersionId + } + is SyncSettingsEvent.LogCurrentSettings -> { + settingsLog.logExistingSettings() + } + is SyncSettingsEvent.MustPushRequest -> { + pushRequestMode = MUST_PUSH + } + is SyncSettingsEvent.DeleteServerData -> { + mergeAndPushAfterProcessingEvents = false + stopSyncingAndRollback(previousState) + deleteServerData(event.afterDeleting) + } + SyncSettingsEvent.DeletedOnCloud -> { + mergeAndPushAfterProcessingEvents = false + stopSyncingAndRollback(previousState) } } if (mergeAndPushAfterProcessingEvents) { mergeAndPush(previousState.idePosition, previousState.cloudPosition, pushRequestMode) } + eventsProcessed.incrementAndGet() } catch (exception: Throwable) { stopSyncingAndRollback(previousState, exception) } + finally { + eventProcessingFlag.set(false) + } } } catch (th: Throwable) { @@ -486,10 +510,15 @@ class SettingsSyncBridge( @TestOnly fun waitForAllExecuted() { runBlocking { - processPendingEvents() + processPendingEvents(force = true) } } + @TestOnly + internal fun stop() { + stopSyncingAndRollback(null, null) + } + companion object { private val LOG = logger() } diff --git a/plugins/settings-sync/src/com/intellij/settingsSync/SettingsSyncMain.kt b/plugins/settings-sync/src/com/intellij/settingsSync/SettingsSyncMain.kt index 7c60a5a5c174..e3c266c6d933 100644 --- a/plugins/settings-sync/src/com/intellij/settingsSync/SettingsSyncMain.kt +++ b/plugins/settings-sync/src/com/intellij/settingsSync/SettingsSyncMain.kt @@ -74,7 +74,7 @@ class SettingsSyncMain(coroutineScope: CoroutineScope) : Disposable { ideMediator.getInitialSnapshot(appConfigPath, currentSnapshot) }) val updateChecker = SettingsSyncUpdateChecker(remoteCommunicator) - val bridge = SettingsSyncBridge(coroutineScope, parentDisposable, appConfigPath, settingsLog, ideMediator, remoteCommunicator, updateChecker) + val bridge = SettingsSyncBridge(coroutineScope, appConfigPath, settingsLog, ideMediator, remoteCommunicator, updateChecker) return SettingsSyncControls(ideMediator, updateChecker, bridge, remoteCommunicator, settingsSyncStorage) } } diff --git a/plugins/settings-sync/tests/com/intellij/settingsSync/SettingsProviderTest.kt b/plugins/settings-sync/tests/com/intellij/settingsSync/SettingsProviderTest.kt index 6d0f25898d42..d50e469b872c 100644 --- a/plugins/settings-sync/tests/com/intellij/settingsSync/SettingsProviderTest.kt +++ b/plugins/settings-sync/tests/com/intellij/settingsSync/SettingsProviderTest.kt @@ -1,9 +1,9 @@ package com.intellij.settingsSync import com.intellij.ide.GeneralSettings +import com.intellij.testFramework.common.timeoutRunBlocking +import com.intellij.testFramework.common.waitUntil import com.intellij.testFramework.registerExtension -import kotlinx.coroutines.test.runCurrent -import kotlinx.coroutines.test.runTest import kotlinx.serialization.Serializable import kotlinx.serialization.encodeToString import kotlinx.serialization.json.Json @@ -12,6 +12,7 @@ import org.junit.jupiter.api.Assertions.assertNotNull import org.junit.jupiter.api.BeforeEach import org.junit.jupiter.api.Test import kotlin.io.path.div +import kotlin.time.Duration.Companion.seconds internal class SettingsProviderTest : SettingsSyncRealIdeTestBase() { @@ -24,14 +25,14 @@ internal class SettingsProviderTest : SettingsSyncRealIdeTestBase() { } @Test - fun `settings from provider should be collected`() { + fun `settings from provider should be collected`() = timeoutRunBlockingAndStopBridge { val ideState = TestState("IDE value") settingsProvider.settings = ideState GeneralSettings.getInstance().initModifyAndSave { autoSaveFiles = false } - initSettingsSync(SettingsSyncBridge.InitMode.JustInit) + initSettingsSync() val expectedContent = TestSettingsProvider().serialize(ideState) assertFileWithContent(expectedContent, settingsSyncStorage / ".metainfo" / settingsProvider.id / settingsProvider.fileName) @@ -49,7 +50,7 @@ internal class SettingsProviderTest : SettingsSyncRealIdeTestBase() { } @Test - fun `settings from provider changed on another client should be applied`() = runTest { + fun `settings from provider changed on another client should be applied`() = timeoutRunBlockingAndStopBridge { val state = TestState("Server value") remoteCommunicator.prepareFileOnServer(settingsSnapshot { provided(settingsProvider.id, state) @@ -57,10 +58,7 @@ internal class SettingsProviderTest : SettingsSyncRealIdeTestBase() { initSettingsSync(SettingsSyncBridge.InitMode.JustInit) - - fireSettingsChangeAndRunCurrent() - - waitForAllExecutedAndRunCurrent() + syncSettingsAndWait() val expectedContent = TestSettingsProvider().serialize(state) assertFileWithContent(expectedContent, settingsSyncStorage / ".metainfo" / settingsProvider.id / settingsProvider.fileName) @@ -68,20 +66,19 @@ internal class SettingsProviderTest : SettingsSyncRealIdeTestBase() { } @Test - fun `test merge settings provider settings`() { + fun `test merge settings provider settings`() = timeoutRunBlockingAndStopBridge { val serverState = TestState(property = "Server value") remoteCommunicator.prepareFileOnServer(settingsSnapshot { provided(settingsProvider.id, serverState) }) - initSettingsSync(SettingsSyncBridge.InitMode.JustInit) + initSettingsSync() val localState = TestState(foo = "Local value") SettingsSyncEvents.getInstance().fireSettingsChanged(SyncSettingsEvent.IdeChange(settingsSnapshot { provided(settingsProvider.id, localState) })) - fireSettingsChangeAndRunCurrent() - waitForAllExecutedAndRunCurrent() + syncSettingsAndWait() val expectedState = TestState(property = "Server value", foo = "Local value") assertFileWithContent(TestSettingsProvider().serialize(expectedState), @@ -89,15 +86,16 @@ internal class SettingsProviderTest : SettingsSyncRealIdeTestBase() { assertEquals(expectedState, settingsProvider.settings, "Settings were not applied") } - private fun waitForAllExecutedAndRunCurrent() { + private fun syncSettingsAndWait(event: SyncSettingsEvent = SyncSettingsEvent.SyncRequest) { + SettingsSyncEvents.getInstance().fireSettingsChanged(event) bridge.waitForAllExecuted() - testScope.runCurrent() + timeoutRunBlocking { + waitUntil("Waiting for queue to finish processing") { + bridge.queueSize == 0 + } + } } - private fun fireSettingsChangeAndRunCurrent() { - SettingsSyncEvents.getInstance().fireSettingsChanged(SyncSettingsEvent.SyncRequest) - testScope.runCurrent() - } @Serializable internal data class TestState( diff --git a/plugins/settings-sync/tests/com/intellij/settingsSync/SettingsSyncFlowTest.kt b/plugins/settings-sync/tests/com/intellij/settingsSync/SettingsSyncFlowTest.kt index 2bcf36d54e91..9f5f06ba4f96 100644 --- a/plugins/settings-sync/tests/com/intellij/settingsSync/SettingsSyncFlowTest.kt +++ b/plugins/settings-sync/tests/com/intellij/settingsSync/SettingsSyncFlowTest.kt @@ -2,13 +2,15 @@ package com.intellij.settingsSync import com.intellij.idea.TestFor import com.intellij.openapi.components.SettingsCategory +import com.intellij.openapi.progress.currentThreadCoroutineScope import com.intellij.testFramework.LoggedErrorProcessor +import com.intellij.testFramework.common.timeoutRunBlocking +import com.intellij.testFramework.common.waitUntil import com.intellij.util.ConcurrencyUtil import com.intellij.util.concurrency.AppExecutorUtil.createBoundedScheduledExecutorService import com.intellij.util.io.createParentDirectories import com.intellij.util.io.write -import kotlinx.coroutines.test.runCurrent -import kotlinx.coroutines.test.runTest +import com.intellij.util.progress.sleepCancellable import org.eclipse.jgit.api.Git import org.eclipse.jgit.lib.Repository import org.eclipse.jgit.revwalk.RevCommit @@ -23,6 +25,7 @@ import java.time.Instant import java.util.concurrent.Callable import java.util.concurrent.CountDownLatch import kotlin.io.path.* +import kotlin.time.Duration.Companion.seconds internal class SettingsSyncFlowTest : SettingsSyncTestBase() { @@ -33,16 +36,26 @@ internal class SettingsSyncFlowTest : SettingsSyncTestBase() { ideMediator = MockSettingsSyncIdeMediator() } - private fun initSettingsSync(initMode: SettingsSyncBridge.InitMode = SettingsSyncBridge.InitMode.JustInit) { - val controls = SettingsSyncMain.init(testScope, disposable, settingsSyncStorage, configDir, remoteCommunicator, ideMediator) + private fun initSettingsSync( + initMode: SettingsSyncBridge.InitMode = SettingsSyncBridge.InitMode.JustInit, + waitForInit: Boolean = true, + ) { + SettingsSyncSettings.getInstance().state = SettingsSyncSettings.getInstance().state.withSyncEnabled(true) + val controls = SettingsSyncMain.init(currentThreadCoroutineScope(), disposable, settingsSyncStorage, configDir, remoteCommunicator, ideMediator) updateChecker = controls.updateChecker bridge = controls.bridge bridge.initialize(initMode) - testScope.runCurrent() + if (waitForInit) { + timeoutRunBlocking(2.seconds) { + while (!bridge.isInitialized) { + sleepCancellable(10) + } + } + } } @Test - fun `existing settings should be copied on initialization`() { + fun `existing settings should be copied on initialization`() = timeoutRunBlockingAndStopBridge { writeToConfig { fileState("options/laf.xml", "LaF Initial") } @@ -55,7 +68,7 @@ internal class SettingsSyncFlowTest : SettingsSyncTestBase() { } @Test - fun `settings modified between IDE sessions should be logged`() { + fun `settings modified between IDE sessions should be logged`() = timeoutRunBlockingAndStopBridge { // emulate first session with initialization val fileName = "options/laf.xml" val file = configDir.resolve(fileName).write("LaF Initial") @@ -76,7 +89,7 @@ internal class SettingsSyncFlowTest : SettingsSyncTestBase() { } @Test - fun `delete server data`() { + fun `delete server data`() = timeoutRunBlockingAndStopBridge { writeToConfig { fileState("options/laf.xml", "LaF Initial") } @@ -109,7 +122,7 @@ internal class SettingsSyncFlowTest : SettingsSyncTestBase() { } @Test - fun `disable settings sync if data on server was deleted`() { + fun `disable settings sync if data on server was deleted`() = timeoutRunBlockingAndStopBridge { val fileName = "options/laf.xml" val initialContent = "LaF Initial" configDir.resolve(fileName).write(initialContent) @@ -120,12 +133,11 @@ internal class SettingsSyncFlowTest : SettingsSyncTestBase() { val metaInfo = SettingsSnapshot.MetaInfo(Instant.now(), getLocalApplicationInfo(), isDeleted = true) remoteCommunicator.prepareFileOnServer(SettingsSnapshot(metaInfo, emptySet(), null, emptyMap(), emptySet())) syncSettingsAndWait() - Assertions.assertFalse(SettingsSyncSettings.getInstance().syncEnabled, "Settings sync was not disabled") } @Test - fun `first push after IDE start should update from server if needed`() { + fun `first push after IDE start should update from server if needed`() = timeoutRunBlockingAndStopBridge { // prepare settings on server val editorXml = "options/editor.xml" val editorContent = "Editor from Server" @@ -139,7 +151,7 @@ internal class SettingsSyncFlowTest : SettingsSyncTestBase() { configDir.resolve(lafXml).write(lafContent) initSettingsSync() - + bridge.waitForAllExecuted() val pushedSnapshot = remoteCommunicator.getVersionOnServer() Assertions.assertNotNull(pushedSnapshot, "Nothing has been pushed") pushedSnapshot!!.assertSettingsSnapshot { @@ -149,7 +161,7 @@ internal class SettingsSyncFlowTest : SettingsSyncTestBase() { } @Test - fun `enable settings sync with Push to Server should overwrite server snapshot instead of merging with it`() { + fun `enable settings sync with Push to Server should overwrite server snapshot instead of merging with it`() = timeoutRunBlockingAndStopBridge { // prepare settings on server val editorXml = "options/editor.xml" val editorContent = "Editor from Server" @@ -170,7 +182,7 @@ internal class SettingsSyncFlowTest : SettingsSyncTestBase() { } @Test - fun `enable settings via Take from Server should log existing settings`() { + fun `enable settings via Take from Server should log existing settings`() = timeoutRunBlockingAndStopBridge { val fileName = "options/laf.xml" val initialContent = "LaF Initial" configDir.resolve(fileName).write(initialContent) @@ -203,7 +215,7 @@ internal class SettingsSyncFlowTest : SettingsSyncTestBase() { } @Test - fun `enable settings with migration`() = runTest { + fun `enable settings with migration`() = timeoutRunBlockingAndStopBridge { val migration = migrationFromLafXml() initSettingsSync(SettingsSyncBridge.InitMode.MigrateFromOldStorage(migration)) @@ -212,7 +224,7 @@ internal class SettingsSyncFlowTest : SettingsSyncTestBase() { } @Test - fun `enable settings with migration and data on server should merge but prefer server data in case of conflicts`() { + fun `enable settings with migration and data on server should merge but prefer server data in case of conflicts`() = timeoutRunBlockingAndStopBridge { val migration = migration(settingsSnapshot { fileState("options/laf.xml", "Migration Data") fileState("options/editor.xml", "Migration Data") @@ -231,7 +243,7 @@ internal class SettingsSyncFlowTest : SettingsSyncTestBase() { //@Test // the implementation is postponed - fun `migrated settings with disabled categories should be pushed without settings from these categories`() { + fun `migrated settings with disabled categories should be pushed without settings from these categories`() = timeoutRunBlockingAndStopBridge { val migration = object : SettingsSyncMigration { override fun isLocalDataAvailable(appConfigDir: Path): Boolean = true @@ -254,7 +266,7 @@ internal class SettingsSyncFlowTest : SettingsSyncTestBase() { } @Test - fun `rollback settings and stop sync in case of error`() { + fun `rollback settings and stop sync in case of error`() = timeoutRunBlockingAndStopBridge { val fileName = "options/laf.xml" val initialContent = "LaF Initial" configDir.resolve(fileName).write(initialContent) @@ -278,7 +290,7 @@ internal class SettingsSyncFlowTest : SettingsSyncTestBase() { } @Test - fun `rollback settings and stop sync if error happens on initialization`() { + fun `rollback settings and stop sync if error happens on initialization`() = timeoutRunBlockingAndStopBridge { val initialContent = "LaF Initial" configDir.resolve("options/laf.xml").write(initialContent) @@ -292,7 +304,9 @@ internal class SettingsSyncFlowTest : SettingsSyncTestBase() { ideMediator.throwOnApply(exceptionToThrow) suppressFailureOnLogError(exceptionToThrow) { - initSettingsSync(SettingsSyncBridge.InitMode.TakeFromServer(SyncSettingsEvent.CloudChange(snapshot, null))) + timeoutRunBlocking { + initSettingsSync(SettingsSyncBridge.InitMode.TakeFromServer(SyncSettingsEvent.CloudChange(snapshot, null))) + } } Assertions.assertFalse((settingsSyncStorage / "options" / "editor.xml").exists(), "Partial apply was not rolled back") @@ -302,7 +316,7 @@ internal class SettingsSyncFlowTest : SettingsSyncTestBase() { } @Test - fun `sync settings`() { + fun `sync settings`() = timeoutRunBlockingAndStopBridge { writeToConfig { fileState("options/laf.xml", "LaF Initial") } @@ -313,14 +327,13 @@ internal class SettingsSyncFlowTest : SettingsSyncTestBase() { }) syncSettingsAndWait() - assertFileWithContent("Editor from Server", (settingsSyncStorage / "options" / "editor.xml")) assertFileWithContent("LaF Initial", (settingsSyncStorage / "options" / "laf.xml")) assertAppliedToIde("options/editor.xml", "Editor from Server") } @Test - fun `concurrent sync does not disable sync during initialization`() { + fun `concurrent sync does not disable sync during initialization`() = timeoutRunBlockingAndStopBridge { writeToConfig { fileState("options/laf.xml", "LaF Initial") } @@ -346,7 +359,7 @@ internal class SettingsSyncFlowTest : SettingsSyncTestBase() { } @Test - fun `migration should respect deletion on server`() { + fun `migration should respect deletion on server`() = timeoutRunBlockingAndStopBridge { val deletionSnapshot = SettingsSnapshot(SettingsSnapshot.MetaInfo(Instant.now(), getLocalApplicationInfo(), isDeleted = true), emptySet(), null, emptyMap(), emptySet()) remoteCommunicator.prepareFileOnServer(deletionSnapshot) @@ -359,12 +372,12 @@ internal class SettingsSyncFlowTest : SettingsSyncTestBase() { } @Test - fun `regular sync should push if there is nothing on server`() { + fun `regular sync should push if there is nothing on server`() = timeoutRunBlockingAndStopBridge { writeToConfig { fileState("options/editor.xml", "Editor Initial") } - initSettingsSync(SettingsSyncBridge.InitMode.PushToServer) remoteCommunicator.deleteAllFiles() + initSettingsSync(SettingsSyncBridge.InitMode.PushToServer) syncSettingsAndWait() @@ -374,15 +387,16 @@ internal class SettingsSyncFlowTest : SettingsSyncTestBase() { } @Test - fun `unknown additional files should be stored to the history`() { + fun `unknown additional files should be stored to the history`() = timeoutRunBlockingAndStopBridge { initSettingsSync() remoteCommunicator.prepareFileOnServer(settingsSnapshot { additionalFile("newformat.json", "File with new unknown format") }) syncSettingsAndWait() + val newFormatJson = settingsSyncStorage / ".metainfo" / "newformat.json" - assertFileWithContent("File with new unknown format", settingsSyncStorage / ".metainfo" / "newformat.json") + assertFileWithContent("File with new unknown format", newFormatJson) FileRepositoryBuilder.create(settingsSyncStorage.resolve(".git").toFile()).use { repository -> val git = Git(repository) val latestCommit = git.log().add(repository.findRef("HEAD").objectId).call().toList().first() @@ -391,7 +405,7 @@ internal class SettingsSyncFlowTest : SettingsSyncTestBase() { } @Test - fun `unknown additional files should be sent to the server`() { + fun `unknown additional files should be sent to the server`() = timeoutRunBlockingAndStopBridge { (settingsSyncStorage / ".metainfo" / "newformat.json").write("File with new unknown format") initSettingsSync(SettingsSyncBridge.InitMode.PushToServer) @@ -402,7 +416,7 @@ internal class SettingsSyncFlowTest : SettingsSyncTestBase() { @TestFor(issues = ["IDEA-326189"]) @Test - fun `create initial commit for empty repo`(){ + fun `create initial commit for empty repo`() = timeoutRunBlockingAndStopBridge { val dotGit: Path = settingsSyncStorage.resolve(".git") val repository = FileRepositoryBuilder().setGitDir(dotGit.toFile()).setAutonomous(true).readEnvironment().build() repository.create() @@ -412,7 +426,7 @@ internal class SettingsSyncFlowTest : SettingsSyncTestBase() { } @Test - fun `disable sync if init failed`() { + fun `disable sync if init failed`() = timeoutRunBlockingAndStopBridge { SettingsSyncSettings.getInstance().syncEnabled = true val dotGit: Path = settingsSyncStorage.resolve(".git") val repository = FileRepositoryBuilder().setGitDir(dotGit.toFile()).setAutonomous(true).readEnvironment().build() @@ -430,16 +444,24 @@ internal class SettingsSyncFlowTest : SettingsSyncTestBase() { ideMediator.throwOnGetInitial(RuntimeException(errorMessage)) LoggedErrorProcessor.executeAndReturnLoggedError { - initSettingsSync() + timeoutRunBlocking { + initSettingsSync(waitForInit = false) + waitUntil { + SettingsSyncStatusTracker.getInstance().getErrorMessage() != null + } + } } Assertions.assertFalse(SettingsSyncSettings.getInstance().syncEnabled) } private fun syncSettingsAndWait(event: SyncSettingsEvent = SyncSettingsEvent.SyncRequest) { SettingsSyncEvents.getInstance().fireSettingsChanged(event) - testScope.runCurrent() bridge.waitForAllExecuted() - testScope.runCurrent() + timeoutRunBlocking(2.seconds) { + waitUntil("Waiting for file to appear", 2.seconds) { + bridge.queueSize == 0 + } + } } private fun suppressFailureOnLogError(expectedException: RuntimeException, activity: () -> Unit) { diff --git a/plugins/settings-sync/tests/com/intellij/settingsSync/SettingsSyncRealIdeTest.kt b/plugins/settings-sync/tests/com/intellij/settingsSync/SettingsSyncRealIdeTest.kt index cb7d5ba79142..5a79307536fa 100644 --- a/plugins/settings-sync/tests/com/intellij/settingsSync/SettingsSyncRealIdeTest.kt +++ b/plugins/settings-sync/tests/com/intellij/settingsSync/SettingsSyncRealIdeTest.kt @@ -2,9 +2,7 @@ package com.intellij.settingsSync import com.intellij.configurationStore.getPerOsSettingsStorageFolderName import com.intellij.ide.GeneralSettings -import com.intellij.ide.ui.LafManager import com.intellij.ide.ui.UISettings -import com.intellij.ide.ui.laf.LafManagerImpl import com.intellij.openapi.Disposable import com.intellij.openapi.components.* import com.intellij.openapi.editor.ex.EditorSettingsExternalizable @@ -12,23 +10,26 @@ import com.intellij.openapi.keymap.impl.KeymapImpl import com.intellij.openapi.keymap.impl.KeymapManagerImpl import com.intellij.openapi.util.Disposer import com.intellij.settingsSync.SettingsSnapshot.MetaInfo +import com.intellij.testFramework.common.DEFAULT_TEST_TIMEOUT +import com.intellij.testFramework.common.timeoutRunBlocking import com.intellij.util.toByteArray import com.intellij.util.xmlb.annotations.Attribute +import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.runBlocking -import kotlinx.coroutines.test.runCurrent import org.junit.jupiter.api.Assertions -import org.junit.jupiter.api.Disabled import org.junit.jupiter.api.Test import java.nio.charset.Charset import java.time.Instant import kotlin.io.path.div import kotlin.io.path.exists import kotlin.io.path.readText +import kotlin.time.Duration +import kotlin.time.Duration.Companion.seconds internal class SettingsSyncRealIdeTest : SettingsSyncRealIdeTestBase() { @Test - fun `settings are pushed`() { + fun `settings are pushed`() = timeoutRunBlockingAndStopBridge { initSettingsSync(SettingsSyncBridge.InitMode.JustInit) executeAndWaitUntilPushed { @@ -47,7 +48,7 @@ internal class SettingsSyncRealIdeTest : SettingsSyncRealIdeTestBase() { } @Test - fun `scheme changes are logged`() { + fun `scheme changes are logged`() = timeoutRunBlockingAndStopBridge { initSettingsSync(SettingsSyncBridge.InitMode.JustInit) val keymap = createKeymap() @@ -76,7 +77,7 @@ internal class SettingsSyncRealIdeTest : SettingsSyncRealIdeTestBase() { } @Test - fun `quickly modified settings are pushed together`() { + fun `quickly modified settings are pushed together`() = timeoutRunBlockingAndStopBridge { initSettingsSync(SettingsSyncBridge.InitMode.JustInit) GeneralSettings.getInstance().initModifyAndSave { @@ -103,7 +104,7 @@ internal class SettingsSyncRealIdeTest : SettingsSyncRealIdeTestBase() { } @Test - fun `existing settings are copied on initialization`() { + fun `existing settings are copied on initialization`() = timeoutRunBlockingAndStopBridge { GeneralSettings.getInstance().initModifyAndSave { autoSaveFiles = false } @@ -131,7 +132,7 @@ internal class SettingsSyncRealIdeTest : SettingsSyncRealIdeTestBase() { } @Test - fun `disabled categories should be ignored when copying settings on initialization`() { + fun `disabled categories should be ignored when copying settings on initialization`() = timeoutRunBlockingAndStopBridge { GeneralSettings.getInstance().initModifyAndSave { autoSaveFiles = false } @@ -166,7 +167,7 @@ internal class SettingsSyncRealIdeTest : SettingsSyncRealIdeTestBase() { } @Test - fun `settings from server are applied`() { + fun `settings from server are applied`() = timeoutRunBlockingAndStopBridge(5.seconds) { val generalSettings = GeneralSettings.getInstance().init() initSettingsSync(SettingsSyncBridge.InitMode.JustInit) @@ -180,10 +181,11 @@ internal class SettingsSyncRealIdeTest : SettingsSyncRealIdeTestBase() { SettingsSyncEvents.getInstance().fireSettingsChanged(SyncSettingsEvent.SyncRequest) } Assertions.assertFalse(generalSettings.isSaveOnFrameDeactivation) + bridge.waitForAllExecuted() } @Test - fun `enabling category should copy existing settings from that category`() { + fun `enabling category should copy existing settings from that category`() = timeoutRunBlockingAndStopBridge { SettingsSyncSettings.getInstance().setCategoryEnabled(SettingsCategory.CODE, isEnabled = false) GeneralSettings.getInstance().initModifyAndSave { autoSaveFiles = false @@ -223,16 +225,16 @@ internal class SettingsSyncRealIdeTest : SettingsSyncRealIdeTestBase() { } @Test - fun `exportable non-roamable settings should not be synced`() { + fun `exportable non-roamable settings should not be synced`() = timeoutRunBlockingAndStopBridge { testVariousComponentsShouldBeSyncedOrNot(ExportableNonRoamable(), expectedToBeSynced = false) } @Test - fun `roamable settings should be synced`() { + fun `roamable settings should be synced`() = timeoutRunBlockingAndStopBridge { testVariousComponentsShouldBeSyncedOrNot(Roamable(), expectedToBeSynced = true) } - private fun testVariousComponentsShouldBeSyncedOrNot(component: BaseComponent, expectedToBeSynced: Boolean) { + private suspend fun testVariousComponentsShouldBeSyncedOrNot(component: BaseComponent, expectedToBeSynced: Boolean) { component.aState.foo = "bar" runBlocking { application.componentStore.saveComponent(component) @@ -276,7 +278,7 @@ internal class SettingsSyncRealIdeTest : SettingsSyncRealIdeTestBase() { } @Test - fun `local and remote changes in different files are both applied`() { + fun `local and remote changes in different files are both applied`() = timeoutRunBlockingAndStopBridge { val generalSettings = GeneralSettings.getInstance().init() initSettingsSync(SettingsSyncBridge.InitMode.JustInit) diff --git a/plugins/settings-sync/tests/com/intellij/settingsSync/SettingsSyncRealIdeTestBase.kt b/plugins/settings-sync/tests/com/intellij/settingsSync/SettingsSyncRealIdeTestBase.kt index 1e158a72a372..91b907fe375c 100644 --- a/plugins/settings-sync/tests/com/intellij/settingsSync/SettingsSyncRealIdeTestBase.kt +++ b/plugins/settings-sync/tests/com/intellij/settingsSync/SettingsSyncRealIdeTestBase.kt @@ -7,18 +7,19 @@ import com.intellij.openapi.application.ApplicationManager import com.intellij.openapi.components.PersistentStateComponent import com.intellij.openapi.components.StateStorage import com.intellij.openapi.components.impl.stores.IComponentStore +import com.intellij.openapi.progress.currentThreadCoroutineScope +import com.intellij.testFramework.common.timeoutRunBlocking import com.intellij.testFramework.replaceService -import kotlinx.coroutines.ExperimentalCoroutinesApi +import com.intellij.util.progress.sleepCancellable import kotlinx.coroutines.runBlocking -import kotlinx.coroutines.test.runCurrent import org.junit.jupiter.api.AfterEach import org.junit.jupiter.api.Assertions.assertTrue import org.junit.jupiter.api.BeforeEach import java.lang.reflect.Constructor import java.nio.file.Path import java.util.concurrent.CountDownLatch +import kotlin.time.Duration.Companion.seconds -@OptIn(ExperimentalCoroutinesApi::class) internal abstract class SettingsSyncRealIdeTestBase : SettingsSyncTestBase() { protected lateinit var componentStore: TestComponentStore @@ -33,13 +34,18 @@ internal abstract class SettingsSyncRealIdeTestBase : SettingsSyncTestBase() { componentStore.resetComponents() } - protected fun initSettingsSync(initMode: SettingsSyncBridge.InitMode = SettingsSyncBridge.InitMode.JustInit) { + protected suspend fun initSettingsSync(initMode: SettingsSyncBridge.InitMode = SettingsSyncBridge.InitMode.JustInit) { + SettingsSyncSettings.getInstance().state = SettingsSyncSettings.getInstance().state.withSyncEnabled(true) val ideMediator = SettingsSyncIdeMediatorImpl(componentStore, configDir, enabledCondition = { true }) - val controls = SettingsSyncMain.init(testScope, disposable, settingsSyncStorage, configDir, remoteCommunicator, ideMediator) + val controls = SettingsSyncMain.init(currentThreadCoroutineScope(), disposable, settingsSyncStorage, configDir, remoteCommunicator, ideMediator) updateChecker = controls.updateChecker bridge = controls.bridge bridge.initialize(initMode) - testScope.runCurrent() + timeoutRunBlocking(5.seconds) { + while (!bridge.isInitialized) { + sleepCancellable(10) + } + } } protected fun waitForSettingsToBeApplied(vararg componentsToReinit: PersistentStateComponent<*>, execution: () -> Unit) { @@ -47,9 +53,7 @@ internal abstract class SettingsSyncRealIdeTestBase : SettingsSyncTestBase() { componentStore.reinitLatch = cdl execution() - testScope.runCurrent() bridge.waitForAllExecuted() - testScope.runCurrent() assertTrue(cdl.wait(), "Didn't await until new settings are applied") diff --git a/plugins/settings-sync/tests/com/intellij/settingsSync/SettingsSyncTestBase.kt b/plugins/settings-sync/tests/com/intellij/settingsSync/SettingsSyncTestBase.kt index b0fc2683af6c..c3e643eb1667 100644 --- a/plugins/settings-sync/tests/com/intellij/settingsSync/SettingsSyncTestBase.kt +++ b/plugins/settings-sync/tests/com/intellij/settingsSync/SettingsSyncTestBase.kt @@ -4,6 +4,8 @@ import com.intellij.openapi.Disposable import com.intellij.openapi.application.ApplicationManager import com.intellij.openapi.application.impl.ApplicationImpl import com.intellij.openapi.diagnostic.logger +import com.intellij.testFramework.common.DEFAULT_TEST_TIMEOUT +import com.intellij.testFramework.common.timeoutRunBlocking import com.intellij.testFramework.junit5.TestApplication import com.intellij.testFramework.junit5.TestDisposable import com.intellij.util.io.createDirectories @@ -22,6 +24,7 @@ import java.util.concurrent.CountDownLatch import java.util.concurrent.TimeUnit import kotlin.io.path.exists import kotlin.io.path.readText +import kotlin.time.Duration internal val TIMEOUT_UNIT = TimeUnit.SECONDS @@ -38,10 +41,6 @@ internal abstract class SettingsSyncTestBase { protected lateinit var updateChecker: SettingsSyncUpdateChecker protected lateinit var bridge: SettingsSyncBridge - protected lateinit var testScheduler: TestCoroutineScheduler - protected lateinit var testScope: TestScope - - @TestDisposable protected lateinit var disposable: Disposable protected val settingsSyncStorage: Path get() = configDir.resolve("settingsSync") @@ -61,9 +60,6 @@ internal abstract class SettingsSyncTestBase { MockRemoteCommunicator() } - testScheduler = TestCoroutineScheduler() - testScope = TestScope(testScheduler) - val serverState = remoteCommunicator.checkServerState() if (serverState != ServerState.FileNotExists) { LOG.warn("Server state: $serverState") @@ -73,13 +69,26 @@ internal abstract class SettingsSyncTestBase { @AfterEach fun cleanup() { + remoteCommunicator.deleteAllFiles() if (::bridge.isInitialized) { bridge.waitForAllExecuted() + bridge.stop() } - - remoteCommunicator.deleteAllFiles() } + protected fun timeoutRunBlockingAndStopBridge( + timeout: Duration = DEFAULT_TEST_TIMEOUT, + coroutineName: String? = null, + action: suspend CoroutineScope.() -> T, + ): T { + return timeoutRunBlocking(timeout, coroutineName) { + val retval = action() + cleanup() + retval + } + } + + protected fun writeToConfig(build: SettingsSnapshotBuilder.() -> Unit) { val builder = SettingsSnapshotBuilder() builder.build() @@ -95,7 +104,6 @@ internal abstract class SettingsSyncTestBase { } protected fun assertServerSnapshot(build: SettingsSnapshotBuilder.() -> Unit) { - testScope.runCurrent() val pushedSnapshot = remoteCommunicator.getVersionOnServer() assertNotNull(pushedSnapshot, "Nothing has been pushed") pushedSnapshot!!.assertSettingsSnapshot { @@ -106,9 +114,7 @@ internal abstract class SettingsSyncTestBase { protected fun executeAndWaitUntilPushed(testExecution: () -> Unit): SettingsSnapshot { val snapshot = remoteCommunicator.awaitForPush { testExecution() - testScope.runCurrent() bridge.waitForAllExecuted() - testScope.runCurrent() } return snapshot }