Merge branch 'main' into SnapshotValueReader-Edwin

Author: nedtwigg

jvm/CHANGELOG.md

@@ -12,6 +12,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+## [2.0.2] - 2024-03-20
+### Fixed
+- `toBeFile` now checks for duplicate writes and throws a more helpful error message if the file doesn't exist. ([#277](https://github.com/diffplug/selfie/pull/277))
+
 ## [2.0.1] - 2024-02-24
 ### Fixed
 - The `coroutines` methods used to eagerly throw an exception if they were ever called from anywhere besides a Kotest method. Now they wait until `toMatchDisk()` is called, because they can work just fine anywhere if you use `toBe`. ([#247](https://github.com/diffplug/selfie/pull/247))

jvm/example-junit5/build.gradle

@@ -5,10 +5,10 @@ repositories {
   mavenCentral()
 }
 dependencies {
-  implementation 'io.jooby:jooby:3.0.7'
-  implementation 'io.jooby:jooby-netty:3.0.7'
-  implementation 'jakarta.mail:jakarta.mail-api:2.1.2'
-  testImplementation 'io.jooby:jooby-test:3.0.7'
+  implementation 'io.jooby:jooby:3.0.9'
+  implementation 'io.jooby:jooby-netty:3.0.9'
+  implementation 'jakarta.mail:jakarta.mail-api:2.1.3'
+  testImplementation 'io.jooby:jooby-test:3.0.9'
   testImplementation "org.junit.jupiter:junit-jupiter:$ver_JUNIT_USE"
   testRuntimeOnly 'org.junit.platform:junit-platform-launcher'
   testImplementation 'io.rest-assured:rest-assured:5.4.0'

jvm/gradle.properties

@@ -7,7 +7,7 @@ org.gradle.configuration-cache=true
 ver_JUNIT_USE=5.10.2
 ver_JUNIT_PIONEER=2.2.0
 ver_OKIO=3.8.0
-ver_KOTLIN_TEST=1.9.22
+ver_KOTLIN_TEST=1.9.23
 ver_KOTLIN_SERIALIZATION=1.6.3
 # Kotest 5.4.0 is the oldest that we support
 ver_KOTEST=5.4.0

jvm/selfie-lib/src/commonMain/kotlin/com/diffplug/selfie/CacheSelfieBinary.kt

@@ -74,13 +74,18 @@ class CacheSelfieBinary<T>(
       if (isTodo) {
         Selfie.system.writeInline(TodoStub.toBeFile.createLiteral(), call)
       }
-      Selfie.system.fs.fileWriteBinary(resolvePath(subpath), roundtrip.serialize(actual))
+      Selfie.system.writeToBeFile(resolvePath(subpath), roundtrip.serialize(actual), call)
       return actual
     } else {
       if (isTodo) {
         throw Selfie.system.fs.assertFailed("Can't call `toBeFile_TODO` in ${Mode.readonly} mode!")
       } else {
-        return roundtrip.parse(Selfie.system.fs.fileReadBinary(resolvePath(subpath)))
+        val path = resolvePath(subpath)
+        if (!Selfie.system.fs.fileExists(path)) {
+          throw Selfie.system.fs.assertFailed(
+              Selfie.system.mode.msgSnapshotNotFoundNoSuchFile(path))
+        }
+        return roundtrip.parse(Selfie.system.fs.fileReadBinary(path))
       }
     }
   }

jvm/selfie-lib/src/commonMain/kotlin/com/diffplug/selfie/Mode.kt

@@ -18,6 +18,7 @@ package com.diffplug.selfie
 import com.diffplug.selfie.guts.CallStack
 import com.diffplug.selfie.guts.CommentTracker
 import com.diffplug.selfie.guts.SnapshotSystem
+import com.diffplug.selfie.guts.TypedPath
 
 enum class Mode {
   interactive,
@@ -39,6 +40,8 @@ enum class Mode {
         overwrite -> true
       }
   internal fun msgSnapshotNotFound() = msg("Snapshot not found")
+  internal fun msgSnapshotNotFoundNoSuchFile(file: TypedPath) =
+      msg("Snapshot not found: no such file $file")
   internal fun msgSnapshotMismatch() = msg("Snapshot mismatch")
   private fun msg(headline: String) =
       when (this) {

jvm/selfie-lib/src/commonMain/kotlin/com/diffplug/selfie/SelfieImplementations.kt

@@ -132,13 +132,18 @@ class BinarySelfie(actual: Snapshot, disk: DiskStorage, private val onlyFacet: S
       if (isTodo) {
         Selfie.system.writeInline(TodoStub.toBeFile.createLiteral(), call)
       }
-      Selfie.system.fs.fileWriteBinary(resolvePath(subpath), actualBytes)
+      Selfie.system.writeToBeFile(resolvePath(subpath), actualBytes, call)
       return actualBytes
     } else {
       if (isTodo) {
         throw Selfie.system.fs.assertFailed("Can't call `toBeFile_TODO` in ${Mode.readonly} mode!")
       } else {
-        val expected = Selfie.system.fs.fileReadBinary(resolvePath(subpath))
+        val path = resolvePath(subpath)
+        if (!Selfie.system.fs.fileExists(path)) {
+          throw Selfie.system.fs.assertFailed(
+              Selfie.system.mode.msgSnapshotNotFoundNoSuchFile(path))
+        }
+        val expected = Selfie.system.fs.fileReadBinary(path)
         if (expected.contentEquals(actualBytes)) {
           return actualBytes
         } else {

jvm/selfie-lib/src/commonMain/kotlin/com/diffplug/selfie/coroutines/CacheSelfieBinarySuspend.kt

@@ -78,13 +78,18 @@ class CacheSelfieBinarySuspend<T>(
       if (isTodo) {
         Selfie.system.writeInline(TodoStub.toBeFile.createLiteral(), call)
       }
-      Selfie.system.fs.fileWriteBinary(resolvePath(subpath), roundtrip.serialize(actual))
+      Selfie.system.writeToBeFile(resolvePath(subpath), roundtrip.serialize(actual), call)
       return actual
     } else {
       if (isTodo) {
         throw Selfie.system.fs.assertFailed("Can't call `toBeFile_TODO` in ${Mode.readonly} mode!")
       } else {
-        return roundtrip.parse(Selfie.system.fs.fileReadBinary(resolvePath(subpath)))
+        val path = resolvePath(subpath)
+        if (!Selfie.system.fs.fileExists(path)) {
+          throw Selfie.system.fs.assertFailed(
+              Selfie.system.mode.msgSnapshotNotFoundNoSuchFile(path))
+        }
+        return roundtrip.parse(Selfie.system.fs.fileReadBinary(path))
       }
     }
   }

jvm/selfie-lib/src/commonMain/kotlin/com/diffplug/selfie/guts/SnapshotSystem.kt

@@ -80,6 +80,11 @@ data class TypedPath(val absolutePath: String) : Comparable<TypedPath> {
 }
 
 interface FS {
+  /**
+   * Returns true if the given path exists *and is a file*, false if it doesn't or if it is a
+   * folder.
+   */
+  fun fileExists(typedPath: TypedPath): Boolean
   /** Walks the files (not directories) which are children and grandchildren of the given path. */
   fun <T> fileWalk(typedPath: TypedPath, walk: (Sequence<TypedPath>) -> T): T
   fun fileRead(typedPath: TypedPath) = fileReadBinary(typedPath).decodeToString()
@@ -100,6 +105,8 @@ interface SnapshotSystem {
   fun sourceFileHasWritableComment(call: CallStack): Boolean
   /** Indicates that the following value should be written into test sourcecode. */
   fun writeInline(literalValue: LiteralValue<*>, call: CallStack)
+  /** Writes the given bytes to the given file, checking for duplicate writes. */
+  fun writeToBeFile(path: TypedPath, data: ByteArray, call: CallStack)
   /** Returns the DiskStorage for the test associated with this thread, else error. */
   fun diskThreadLocal(): DiskStorage
 }

jvm/selfie-lib/src/commonMain/kotlin/com/diffplug/selfie/guts/WriteTracker.kt

@@ -80,7 +80,8 @@ sealed class WriteTracker<K : Comparable<K>, V> {
               expectSelfie(underTest).toBe("bash$")
             }
         """
-                  .trimIndent()
+          is ToBeFileWriteTracker ->
+              "You can fix this with `.toBeFile(String filename)` and pass a unique filename for each code path."
         }
     if (existing.snapshot != snapshot) {
       throw layout.fs.assertFailed(
@@ -102,6 +103,46 @@ class DiskWriteTracker : WriteTracker<String, Snapshot>() {
   }
 }
 
+class ToBeFileWriteTracker : WriteTracker<TypedPath, ToBeFileLazyBytes>() {
+  fun writeToDisk(
+      key: TypedPath,
+      snapshot: ByteArray,
+      call: CallStack,
+      layout: SnapshotFileLayout
+  ) {
+    val lazyBytes = ToBeFileLazyBytes(key, layout, snapshot)
+    recordInternal(key, lazyBytes, call, layout)
+    // recordInternal will throw an exception on a duplicate write, so we can safely write to disk
+    lazyBytes.writeToDisk()
+    // and because we are doing duplicate checks, `ToBeFileLazyBytes` can allow its in-memory
+    // data to be garbage collected, because it can safely read from disk in the future
+  }
+}
+
+class ToBeFileLazyBytes(val location: TypedPath, val layout: SnapshotFileLayout, data: ByteArray) {
+  /** When constructed, we always have the data. */
+  var data: ByteArray? = data
+  /**
+   * Shortly after being construted, this data is written to disk, and we can stop holding it in
+   * memory.
+   */
+  internal fun writeToDisk() {
+    data?.let { layout.fs.fileWriteBinary(location, it) }
+        ?: throw IllegalStateException("Data has already been written to disk!")
+    data = null
+  }
+  /**
+   * If we need to read our data, we do it from memory if it's still there, or from disk if it
+   * isn't.
+   */
+  private fun readData(): ByteArray = data ?: layout.fs.fileReadBinary(location)
+  /** We calculate equality based on this data. */
+  override fun equals(other: Any?): Boolean =
+      if (this === other) true
+      else if (other is ToBeFileLazyBytes) readData().contentEquals(other.readData()) else false
+  override fun hashCode(): Int = readData().contentHashCode()
+}
+
 class InlineWriteTracker : WriteTracker<CallLocation, LiteralValue<*>>() {
   fun record(call: CallStack, literalValue: LiteralValue<*>, layout: SnapshotFileLayout) {
     recordInternal(call.location, literalValue, call, layout)

jvm/selfie-runner-junit5/src/main/kotlin/com/diffplug/selfie/junit5/SnapshotSystemJUnit5.kt

@@ -26,6 +26,7 @@ import com.diffplug.selfie.guts.LiteralValue
 import com.diffplug.selfie.guts.SnapshotFileLayout
 import com.diffplug.selfie.guts.SnapshotSystem
 import com.diffplug.selfie.guts.SourceFile
+import com.diffplug.selfie.guts.ToBeFileWriteTracker
 import com.diffplug.selfie.guts.TypedPath
 import com.diffplug.selfie.guts.WithinTestGC
 import com.diffplug.selfie.guts.atomic
@@ -41,6 +42,7 @@ import org.opentest4j.AssertionFailedError
 internal fun TypedPath.toPath(): java.nio.file.Path = java.nio.file.Path.of(absolutePath)
 
 internal object FSJava : FS {
+  override fun fileExists(typedPath: TypedPath): Boolean = Files.isRegularFile(typedPath.toPath())
   override fun fileWriteBinary(typedPath: TypedPath, content: ByteArray) =
       typedPath.toPath().writeBytes(content)
   override fun fileReadBinary(typedPath: TypedPath) = typedPath.toPath().readBytes()
@@ -65,6 +67,7 @@ internal object SnapshotSystemJUnit5 : SnapshotSystem {
   override val layout = SnapshotFileLayoutJUnit5(SelfieSettingsAPI.initialize(), fs)
   private val commentTracker = CommentTracker()
   private val inlineWriteTracker = InlineWriteTracker()
+  private val toBeFileWriteTracker = ToBeFileWriteTracker()
   private val progressPerClass = atomic(ArrayMap.empty<String, SnapshotFileProgress>())
   fun forClass(className: String): SnapshotFileProgress {
     // optimize for reads
@@ -90,6 +93,9 @@ internal object SnapshotSystemJUnit5 : SnapshotSystem {
   override fun writeInline(literalValue: LiteralValue<*>, call: CallStack) {
     inlineWriteTracker.record(call, literalValue, layout)
   }
+  override fun writeToBeFile(path: TypedPath, data: ByteArray, call: CallStack) {
+    toBeFileWriteTracker.writeToDisk(path, data, call, layout)
+  }
   internal val testListenerRunning = AtomicBoolean(false)
   fun finishedAllTests() {
     val snapshotsFilesWrittenToDisk =