Don't use test scope in tests (use timeoutRunBlocking instead). Related to IJPL-158038

GitOrigin-RevId: d9d4c92e77c7f28a80c05d173cdcd08d59e9c139

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<SettingsSyncBridge>()
   }

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)
     }
   }

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(

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) {

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)
 

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")
 

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 <T> 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
   }