cashapp / paparazzi

Render your Android screens without a physical device or emulator
https://cashapp.github.io/paparazzi/
Apache License 2.0
2.25k stars 213 forks source link

Second and subsequent tests are falsely succeed if error occur during rendering #1279

Closed DmitryKolpo closed 6 months ago

DmitryKolpo commented 7 months ago

Description Create 2 or more snapshot tests using Compose. If 2nd and subsequent tests throw exception inside SubcomposeLayout, test gonna succeed instead of fail. Exception is logged, but test not failed. Empty screenshots are generated.

Same behavior for view that uses SubcomposeLayout under the hood(LazyList, Scaffold).

Steps to Reproduce

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

    @Test fun test1() { paparazzi.snapshot { ComposeContent() } }
    @Test fun test2() { paparazzi.snapshot { ComposeContent() } }
    @Test fun test3() { paparazzi.snapshot { ComposeContent() } }

    @Composable
    private fun ComposeContent() {
        SubcomposeLayout { throw Exception() }
    }
}

Screenshot 2024-02-02 at 18 20 26

Expected behavior All tests are failed, if there are exceptions during execution.

Additional information:

Screenshots

My research The problem is how Renderer is instantiated. Renderer is static, but logger is not.

    if (!isInitialized) {
      renderer = Renderer(environment, layoutlibCallback, logger)
      sessionParamsBuilder = renderer.prepare()
    }

Launch first test. Paparazzi instance is created. Local private PaparazziLogger is created. This logger is passed inside static Renderer. Then second send is launched. New Paparazzi instance is created. New local private PaparazziLogger is created. But renderer is not going to recreate and new logger is not passed. So Renderer has old logger. Then, when exception is thrown during i.e. measure, View log error inside renderer and old PaparazziLogger collect this errors.

//PaparazziLogger
override fun error(
    throwable: Throwable?,
    format: String?,
    vararg args: Any
  ) {
    logger.log(Level.SEVERE, format?.format(args), throwable) //here is logger from first test, not from second
    if (throwable != null) {
      errors += throwable
    }
  }

But in the end Paparazzi gonna check if there are errors in old logger

//Paparazzi
override fun apply(
...
        logger.assertNoErrors() //this is new logger, and here we gonna check if there are errors
...
//PaparazziLogger
fun assertNoErrors() {
    when (errors.size) {
      0 -> return
      1 -> throw errors[0]
      else -> throw MultipleFailuresException(errors)
    }
  }

And find nothing, because all errors are in old PaparazziLogger. No errors - no fail.

Maybe Renderer should not be static and should be recreated. Or can be still static but new logger should be passed inside Renderer.