[jvm] NullPointerException in SnapshotSystemJUnit5

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
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())
- 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.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)