Paparazzi breaks delay functionality when overriding Dispatchers.Main with a TestDispatcher

[cbruegg]

Description
The Android Developer Documentation recommends overriding Dispatchers.Main with a TestDispatcher. This enables control over the execution of coroutines. Doing this after a Paparazzi snapshot has been taken breaks delay scheduling, causing tests executed using kotlinx-coroutiens-test' runTest to instantly time out.

Steps to Reproduce
The following test requires kotlinx-coroutines-test = { module = "org.jetbrains.kotlinx:kotlinx-coroutines-test", version = "1.7.3" } .

class DispatchingTest {
  @get:Rule
  val paparazzi = Paparazzi()

  @Test
  @OptIn(ExperimentalCoroutinesApi::class)
  fun shouldNotBreakDelayScheduling() {
    // As set by PaparazziPlugin
    withSystemProperty("kotlinx.coroutines.main.delay", "true") {
      // Simulate a Paparazzi test
      paparazzi.snapshot {
        HelloPaparazzi()
      }

      // Now we simulate a subsequent non-Paparazzi test
      Dispatchers.setMain(UnconfinedTestDispatcher())
      try {
        runTest(timeout = 10.seconds) {
          testScheduler.advanceUntilIdle()
        }
      } catch (e: AssertionError) {
        if (e.javaClass.name == "kotlinx.coroutines.test.UncompletedCoroutinesError") {
          throw AssertionError("Timeout delay was triggered instantly", e)
        } else {
          throw e
        }
      } finally {
        Dispatchers.resetMain()
      }
    }
  }

  private fun withSystemProperty(name: String, value: String, block: () -> Unit) {
    val prev = System.getProperty(name)
    System.setProperty(name, value)
    try {
      block()
    } finally {
      if (prev != null) {
        System.setProperty(name, prev)
      } else {
        System.clearProperty(name)
      }
    }
  }
}

Please note that the error also happens when the code within runTest { ... } is in a separate @Test method. Using Paparazzi therefore has an impact on tests that are unrelated to the Paparazzi tests of a given project.

Expected behavior
The runTest code should not time out.

Additional information:

• Paparazzi Version: 1.3.1
• OS: macOS
• Compile SDK: 34
• Gradle Version: 8.1.1
• Android Gradle Plugin Version: 8.0.2

Analysis:
First off, I believe this change introduced the bug. It sets the system property kotlinx.coroutines.main.delay to true. This then causes a very unfortunate interaction of different global coroutine initialization and testing mechanisms. Now onto more details:

Paparazzi needs a simulated Android environment. When it runs, it initializes the main Looper:

