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 =

jvm/selfie-runner-junit5/src/test/kotlin/com/diffplug/selfie/junit5/DuplicateWriteToBeFileTest.kt

@@ -0,0 +1,69 @@
+/*
+ * Copyright (C) 2023-2024 DiffPlug
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package com.diffplug.selfie.junit5
+
+import io.kotest.matchers.shouldBe
+import io.kotest.matchers.string.shouldStartWith
+import kotlin.test.Test
+import org.junit.jupiter.api.MethodOrderer
+import org.junit.jupiter.api.Order
+import org.junit.jupiter.api.TestMethodOrder
+import org.junitpioneer.jupiter.DisableIfTestFails
+
+/** Simplest test for verifying read/write of a snapshot. */
+@TestMethodOrder(MethodOrderer.OrderAnnotation::class)
+@DisableIfTestFails
+class DuplicateWriteToBeFileTest : HarnessJUnit() {
+  @Test @Order(1)
+  fun noSelfie() {
+    ut_snapshot().deleteIfExists()
+    ut_snapshot().assertDoesNotExist()
+  }
+
+  @Test @Order(2)
+  fun cannot_write_multiple_things_to_one_snapshot() {
+    ut_mirrorKt().linesFrom("fun shouldFail()").toFirst("}").uncomment()
+    ut_mirrorKt().linesFrom("fun shouldPass()").toFirst("}").commentOut()
+    gradlew("test", "-PunderTest=true", "-Pselfie=overwrite")!!.message shouldStartWith
+        "Snapshot was set to multiple values"
+  }
+
+  @Test @Order(3)
+  fun can_write_one_thing_multiple_times_to_one_snapshot() {
+    ut_mirrorKt().linesFrom("fun shouldFail()").toFirst("}").commentOut()
+    ut_mirrorKt().linesFrom("fun shouldPass()").toFirst("}").uncomment()
+    gradlew("test", "-PunderTest=true", "-Pselfie=overwrite") shouldBe null
+  }
+
+  @Test @Order(4)
+  fun can_read_one_thing_multiple_times_from_one_snapshot() {
+    ut_mirrorKt().linesFrom("fun shouldFail()").toFirst("}").commentOut()
+    ut_mirrorKt().linesFrom("fun shouldPass()").toFirst("}").uncomment()
+    gradlew("test", "-PunderTest=true", "-Pselfie=readonly") shouldBe null
+  }
+
+  @Test @Order(5)
+  fun writeonce_mode() {
+    ut_mirrorKt().linesFrom("fun shouldFail()").toFirst("}").commentOut()
+    ut_mirrorKt().linesFrom("fun shouldPass()").toFirst("}").uncomment()
+    gradlew(
+            "test",
+            "-PunderTest=true",
+            "-Pselfie=overwrite",
+            "-Pselfie.settings=undertest.junit5.SelfieWriteOnce")!!
+        .message shouldStartWith "Snapshot was set to the same value multiple times"
+  }
+}

jvm/selfie-runner-kotest/src/commonMain/kotlin/com/diffplug/selfie/kotest/FSOkio.kt

