[jvm] NullPointerException in SnapshotSystemJUnit5

Bắt đầu với SnapshotSystemJUnit5.kt tại forClass và các callers trong SelfieExtension.kt và SelfieTestExecutionListener.kt. Kiểm tra cách map nguyên tử progressPerClass được cập nhật và tái hiện NullPointerException phụ thuộc vào thứ tự test, vì không có reproducer đơn giản nào được cung cấp. Được xem là hoàn thành khi trường hợp SnapshotFileProgress bị thiếu được giải thích hoặc ngăn chặn, kèm coverage cho kịch bản JUnit5 không ổn định.

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)