[jvm] NullPointerException in SnapshotSystemJUnit5

Empieza por SnapshotSystemJUnit5.kt en forClass y por los callers de SelfieExtension.kt y SelfieTestExecutionListener.kt. Inspecciona cómo se actualiza el mapa atómico progressPerClass y reproduce la NullPointerException dependiente del orden de las pruebas, ya que no se proporciona un reproducer sencillo. Se considera terminado cuando se explique o se prevenga el caso faltante de SnapshotFileProgress, con cobertura para el escenario intermitente de JUnit5.

HI everyone! Thanks for the amazing project, it really makes jvm testing fun/easy.

When using Junit5, I'm intermittently getting the following NullPointerException:

    java.lang.NullPointerException
        at com.diffplug.selfie.junit5.SnapshotSystemJUnit5.forClass(SnapshotSystemJUnit5.kt:77)
        at com.diffplug.selfie.junit5.SelfieTestExecutionListener.executionStarted(SelfieTestExecutionListener.kt:33)

When investigating, I noticed that it's linked to the order of execution for the tests. In my case, only a single test in the suite uses selfie.

Digging deeper, I've found that the following patch solves the issue for my particular use case:

diff --git a/jvm/selfie-runner-junit5/src/main/kotlin/com/diffplug/selfie/junit5/SelfieExtension.kt b/jvm/selfie-runner-junit5/src/main/kotlin/com/diffplug/selfie/junit5/SelfieExtension.kt
index aed9aee4..dcc5f391 100644
--- a/jvm/selfie-runner-junit5/src/main/kotlin/com/diffplug/selfie/junit5/SelfieExtension.kt
+++ b/jvm/selfie-runner-junit5/src/main/kotlin/com/diffplug/selfie/junit5/SelfieExtension.kt
@@ -36,13 +36,13 @@ import kotlinx.coroutines.withContext
 class SelfieExtension(projectConfig: AbstractProjectConfig) :
     Extension, BeforeSpecListener, TestCaseExtension, FinalizeSpecListener, AfterProjectListener {
   override suspend fun beforeSpec(spec: Spec) {
-    SnapshotSystemJUnit5.forClass(spec.javaClass.name).incrementContainers()
+    SnapshotSystemJUnit5.forClass(spec.javaClass.name)?.incrementContainers()
   }
   override suspend fun intercept(
       testCase: TestCase,
       execute: suspend (TestCase) -> TestResult
   ): TestResult {
-    val file = SnapshotSystemJUnit5.forClass(testCase.spec::class.java.name)
+    val file = SnapshotSystemJUnit5.forClass(testCase.spec::class.java.name)!!
     val coroutineLocal = CoroutineDiskStorage(DiskStorageJUnit5(file, testCase.name.testName))
     return withContext(currentCoroutineContext() + coroutineLocal) {
       file.startTest(testCase.name.testName, false)
@@ -58,12 +58,12 @@ class SelfieExtension(projectConfig: AbstractProjectConfig) :
     val file = SnapshotSystemJUnit5.forClass(kclass.java.name)
     results.entries.forEach {
       if (it.value.isIgnored) {
-        file.startTest(it.key.name.testName, false)
-        file.finishedTestWithSuccess(it.key.name.testName, false, false)
+        file?.startTest(it.key.name.testName, false)
+        file?.finishedTestWithSuccess(it.key.name.testName, false, false)
       }
     }
-    SnapshotSystemJUnit5.forClass(kclass.java.name)
-        .decrementContainersWithSuccess(results.values.all { it.isSuccess })
+      SnapshotSystemJUnit5.forClass(kclass.java.name)
+          ?.decrementContainersWithSuccess(results.values.all { it.isSuccess })
   }
   override suspend fun afterProject() {
     // If you run from the CLI, `SelfieTestExecutionListener` will run and so will `afterProject`
diff --git a/jvm/selfie-runner-junit5/src/main/kotlin/com/diffplug/selfie/junit5/SelfieTestExecutionListener.kt b/jvm/selfie-runner-junit5/src/main/kotlin/com/diffplug/selfie/junit5/SelfieTestExecutionListener.kt
index b6cbe3fb..1e5c0a97 100644
--- a/jvm/selfie-runner-junit5/src/main/kotlin/com/diffplug/selfie/junit5/SelfieTestExecutionListener.kt
+++ b/jvm/selfie-runner-junit5/src/main/kotlin/com/diffplug/selfie/junit5/SelfieTestExecutionListener.kt
@@ -32,9 +32,9 @@ class SelfieTestExecutionListener : TestExecutionListener {
       val (clazz, test) = parseClassTest(testIdentifier)
       val snapshotFile = system.forClass(clazz)
       if (test == null) {
-        snapshotFile.incrementContainers()
+        snapshotFile?.incrementContainers()
       } else {
-        system.forClass(clazz).startTest(test, true)
+        system.forClass(clazz)?.startTest(test, true)
       }
     } catch (e: Throwable) {
       system.layout.smuggledError.set(e)
@@ -44,8 +44,8 @@ class SelfieTestExecutionListener : TestExecutionListener {
     try {
       val (clazz, test) = parseClassTest(testIdentifier)
       if (test == null) {
-        system.forClass(clazz).incrementContainers()
-        system.forClass(clazz).decrementContainersWithSuccess(false)
+        system.forClass(clazz)?.incrementContainers()
+        system.forClass(clazz)?.decrementContainersWithSuccess(false)
       } else {
         // TODO: using reflection right now, but we should probably listen to these
       }
@@ -63,9 +63,9 @@ class SelfieTestExecutionListener : TestExecutionListener {
       val isSuccess = testExecutionResult.status == TestExecutionResult.Status.SUCCESSFUL
       val snapshotFile = system.forClass(clazz)
       if (test == null) {
-        snapshotFile.decrementContainersWithSuccess(isSuccess)
+        snapshotFile?.decrementContainersWithSuccess(isSuccess)
       } else {
-        snapshotFile.finishedTestWithSuccess(test, true, isSuccess)
+        snapshotFile?.finishedTestWithSuccess(test, true, isSuccess)
       }
     } catch (e: Throwable) {
       system.layout.smuggledError.set(e)
diff --git a/jvm/selfie-runner-junit5/src/main/kotlin/com/diffplug/selfie/junit5/SnapshotSystemJUnit5.kt b/jvm/selfie-runner-junit5/src/main/kotlin/com/diffplug/selfie/junit5/SnapshotSystemJUnit5.kt
index a0358d0d..1e4b6aa8 100644
--- a/jvm/selfie-runner-junit5/src/main/kotlin/com/diffplug/selfie/junit5/SnapshotSystemJUnit5.kt
+++ b/jvm/selfie-runner-junit5/src/main/kotlin/com/diffplug/selfie/junit5/SnapshotSystemJUnit5.kt
@@ -68,7 +68,7 @@ internal object SnapshotSystemJUnit5 : SnapshotSystem {
   private val inlineWriteTracker = InlineWriteTracker()
   private val toBeFileWriteTracker = ToBeFileWriteTracker()
   private val progressPerClass = atomic(ArrayMap.empty<String, SnapshotFileProgress>())
-  fun forClass(className: String): SnapshotFileProgress {
+  fun forClass(className: String): SnapshotFileProgress? {
     // optimize for reads
     progressPerClass.get()[className]?.let {
       return it
@@ -76,7 +76,7 @@ internal object SnapshotSystemJUnit5 : SnapshotSystem {
     // sometimes we'll have to write
     return progressPerClass
         .updateAndGet { it.plusOrNoOp(className, SnapshotFileProgress(this, className)) }[
-            className]!!
+            className]
   }
   private val checkForInvalidStale = atomic<ArraySet<TypedPath>?>(ArraySet.empty())
   internal fun markPathAsWritten(typedPath: TypedPath) {

This makes SnapshotSystemJUnit5.forClass return SnapshotFileProgress? instead of SnapshotFileProgress and updates references.

I couldn't come up with a simple reproducer yet, but my question is: should this happen at all? progressPerClass is an atomic reference to an ArrayMap which should always contain className (thanks to the plusOrNoOp call)