class Paparazzi @JvmOverloads constructor(
// ...

) : TestRule {
  // ...

  fun prepare(description: Description) {
    // ...
    prepareThread() // Calls com.android.layoutlib.bridge.Bridge.prepareThread() 
    // ...
  }

public static synchronized void prepareThread() {
    if (Looper.myLooper() == null) {
        synchronized(Looper.class) {
            if (Looper.getMainLooper() == null) {
                Looper.prepareMainLooper();
            }
        }
    }
}

This has an impact on how coroutines' Dispatchers.Main is initialized when it's first accessed. Paparazzi also happens to access it as it uses a ComposeView under the hood, which in turn uses coroutines and Dispatchers.Main.

Dispatchers.Main is initialized in MainDispatcherLoader by trying different factories. One of these factories is the AndroidDispatcherFactory:

class AndroidDispatcherFactory : MainDispatcherFactory {

    override fun createDispatcher(allFactories: List<MainDispatcherFactory>): MainCoroutineDispatcher {
        val mainLooper = Looper.getMainLooper() ?: throw IllegalStateException("The main looper is not available")
        return HandlerContext(mainLooper.asHandler(async = true))
    }

    // ...
}

We can immediately see here that this factory only succeeds if the main Looper is initialized, which is the case when Dispatchers.Main is initialized from within a Paparazzi test. If this wasn't the case, the MainDispatcherLoader would fall back to the next factory, i.e. TestMainDispatcherFactory. This would create a TestMainDispatcher that partially delegates to a placeholder MissingMainCoroutineDispatcher.

Even though Paparazzi tears down the main Looper in Paparazzi.close() when a Paparazzi test completes, it cannot undo the initialization of Dispatchers.Main. Surprisingly, even Dispatchers.resetMain() does not fully reset it, as it just switches back to the default mainDispatcher which is already affected by Paparazzi.

Now let's assume our Paparazzi snapshot got taken and we proceed to the call to runTest with timeout = 10.seconds. This sets up a timeout using coroutineContext.delay (see setupTimeout in Timeout.kt). It calls coroutineContext.delay.invokeOnTimeout, which delegates to
DefaultDelay.invokeOnTimeout in the default implementation. DefaultDelay is a global variable that gets initialized as follows:

private val defaultMainDelayOptIn = systemProp("kotlinx.coroutines.main.delay", false)

internal actual val DefaultDelay: Delay = initializeDefaultDelay()

private fun initializeDefaultDelay(): Delay {
    if (!defaultMainDelayOptIn) return DefaultExecutor
    val main = Dispatchers.Main
    // ...
    return if (main.isMissing() || main !is Delay) DefaultExecutor else main
}

There are two ways this can go (assuming kotlinx.coroutines.main.delay is true, as introduced by the change linked to earlier):

1. If the main dispatcher is missing, DefaultExecutor is picked as the implementation for DefaultDelay.
2. If the main dispatcher is not missing, it is picked as the implementation for DefaultDelay.

Remember that the main dispatcher is not missing (= MissingMainCoroutineDispatcher) as the AndroidDispatcherFactory succeeded. So now delays are scheduled using the main dispatcher. Note that with kotlinx.coroutines.main.delay set to false the DefaultExecutor would always used as the DefaultDelay, ensuring delays would be scheduled correctly.

But this is all not a problem yet: The HandlerContext returned by the AndroidDispatcherFactory has been installed as the main dispatcher and knows how to schedule delays.

However, note that the test installs an UnconfinedTestDispatcher as Dispatchers.Main, as is recommended.

Among other features, the TestDispatcher skips delays! It does this to accelerate tests. Unfortunately for us, this has the side-effect that the delay of 10 seconds scheduled by invokeOnTimeout is skipped and the timeout hits instantly.

So in short, this is what happens:

1. Paparazzi initializes the main Looper
2. Due to that, Dispatchers.Main is the HandlerContext returned by AndroidDispatcherFactory instead of TestMainDispatcher backed by a MissingMainCoroutineDispatcher
3. This causes DefaultDelay to delegate to Dispatchers.Main as the Paparazzi Gradle plugin has set kotlinx.coroutines.main.delay to true
4. Our test overrides Dispatchers.Main with a dispatcher that skips delays
5. The timeout from runTest is scheduled using the DefaultDelay implementation, which is Dispatchers.Main . Unfortunately we have overridden Dispatchers.Main with a dispatcher that skips delays, so the timeout happens instantly.

To solve this problem, I found multiple possible alternatives to consider. There may be others.

1. Advise users not to use StandardTestDispatcher and instead to use one that does not skip delays. This requires users to touch non-Paparazzi tests and potentially implement a custom dispatcher if they at least need the control features provided by TestDispatcher, like advanceUntilIdle()
2. Access Dispatchers.Main and DefaultDelay once before Paparazzi prepares the Looper, to force them to initialize with the correct environment. This is what we do in our project now.
3. Run Paparazzi tests in a separate JVM process so that other tests are not affected by its modification of JVM-global state. We did not find a nice way to configure Gradle/JUnit to do this.
4. Run Paparazzi tests at the end, after all other tests have completed. We did not find a nice way to configure Gradle/JUnit to do this.
5. Try and reset the global state after each Paparazzi test (but this requires Reflection and might require adaptions when updating dependencies).
6. Revert this change

[TWiStErRob]

Nice :micdrop: report.

Re solution 3. see #1161; otherwise I'll let someone more knowledgeable reply.

[saket]

I believe this has been fixed by #1164. Wanna verify this by using 1.3.2-SNAPSHOT?

[cbruegg]

@saket Right, #1164 looks promising! I’ll try it immediately after the holidays and will update this issue. Thanks for the quick feedback!

[cbruegg]

@saket Indeed 1.3.2-SNAPSHOT has resolved this issue. Many thanks again! I'll close this ticket.