@@ -28,6 +28,8 @@ expect internal val FS_SYSTEM: FileSystem
 internal fun TypedPath.toPath(): okio.Path = absolutePath.toPath()
 
 internal object FSOkio : FS {
+  override fun fileExists(typedPath: TypedPath): Boolean =
+      FS_SYSTEM.metadataOrNull(typedPath.toPath())?.isRegularFile ?: false
   /** Walks the files (not directories) which are children and grandchildren of the given path. */
   override fun <T> fileWalk(typedPath: TypedPath, walk: (Sequence<TypedPath>) -> T): T =
       walk(

jvm/selfie-runner-kotest/src/commonMain/kotlin/com/diffplug/selfie/kotest/SnapshotSystemKotest.kt

@@ -30,6 +30,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
@@ -40,6 +41,7 @@ internal class SnapshotSystemKotest(settings: SelfieSettingsAPI) : SnapshotSyste
   override val layout = SnapshotFileLayoutKotest(settings, fs)
   private val commentTracker = CommentTracker()
   private val inlineWriteTracker = InlineWriteTracker()
+  private val toBeFileWriteTracker = ToBeFileWriteTracker()
   private val progressPerFile = atomic(ArrayMap.empty<String, SnapshotFileProgress>())
   fun forClassOrFilename(classOrFilename: String): SnapshotFileProgress {
     // optimize for reads
@@ -66,6 +68,9 @@ internal class SnapshotSystemKotest(settings: SelfieSettingsAPI) : SnapshotSyste
   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)
+  }
   override fun diskThreadLocal(): DiskStorage =
       throw fs.assertFailed(
           """

jvm/settings.gradle

@@ -18,13 +18,13 @@ plugins {
   // https://github.com/gradle-nexus/publish-plugin/releases
   id 'io.github.gradle-nexus.publish-plugin' version '2.0.0-rc-2' apply false
   // https://plugins.gradle.org/plugin/org.jetbrains.dokka
-  id 'org.jetbrains.dokka'                       version '1.9.10' apply false
+  id 'org.jetbrains.dokka'                       version '1.9.20' apply false
   // https://plugins.gradle.org/plugin/org.jetbrains.kotlin.jvm
-  id 'org.jetbrains.kotlin.jvm'                  version '1.9.22' apply false
+  id 'org.jetbrains.kotlin.jvm'                  version '1.9.23' apply false
   // https://plugins.gradle.org/plugin/org.jetbrains.kotlin.plugin.serialization
-  id 'org.jetbrains.kotlin.plugin.serialization' version '1.9.22' apply false
+  id 'org.jetbrains.kotlin.plugin.serialization' version '1.9.23' apply false
   // https://plugins.gradle.org/plugin/org.jetbrains.kotlin.multiplatform
-  id 'org.jetbrains.kotlin.multiplatform' version '1.9.22' apply false
+  id 'org.jetbrains.kotlin.multiplatform' version '1.9.23' apply false
   // https://github.com/adamko-dev/dokkatoo/releases
   id 'dev.adamko.dokkatoo-html' version '2.0.0' apply false
 }

jvm/undertest-junit5-kotest/harness/settings.gradle

@@ -18,13 +18,13 @@ plugins {
     // https://github.com/gradle-nexus/publish-plugin/releases
     id 'io.github.gradle-nexus.publish-plugin' version '2.0.0-rc-2' apply false
     // https://plugins.gradle.org/plugin/org.jetbrains.dokka
-    id 'org.jetbrains.dokka'                       version '1.9.10' apply false
+    id 'org.jetbrains.dokka'                       version '1.9.20' apply false
     // https://plugins.gradle.org/plugin/org.jetbrains.kotlin.jvm
-    id 'org.jetbrains.kotlin.jvm'                  version '1.9.22' apply false
+    id 'org.jetbrains.kotlin.jvm'                  version '1.9.23' apply false
     // https://plugins.gradle.org/plugin/org.jetbrains.kotlin.plugin.serialization
-    id 'org.jetbrains.kotlin.plugin.serialization' version '1.9.22' apply false
+    id 'org.jetbrains.kotlin.plugin.serialization' version '1.9.23' apply false
     // https://plugins.gradle.org/plugin/org.jetbrains.kotlin.multiplatform
-    id 'org.jetbrains.kotlin.multiplatform' version '1.9.22' apply false
+    id 'org.jetbrains.kotlin.multiplatform' version '1.9.23' apply false
     // https://github.com/adamko-dev/dokkatoo/releases
     id 'dev.adamko.dokkatoo-html' version '2.0.0' apply false
 }

jvm/undertest-junit5/src/test/java/duplicate_tobefile.data

@@ -0,0 +1 @@
+twins

jvm/undertest-junit5/src/test/kotlin/undertest/junit5/UT_DuplicateWriteToBeFileTest.kt

@@ -0,0 +1,15 @@
+package undertest.junit5
+
+import com.diffplug.selfie.Selfie.expectSelfie
+import org.junit.jupiter.api.Test
+
+class UT_DuplicateWriteToBeFileTest {
+//  @Test fun shouldFail() {
+//    expectSelfie("apples".toByteArray()).toBeFile("duplicate_tobefile.data")
+//    expectSelfie("oranges".toByteArray()).toBeFile("duplicate_tobefile.data")
+//  }
+  @Test fun shouldPass() {
+    expectSelfie("twins".toByteArray()).toBeFile("duplicate_tobefile.data")
+    expectSelfie("twins".toByteArray()).toBeFile("duplicate_tobefile.data")
+  }
+}