From 0000bf0a3e3bfea9884b505d8cad13c27902c8d6 Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Mon, 3 Mar 2025 22:13:01 -0800 Subject: [PATCH 01/17] Adjust the sort order in ArrayMap to sort (1,2,...,9,10,11) instead of (1,11,2,...). --- .../kotlin/com/diffplug/selfie/ArrayMap.kt | 44 ++++++++++++++++--- 1 file changed, 38 insertions(+), 6 deletions(-) diff --git a/jvm/selfie-lib/src/commonMain/kotlin/com/diffplug/selfie/ArrayMap.kt b/jvm/selfie-lib/src/commonMain/kotlin/com/diffplug/selfie/ArrayMap.kt index 621fbd26..e5699a32 100644 --- a/jvm/selfie-lib/src/commonMain/kotlin/com/diffplug/selfie/ArrayMap.kt +++ b/jvm/selfie-lib/src/commonMain/kotlin/com/diffplug/selfie/ArrayMap.kt @@ -1,5 +1,5 @@ /* - * Copyright (C) 2023-2024 DiffPlug + * Copyright (C) 2023-2025 DiffPlug * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -23,16 +23,48 @@ private val STRING_SLASHFIRST = val charA = a[i] val charB = b[i] if (charA != charB) { - return@Comparator ( // treat - if (charA == '/') -1 // slash as - else if (charB == '/') 1 // the lowest - else charA.compareTo(charB) // character - ) + // Check for slash first as it's special + if (charA == '/') return@Comparator -1 // treat slash as the lowest character + if (charB == '/') return@Comparator 1 + + // Check for embedded numbers + if (charA.isDigit() && charB.isDigit()) { + // Extract the complete numbers from both strings + val numA = extractNumber(a, i) + val numB = extractNumber(b, i) + + // Compare the numeric values + val numCompare = numA.first.compareTo(numB.first) + if (numCompare != 0) return@Comparator numCompare + + // If the numbers are equal, adjust index to after the numbers + // and continue comparing the rest of the string + i = + maxOf(numA.second, numB.second) - + 1 // -1 because we increment i at the end of the loop + } else { + // Regular character comparison + return@Comparator charA.compareTo(charB) + } } i++ } a.length.compareTo(b.length) } + +/** + * Extracts a numeric substring starting at the given index. + * + * @return Pair of (numeric value, index after the last digit) + */ +private fun extractNumber(s: String, startIndex: Int): Pair { + var endIndex = startIndex + while (endIndex < s.length && s[endIndex].isDigit()) { + endIndex++ + } + val number = s.substring(startIndex, endIndex).toInt() + return Pair(number, endIndex) +} private val PAIR_STRING_SLASHFIRST = Comparator> { a, b -> STRING_SLASHFIRST.compare(a.first, b.first) } From 819fa1e037a411742c7eb92df5ada9725d07fc36 Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Mon, 3 Mar 2025 22:27:29 -0800 Subject: [PATCH 02/17] Increment the index after each frame is taken. --- .../src/commonMain/kotlin/com/diffplug/selfie/VcrSelfie.kt | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/jvm/selfie-lib/src/commonMain/kotlin/com/diffplug/selfie/VcrSelfie.kt b/jvm/selfie-lib/src/commonMain/kotlin/com/diffplug/selfie/VcrSelfie.kt index 3de60f0e..dd1b0a31 100644 --- a/jvm/selfie-lib/src/commonMain/kotlin/com/diffplug/selfie/VcrSelfie.kt +++ b/jvm/selfie-lib/src/commonMain/kotlin/com/diffplug/selfie/VcrSelfie.kt @@ -52,7 +52,7 @@ internal constructor( val nextClose = key.indexOf(CLOSE) check(nextClose != -1) val num = key.substring(OPEN.length, nextClose).toInt() - check(num == idx) + check(num == idx) { "expected $idx in $key" } ++idx val keyAfterNum = key.substring(nextClose + 1) state.frames.add(keyAfterNum to value) @@ -70,6 +70,7 @@ internal constructor( var idx = 1 for ((key, value) in state.frames) { snapshot = snapshot.plusFacet("$OPEN$idx$CLOSE$key", value) + ++idx } disk.writeDisk(snapshot, sub, call) } From 7dc03aa948823ad2ad4ca23f9961eb2282bcc788 Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Tue, 4 Mar 2025 09:15:38 -0800 Subject: [PATCH 03/17] Give VCR its own "msgSnapshotNotFound" message. --- .../src/commonMain/kotlin/com/diffplug/selfie/Mode.kt | 1 + .../src/commonMain/kotlin/com/diffplug/selfie/VcrSelfie.kt | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/jvm/selfie-lib/src/commonMain/kotlin/com/diffplug/selfie/Mode.kt b/jvm/selfie-lib/src/commonMain/kotlin/com/diffplug/selfie/Mode.kt index 8b60b431..952c32c8 100644 --- a/jvm/selfie-lib/src/commonMain/kotlin/com/diffplug/selfie/Mode.kt +++ b/jvm/selfie-lib/src/commonMain/kotlin/com/diffplug/selfie/Mode.kt @@ -47,6 +47,7 @@ enum class Mode { msg("Snapshot " + SnapshotNotEqualErrorMsg.forUnequalStrings(expected, actual)) internal fun msgSnapshotMismatchBinary(expected: ByteArray, actual: ByteArray) = msgSnapshotMismatch(expected.toQuotedPrintable(), actual.toQuotedPrintable()) + internal fun msgVcrSnapshotNotFound() = msg("VCR snapshot not found") internal fun msgVcrMismatch(key: String, expected: String, actual: String) = msg("VCR frame $key " + SnapshotNotEqualErrorMsg.forUnequalStrings(expected, actual)) internal fun msgVcrUnread(expected: Int, actual: Int) = diff --git a/jvm/selfie-lib/src/commonMain/kotlin/com/diffplug/selfie/VcrSelfie.kt b/jvm/selfie-lib/src/commonMain/kotlin/com/diffplug/selfie/VcrSelfie.kt index dd1b0a31..c2a70ea2 100644 --- a/jvm/selfie-lib/src/commonMain/kotlin/com/diffplug/selfie/VcrSelfie.kt +++ b/jvm/selfie-lib/src/commonMain/kotlin/com/diffplug/selfie/VcrSelfie.kt @@ -45,7 +45,7 @@ internal constructor( if (state.readMode) { val snapshot = disk.readDisk(sub, call) - ?: throw Selfie.system.fs.assertFailed(Selfie.system.mode.msgSnapshotNotFound()) + ?: throw Selfie.system.fs.assertFailed(Selfie.system.mode.msgVcrSnapshotNotFound()) var idx = 1 for ((key, value) in snapshot.facets) { check(key.startsWith(OPEN)) From 32c97979e2a911017bc6a4146fb63f9294c6bc29 Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Tue, 4 Mar 2025 09:16:50 -0800 Subject: [PATCH 04/17] Give an API to allow creating VCR selfie without a "TestLocator". --- .../src/commonMain/kotlin/com/diffplug/selfie/Selfie.kt | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/jvm/selfie-lib/src/commonMain/kotlin/com/diffplug/selfie/Selfie.kt b/jvm/selfie-lib/src/commonMain/kotlin/com/diffplug/selfie/Selfie.kt index 7b40efca..12906591 100644 --- a/jvm/selfie-lib/src/commonMain/kotlin/com/diffplug/selfie/Selfie.kt +++ b/jvm/selfie-lib/src/commonMain/kotlin/com/diffplug/selfie/Selfie.kt @@ -19,6 +19,7 @@ import com.diffplug.selfie.guts.CallStack import com.diffplug.selfie.guts.DiskStorage import com.diffplug.selfie.guts.SnapshotSystem import com.diffplug.selfie.guts.initSnapshotSystem +import com.diffplug.selfie.guts.recordCall import kotlin.jvm.JvmStatic /** A getter which may or may not be run. */ @@ -97,6 +98,10 @@ object Selfie { @JvmStatic @ExperimentalSelfieVcr fun vcrTestLocator(sub: String = "") = VcrSelfie.TestLocator(sub, deferredDiskStorage) + + @JvmStatic + @ExperimentalSelfieVcr + fun vcrSelfie(sub: String = "") = VcrSelfie(sub, recordCall(false), deferredDiskStorage) } @RequiresOptIn( From 33d8a762c8ce2e6375c73615995d721dbb7aee38 Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Tue, 4 Mar 2025 10:03:01 -0800 Subject: [PATCH 05/17] Extract VcrSelfie's state into separate read and write for mutability control. --- .../kotlin/com/diffplug/selfie/VcrSelfie.kt | 56 ++++++++++++------- 1 file changed, 35 insertions(+), 21 deletions(-) diff --git a/jvm/selfie-lib/src/commonMain/kotlin/com/diffplug/selfie/VcrSelfie.kt b/jvm/selfie-lib/src/commonMain/kotlin/com/diffplug/selfie/VcrSelfie.kt index c2a70ea2..0c6f2ddb 100644 --- a/jvm/selfie-lib/src/commonMain/kotlin/com/diffplug/selfie/VcrSelfie.kt +++ b/jvm/selfie-lib/src/commonMain/kotlin/com/diffplug/selfie/VcrSelfie.kt @@ -33,20 +33,37 @@ internal constructor( fun createVcr() = VcrSelfie(sub, call, disk) } - private class State(val readMode: Boolean) { - var currentFrame = 0 - val frames = mutableListOf>() + internal sealed class State { + class Read(val frames: List>) : State() { + var currentFrame = 0 + } + + class Write : State() { + private val frames = mutableListOf>() + fun add(key: String, value: SnapshotValue) { + frames.add(key to value) + } + fun toSnapshot(): Snapshot { + var snapshot = Snapshot.of("") + var idx = 1 + for ((key, value) in frames) { + snapshot = snapshot.plusFacet("$OPEN$idx$CLOSE$key", value) + ++idx + } + return snapshot + } + } } private val state: State init { val canWrite = Selfie.system.mode.canWrite(isTodo = false, call, Selfie.system) - state = State(readMode = !canWrite) - if (state.readMode) { + if (canWrite) { val snapshot = disk.readDisk(sub, call) ?: throw Selfie.system.fs.assertFailed(Selfie.system.mode.msgVcrSnapshotNotFound()) var idx = 1 + val frames = mutableListOf>() for ((key, value) in snapshot.facets) { check(key.startsWith(OPEN)) val nextClose = key.indexOf(CLOSE) @@ -55,27 +72,24 @@ internal constructor( check(num == idx) { "expected $idx in $key" } ++idx val keyAfterNum = key.substring(nextClose + 1) - state.frames.add(keyAfterNum to value) + frames.add(keyAfterNum to value) } + state = State.Read(frames) + } else { + state = State.Write() } } override fun close() { - if (state.readMode) { + if (state is State.Read) { if (state.frames.size != state.currentFrame) { throw Selfie.system.fs.assertFailed( Selfie.system.mode.msgVcrUnread(state.frames.size, state.currentFrame)) } } else { - var snapshot = Snapshot.of("") - var idx = 1 - for ((key, value) in state.frames) { - snapshot = snapshot.plusFacet("$OPEN$idx$CLOSE$key", value) - ++idx - } - disk.writeDisk(snapshot, sub, call) + disk.writeDisk((state as State.Write).toSnapshot(), sub, call) } } - private fun nextFrameValue(key: String): SnapshotValue { + private fun nextFrameValue(state: State.Read, key: String): SnapshotValue { val mode = Selfie.system.mode val fs = Selfie.system.fs if (state.frames.size <= state.currentFrame) { @@ -91,11 +105,11 @@ internal constructor( return expected.second } fun nextFrame(key: String, roundtripValue: Roundtrip, value: Cacheable): V { - if (state.readMode) { - return roundtripValue.parse(nextFrameValue(key).valueString()) + if (state is State.Read) { + return roundtripValue.parse(nextFrameValue(state, key).valueString()) } else { val value = value.get() - state.frames.add(key to SnapshotValue.of(roundtripValue.serialize(value))) + (state as State.Write).add(key, SnapshotValue.of(roundtripValue.serialize(value))) return value } } @@ -108,11 +122,11 @@ internal constructor( roundtripValue: Roundtrip, value: Cacheable ): V { - if (state.readMode) { - return roundtripValue.parse(nextFrameValue(key).valueBinary()) + if (state is State.Read) { + return roundtripValue.parse(nextFrameValue(state, key).valueBinary()) } else { val value = value.get() - state.frames.add(key to SnapshotValue.of(roundtripValue.serialize(value))) + (state as State.Write).add(key, SnapshotValue.of(roundtripValue.serialize(value))) return value } } From 623be54afd94231c9ba16d0d516cd5bc769a59db Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Tue, 4 Mar 2025 10:34:54 -0800 Subject: [PATCH 06/17] Make VcrSelfie concurrency safe. --- .../kotlin/com/diffplug/selfie/VcrSelfie.kt | 45 +++++++++++-------- 1 file changed, 27 insertions(+), 18 deletions(-) diff --git a/jvm/selfie-lib/src/commonMain/kotlin/com/diffplug/selfie/VcrSelfie.kt b/jvm/selfie-lib/src/commonMain/kotlin/com/diffplug/selfie/VcrSelfie.kt index 0c6f2ddb..9ddd95f6 100644 --- a/jvm/selfie-lib/src/commonMain/kotlin/com/diffplug/selfie/VcrSelfie.kt +++ b/jvm/selfie-lib/src/commonMain/kotlin/com/diffplug/selfie/VcrSelfie.kt @@ -17,7 +17,10 @@ package com.diffplug.selfie import com.diffplug.selfie.guts.CallStack import com.diffplug.selfie.guts.DiskStorage +import com.diffplug.selfie.guts.atomic import com.diffplug.selfie.guts.recordCall +import com.diffplug.selfie.guts.reentrantLock +import com.diffplug.selfie.guts.withLock private const val OPEN = "«" private const val CLOSE = "»" @@ -32,29 +35,34 @@ internal constructor( private val call = recordCall(false) fun createVcr() = VcrSelfie(sub, call, disk) } + private val state: State internal sealed class State { class Read(val frames: List>) : State() { - var currentFrame = 0 + fun currentFrameThenAdvance(): Int = cf.getAndUpdate { it + 1 } + fun framesReadSoFar(): Int = cf.get() + private val cf = atomic(0) } class Write : State() { - private val frames = mutableListOf>() + private val lock = reentrantLock() + private var frames: MutableList>? = + mutableListOf>() fun add(key: String, value: SnapshotValue) { - frames.add(key to value) - } - fun toSnapshot(): Snapshot { - var snapshot = Snapshot.of("") - var idx = 1 - for ((key, value) in frames) { - snapshot = snapshot.plusFacet("$OPEN$idx$CLOSE$key", value) - ++idx + lock.withLock { + frames?.add(entry(key, value)) + ?: throw IllegalStateException("This VCR was already closed.") } - return snapshot } + fun closeAndGetSnapshot(): Snapshot = + Snapshot.ofEntries( + lock.withLock { + val frozen = frames ?: throw IllegalStateException("This VCR was already closed.") + frames = null + frozen + }) } } - private val state: State init { val canWrite = Selfie.system.mode.canWrite(isTodo = false, call, Selfie.system) @@ -81,24 +89,25 @@ internal constructor( } override fun close() { if (state is State.Read) { - if (state.frames.size != state.currentFrame) { + if (state.frames.size != state.framesReadSoFar()) { throw Selfie.system.fs.assertFailed( - Selfie.system.mode.msgVcrUnread(state.frames.size, state.currentFrame)) + Selfie.system.mode.msgVcrUnread(state.frames.size, state.framesReadSoFar())) } } else { - disk.writeDisk((state as State.Write).toSnapshot(), sub, call) + disk.writeDisk((state as State.Write).closeAndGetSnapshot(), sub, call) } } private fun nextFrameValue(state: State.Read, key: String): SnapshotValue { val mode = Selfie.system.mode val fs = Selfie.system.fs - if (state.frames.size <= state.currentFrame) { + val currentFrame = state.currentFrameThenAdvance() + if (state.frames.size <= currentFrame) { throw fs.assertFailed(mode.msgVcrUnderflow(state.frames.size)) } - val expected = state.frames[state.currentFrame++] + val expected = state.frames[currentFrame] if (expected.first != key) { throw fs.assertFailed( - mode.msgVcrMismatch("$sub[$OPEN${state.currentFrame}$CLOSE]", expected.first, key), + mode.msgVcrMismatch("$sub[$OPEN${currentFrame}$CLOSE]", expected.first, key), expected.first, key) } From 2d04e250d6c333cc05278ab8874e98ba35eccf00 Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Tue, 4 Mar 2025 10:35:20 -0800 Subject: [PATCH 07/17] Simplify unused parts of the ReentrantLock KMP interface. --- .../kotlin/com/diffplug/selfie/guts/AtomicFuBroken.kt | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/jvm/selfie-lib/src/commonMain/kotlin/com/diffplug/selfie/guts/AtomicFuBroken.kt b/jvm/selfie-lib/src/commonMain/kotlin/com/diffplug/selfie/guts/AtomicFuBroken.kt index c856acac..40b21c4c 100644 --- a/jvm/selfie-lib/src/commonMain/kotlin/com/diffplug/selfie/guts/AtomicFuBroken.kt +++ b/jvm/selfie-lib/src/commonMain/kotlin/com/diffplug/selfie/guts/AtomicFuBroken.kt @@ -1,5 +1,5 @@ /* - * Copyright (C) 2024 DiffPlug + * Copyright (C) 2024-2025 DiffPlug * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -24,12 +24,8 @@ expect class AtomicRef { /** Replace with atomicfu when stable. */ expect fun atomic(initial: T): AtomicRef -expect fun reentrantLock(): ReentrantLock +expect class ReentrantLock -expect class ReentrantLock { - fun lock(): Unit - fun tryLock(): Boolean - fun unlock(): Unit -} +expect fun reentrantLock(): ReentrantLock expect inline fun ReentrantLock.withLock(block: () -> T): T From 780912bf5bf2b3e5a06d723afc6f2646dc065d63 Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Tue, 4 Mar 2025 10:39:26 -0800 Subject: [PATCH 08/17] Remove VcrSelfie.TestLocator --- .../src/commonMain/kotlin/com/diffplug/selfie/Selfie.kt | 4 ---- .../src/commonMain/kotlin/com/diffplug/selfie/VcrSelfie.kt | 5 ----- 2 files changed, 9 deletions(-) diff --git a/jvm/selfie-lib/src/commonMain/kotlin/com/diffplug/selfie/Selfie.kt b/jvm/selfie-lib/src/commonMain/kotlin/com/diffplug/selfie/Selfie.kt index 12906591..e0e2b888 100644 --- a/jvm/selfie-lib/src/commonMain/kotlin/com/diffplug/selfie/Selfie.kt +++ b/jvm/selfie-lib/src/commonMain/kotlin/com/diffplug/selfie/Selfie.kt @@ -95,10 +95,6 @@ object Selfie { * control whether the VCR is writing or reading. If the caller lives in a package called * `selfie.*` it will keep looking up the stack trace until a caller is not inside `selfie.*`. */ - @JvmStatic - @ExperimentalSelfieVcr - fun vcrTestLocator(sub: String = "") = VcrSelfie.TestLocator(sub, deferredDiskStorage) - @JvmStatic @ExperimentalSelfieVcr fun vcrSelfie(sub: String = "") = VcrSelfie(sub, recordCall(false), deferredDiskStorage) diff --git a/jvm/selfie-lib/src/commonMain/kotlin/com/diffplug/selfie/VcrSelfie.kt b/jvm/selfie-lib/src/commonMain/kotlin/com/diffplug/selfie/VcrSelfie.kt index 9ddd95f6..10aa59c6 100644 --- a/jvm/selfie-lib/src/commonMain/kotlin/com/diffplug/selfie/VcrSelfie.kt +++ b/jvm/selfie-lib/src/commonMain/kotlin/com/diffplug/selfie/VcrSelfie.kt @@ -18,7 +18,6 @@ package com.diffplug.selfie import com.diffplug.selfie.guts.CallStack import com.diffplug.selfie.guts.DiskStorage import com.diffplug.selfie.guts.atomic -import com.diffplug.selfie.guts.recordCall import com.diffplug.selfie.guts.reentrantLock import com.diffplug.selfie.guts.withLock @@ -31,10 +30,6 @@ internal constructor( private val call: CallStack, private val disk: DiskStorage, ) : AutoCloseable { - class TestLocator internal constructor(private val sub: String, private val disk: DiskStorage) { - private val call = recordCall(false) - fun createVcr() = VcrSelfie(sub, call, disk) - } private val state: State internal sealed class State { From 24f1432600783e9b4b8635a75915333191d07bec Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Tue, 4 Mar 2025 10:41:37 -0800 Subject: [PATCH 09/17] Pull the `recordCall` to where the other selfie types do it. --- .../src/commonMain/kotlin/com/diffplug/selfie/Selfie.kt | 3 +-- .../src/commonMain/kotlin/com/diffplug/selfie/VcrSelfie.kt | 3 ++- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/jvm/selfie-lib/src/commonMain/kotlin/com/diffplug/selfie/Selfie.kt b/jvm/selfie-lib/src/commonMain/kotlin/com/diffplug/selfie/Selfie.kt index e0e2b888..beabe7e1 100644 --- a/jvm/selfie-lib/src/commonMain/kotlin/com/diffplug/selfie/Selfie.kt +++ b/jvm/selfie-lib/src/commonMain/kotlin/com/diffplug/selfie/Selfie.kt @@ -19,7 +19,6 @@ import com.diffplug.selfie.guts.CallStack import com.diffplug.selfie.guts.DiskStorage import com.diffplug.selfie.guts.SnapshotSystem import com.diffplug.selfie.guts.initSnapshotSystem -import com.diffplug.selfie.guts.recordCall import kotlin.jvm.JvmStatic /** A getter which may or may not be run. */ @@ -97,7 +96,7 @@ object Selfie { */ @JvmStatic @ExperimentalSelfieVcr - fun vcrSelfie(sub: String = "") = VcrSelfie(sub, recordCall(false), deferredDiskStorage) + fun vcrSelfie(sub: String = "") = VcrSelfie(sub, deferredDiskStorage) } @RequiresOptIn( diff --git a/jvm/selfie-lib/src/commonMain/kotlin/com/diffplug/selfie/VcrSelfie.kt b/jvm/selfie-lib/src/commonMain/kotlin/com/diffplug/selfie/VcrSelfie.kt index 10aa59c6..8d4bdbee 100644 --- a/jvm/selfie-lib/src/commonMain/kotlin/com/diffplug/selfie/VcrSelfie.kt +++ b/jvm/selfie-lib/src/commonMain/kotlin/com/diffplug/selfie/VcrSelfie.kt @@ -18,6 +18,7 @@ package com.diffplug.selfie import com.diffplug.selfie.guts.CallStack import com.diffplug.selfie.guts.DiskStorage import com.diffplug.selfie.guts.atomic +import com.diffplug.selfie.guts.recordCall import com.diffplug.selfie.guts.reentrantLock import com.diffplug.selfie.guts.withLock @@ -27,9 +28,9 @@ private const val CLOSE = "»" class VcrSelfie internal constructor( private val sub: String, - private val call: CallStack, private val disk: DiskStorage, ) : AutoCloseable { + private val call: CallStack = recordCall(false) private val state: State internal sealed class State { From d726d4f4d24e78df722c4aa0b39b80c435ae0e27 Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Tue, 4 Mar 2025 12:25:57 -0800 Subject: [PATCH 10/17] Oops, broke the indices again. --- .../src/commonMain/kotlin/com/diffplug/selfie/VcrSelfie.kt | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/jvm/selfie-lib/src/commonMain/kotlin/com/diffplug/selfie/VcrSelfie.kt b/jvm/selfie-lib/src/commonMain/kotlin/com/diffplug/selfie/VcrSelfie.kt index 8d4bdbee..f7b3dbd5 100644 --- a/jvm/selfie-lib/src/commonMain/kotlin/com/diffplug/selfie/VcrSelfie.kt +++ b/jvm/selfie-lib/src/commonMain/kotlin/com/diffplug/selfie/VcrSelfie.kt @@ -46,8 +46,10 @@ internal constructor( mutableListOf>() fun add(key: String, value: SnapshotValue) { lock.withLock { - frames?.add(entry(key, value)) - ?: throw IllegalStateException("This VCR was already closed.") + frames?.apply { + val idx = size + 1 + add(entry("$OPEN$idx$CLOSE$key", value)) + } ?: throw IllegalStateException("This VCR was already closed.") } } fun closeAndGetSnapshot(): Snapshot = From 8b9bcacb6622f0e1f70be9d4c4124776cc67188a Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Tue, 4 Mar 2025 12:40:54 -0800 Subject: [PATCH 11/17] Better error messages for VCR not finding comment. --- .../src/commonMain/kotlin/com/diffplug/selfie/Mode.kt | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/jvm/selfie-lib/src/commonMain/kotlin/com/diffplug/selfie/Mode.kt b/jvm/selfie-lib/src/commonMain/kotlin/com/diffplug/selfie/Mode.kt index 952c32c8..b07992c0 100644 --- a/jvm/selfie-lib/src/commonMain/kotlin/com/diffplug/selfie/Mode.kt +++ b/jvm/selfie-lib/src/commonMain/kotlin/com/diffplug/selfie/Mode.kt @@ -72,9 +72,11 @@ enum class Mode { interactive -> "$headline\n" + (if (headline.startsWith("Snapshot ")) - "‣ update this snapshot by adding `_TODO` to the function name\n" - else "") + - "‣ update all snapshots in this file by adding `//selfieonce` or `//SELFIEWRITE`" + "‣ update this snapshot by adding `_TODO` to the function name\n" + + "‣ update all snapshots in this file by adding `//selfieonce` or `//SELFIEWRITE`" + else + "‣ update all snapshots in this file by adding `//selfieonce` or `//SELFIEWRITE`\n" + + "‣ if that doesn't work remember to put your test rule into the `selfie` package") readonly -> headline overwrite -> "$headline\n(didn't expect this to ever happen in overwrite mode)" } From 78db66d3315255158ac6a7184148bcae3cddf286 Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Tue, 4 Mar 2025 12:41:03 -0800 Subject: [PATCH 12/17] Fix boneheaded if. --- .../src/commonMain/kotlin/com/diffplug/selfie/VcrSelfie.kt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/jvm/selfie-lib/src/commonMain/kotlin/com/diffplug/selfie/VcrSelfie.kt b/jvm/selfie-lib/src/commonMain/kotlin/com/diffplug/selfie/VcrSelfie.kt index f7b3dbd5..59658e22 100644 --- a/jvm/selfie-lib/src/commonMain/kotlin/com/diffplug/selfie/VcrSelfie.kt +++ b/jvm/selfie-lib/src/commonMain/kotlin/com/diffplug/selfie/VcrSelfie.kt @@ -65,6 +65,8 @@ internal constructor( init { val canWrite = Selfie.system.mode.canWrite(isTodo = false, call, Selfie.system) if (canWrite) { + state = State.Write() + } else { val snapshot = disk.readDisk(sub, call) ?: throw Selfie.system.fs.assertFailed(Selfie.system.mode.msgVcrSnapshotNotFound()) @@ -81,8 +83,6 @@ internal constructor( frames.add(keyAfterNum to value) } state = State.Read(frames) - } else { - state = State.Write() } } override fun close() { From e0c7265b62170fe145aa5fffe4529d2c8a984670 Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Tue, 4 Mar 2025 12:49:13 -0800 Subject: [PATCH 13/17] Turns out we need `TestLocator` still. --- .../src/commonMain/kotlin/com/diffplug/selfie/Selfie.kt | 2 +- .../src/commonMain/kotlin/com/diffplug/selfie/VcrSelfie.kt | 6 +++++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/jvm/selfie-lib/src/commonMain/kotlin/com/diffplug/selfie/Selfie.kt b/jvm/selfie-lib/src/commonMain/kotlin/com/diffplug/selfie/Selfie.kt index beabe7e1..7b40efca 100644 --- a/jvm/selfie-lib/src/commonMain/kotlin/com/diffplug/selfie/Selfie.kt +++ b/jvm/selfie-lib/src/commonMain/kotlin/com/diffplug/selfie/Selfie.kt @@ -96,7 +96,7 @@ object Selfie { */ @JvmStatic @ExperimentalSelfieVcr - fun vcrSelfie(sub: String = "") = VcrSelfie(sub, deferredDiskStorage) + fun vcrTestLocator(sub: String = "") = VcrSelfie.TestLocator(sub, deferredDiskStorage) } @RequiresOptIn( diff --git a/jvm/selfie-lib/src/commonMain/kotlin/com/diffplug/selfie/VcrSelfie.kt b/jvm/selfie-lib/src/commonMain/kotlin/com/diffplug/selfie/VcrSelfie.kt index 59658e22..3719120c 100644 --- a/jvm/selfie-lib/src/commonMain/kotlin/com/diffplug/selfie/VcrSelfie.kt +++ b/jvm/selfie-lib/src/commonMain/kotlin/com/diffplug/selfie/VcrSelfie.kt @@ -28,9 +28,13 @@ private const val CLOSE = "»" class VcrSelfie internal constructor( private val sub: String, + private val call: CallStack, private val disk: DiskStorage, ) : AutoCloseable { - private val call: CallStack = recordCall(false) + class TestLocator internal constructor(private val sub: String, private val disk: DiskStorage) { + private val call = recordCall(false) + fun createVcr() = VcrSelfie(sub, call, disk) + } private val state: State internal sealed class State { From 8aec163b343e7b60a119fdb024ae0328fbef54e3 Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Tue, 4 Mar 2025 13:09:32 -0800 Subject: [PATCH 14/17] Big improvement to VCR error messages. --- .../kotlin/com/diffplug/selfie/Mode.kt | 35 +++++++++++-------- .../kotlin/com/diffplug/selfie/VcrSelfie.kt | 10 +++--- 2 files changed, 27 insertions(+), 18 deletions(-) diff --git a/jvm/selfie-lib/src/commonMain/kotlin/com/diffplug/selfie/Mode.kt b/jvm/selfie-lib/src/commonMain/kotlin/com/diffplug/selfie/Mode.kt index b07992c0..e77c6e88 100644 --- a/jvm/selfie-lib/src/commonMain/kotlin/com/diffplug/selfie/Mode.kt +++ b/jvm/selfie-lib/src/commonMain/kotlin/com/diffplug/selfie/Mode.kt @@ -47,14 +47,15 @@ enum class Mode { msg("Snapshot " + SnapshotNotEqualErrorMsg.forUnequalStrings(expected, actual)) internal fun msgSnapshotMismatchBinary(expected: ByteArray, actual: ByteArray) = msgSnapshotMismatch(expected.toQuotedPrintable(), actual.toQuotedPrintable()) - internal fun msgVcrSnapshotNotFound() = msg("VCR snapshot not found") - internal fun msgVcrMismatch(key: String, expected: String, actual: String) = - msg("VCR frame $key " + SnapshotNotEqualErrorMsg.forUnequalStrings(expected, actual)) - internal fun msgVcrUnread(expected: Int, actual: Int) = - msg("VCR frames unread - only $actual were read out of $expected") - internal fun msgVcrUnderflow(expected: Int) = - msg( - "VCR frames exhausted - only $expected are available but you tried to read ${expected + 1}") + internal fun msgVcrSnapshotNotFound(call: CallStack) = msgVcr("VCR snapshot not found", call) + internal fun msgVcrMismatch(key: String, expected: String, actual: String, call: CallStack) = + msgVcr("VCR frame $key " + SnapshotNotEqualErrorMsg.forUnequalStrings(expected, actual), call) + internal fun msgVcrUnread(expected: Int, actual: Int, call: CallStack) = + msgVcr("VCR frames unread - only $actual were read out of $expected", call) + internal fun msgVcrUnderflow(expected: Int, call: CallStack) = + msgVcr( + "VCR frames exhausted - only $expected are available but you tried to read ${expected + 1}", + call) private fun ByteArray.toQuotedPrintable(): String { val sb = StringBuilder() for (byte in this) { @@ -71,12 +72,18 @@ enum class Mode { when (this) { interactive -> "$headline\n" + - (if (headline.startsWith("Snapshot ")) - "‣ update this snapshot by adding `_TODO` to the function name\n" + - "‣ update all snapshots in this file by adding `//selfieonce` or `//SELFIEWRITE`" - else - "‣ update all snapshots in this file by adding `//selfieonce` or `//SELFIEWRITE`\n" + - "‣ if that doesn't work remember to put your test rule into the `selfie` package") + "‣ update this snapshot by adding `_TODO` to the function name\n" + + "‣ update all snapshots in this file by adding `//selfieonce` or `//SELFIEWRITE`" + readonly -> headline + overwrite -> "$headline\n(didn't expect this to ever happen in overwrite mode)" + } + private fun msgVcr(headline: String, call: CallStack) = + when (this) { + interactive -> + "$headline\n" + + "‣ update all snapshots in this file by adding `//selfieonce` or `//SELFIEWRITE`\n" + + "‣ could not find control comment in ${call.location.ideLink(Selfie.system.layout)}\n" + + "‣ remember to call `Selfie.vcrTestLocator()` in the test itself, or put the file above into the `selfie` package to mark that it is not the test file" readonly -> headline overwrite -> "$headline\n(didn't expect this to ever happen in overwrite mode)" } diff --git a/jvm/selfie-lib/src/commonMain/kotlin/com/diffplug/selfie/VcrSelfie.kt b/jvm/selfie-lib/src/commonMain/kotlin/com/diffplug/selfie/VcrSelfie.kt index 3719120c..a254a57d 100644 --- a/jvm/selfie-lib/src/commonMain/kotlin/com/diffplug/selfie/VcrSelfie.kt +++ b/jvm/selfie-lib/src/commonMain/kotlin/com/diffplug/selfie/VcrSelfie.kt @@ -31,6 +31,7 @@ internal constructor( private val call: CallStack, private val disk: DiskStorage, ) : AutoCloseable { + /** Creates VCRs who store their data based on where this TestLocator was created. */ class TestLocator internal constructor(private val sub: String, private val disk: DiskStorage) { private val call = recordCall(false) fun createVcr() = VcrSelfie(sub, call, disk) @@ -73,7 +74,8 @@ internal constructor( } else { val snapshot = disk.readDisk(sub, call) - ?: throw Selfie.system.fs.assertFailed(Selfie.system.mode.msgVcrSnapshotNotFound()) + ?: throw Selfie.system.fs.assertFailed( + Selfie.system.mode.msgVcrSnapshotNotFound(call)) var idx = 1 val frames = mutableListOf>() for ((key, value) in snapshot.facets) { @@ -93,7 +95,7 @@ internal constructor( if (state is State.Read) { if (state.frames.size != state.framesReadSoFar()) { throw Selfie.system.fs.assertFailed( - Selfie.system.mode.msgVcrUnread(state.frames.size, state.framesReadSoFar())) + Selfie.system.mode.msgVcrUnread(state.frames.size, state.framesReadSoFar(), call)) } } else { disk.writeDisk((state as State.Write).closeAndGetSnapshot(), sub, call) @@ -104,12 +106,12 @@ internal constructor( val fs = Selfie.system.fs val currentFrame = state.currentFrameThenAdvance() if (state.frames.size <= currentFrame) { - throw fs.assertFailed(mode.msgVcrUnderflow(state.frames.size)) + throw fs.assertFailed(mode.msgVcrUnderflow(state.frames.size, call), call) } val expected = state.frames[currentFrame] if (expected.first != key) { throw fs.assertFailed( - mode.msgVcrMismatch("$sub[$OPEN${currentFrame}$CLOSE]", expected.first, key), + mode.msgVcrMismatch("$sub[$OPEN${currentFrame}$CLOSE]", expected.first, key, call), expected.first, key) } From b98b2eeabb40ea589045b3d50c74733027cda2d0 Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Tue, 4 Mar 2025 13:32:23 -0800 Subject: [PATCH 15/17] Fix warnings. --- .../com/diffplug/selfie/guts/AtomicFuBroken.js.kt | 11 +++-------- .../com/diffplug/selfie/guts/AtomicFuBroken.jvm.kt | 4 ++-- 2 files changed, 5 insertions(+), 10 deletions(-) diff --git a/jvm/selfie-lib/src/jsMain/kotlin/com/diffplug/selfie/guts/AtomicFuBroken.js.kt b/jvm/selfie-lib/src/jsMain/kotlin/com/diffplug/selfie/guts/AtomicFuBroken.js.kt index 2ecce4e3..b03607cf 100644 --- a/jvm/selfie-lib/src/jsMain/kotlin/com/diffplug/selfie/guts/AtomicFuBroken.js.kt +++ b/jvm/selfie-lib/src/jsMain/kotlin/com/diffplug/selfie/guts/AtomicFuBroken.js.kt @@ -1,5 +1,5 @@ /* - * Copyright (C) 2024 DiffPlug + * Copyright (C) 2024-2025 DiffPlug * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -29,12 +29,7 @@ actual class AtomicRef(private var value: T) { } } val Lock = ReentrantLock() -actual inline fun reentrantLock() = Lock +actual fun reentrantLock() = Lock -@Suppress("NOTHING_TO_INLINE") -actual class ReentrantLock { - actual inline fun lock(): Unit {} - actual inline fun tryLock() = true - actual inline fun unlock(): Unit {} -} +@Suppress("NOTHING_TO_INLINE") actual class ReentrantLock actual inline fun ReentrantLock.withLock(block: () -> T) = block() diff --git a/jvm/selfie-lib/src/jvmMain/kotlin/com/diffplug/selfie/guts/AtomicFuBroken.jvm.kt b/jvm/selfie-lib/src/jvmMain/kotlin/com/diffplug/selfie/guts/AtomicFuBroken.jvm.kt index 7084443a..21c35bd2 100644 --- a/jvm/selfie-lib/src/jvmMain/kotlin/com/diffplug/selfie/guts/AtomicFuBroken.jvm.kt +++ b/jvm/selfie-lib/src/jvmMain/kotlin/com/diffplug/selfie/guts/AtomicFuBroken.jvm.kt @@ -1,5 +1,5 @@ /* - * Copyright (C) 2024 DiffPlug + * Copyright (C) 2024-2025 DiffPlug * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -24,7 +24,7 @@ actual class AtomicRef(value: T) { actual fun updateAndGet(update: (T) -> T): T = ref.updateAndGet(update) actual fun getAndUpdate(update: (T) -> T) = ref.getAndUpdate(update) } -actual inline fun reentrantLock() = ReentrantLock() +actual fun reentrantLock() = ReentrantLock() actual typealias ReentrantLock = java.util.concurrent.locks.ReentrantLock actual inline fun ReentrantLock.withLock(block: () -> T): T { From 2d97ea57a3b64ad8984991253040aa171ab58452 Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Tue, 4 Mar 2025 13:32:47 -0800 Subject: [PATCH 16/17] Remove `RequiresOptIn`. --- .../commonMain/kotlin/com/diffplug/selfie/Selfie.kt | 11 +---------- 1 file changed, 1 insertion(+), 10 deletions(-) diff --git a/jvm/selfie-lib/src/commonMain/kotlin/com/diffplug/selfie/Selfie.kt b/jvm/selfie-lib/src/commonMain/kotlin/com/diffplug/selfie/Selfie.kt index 7b40efca..a13179aa 100644 --- a/jvm/selfie-lib/src/commonMain/kotlin/com/diffplug/selfie/Selfie.kt +++ b/jvm/selfie-lib/src/commonMain/kotlin/com/diffplug/selfie/Selfie.kt @@ -94,14 +94,5 @@ object Selfie { * control whether the VCR is writing or reading. If the caller lives in a package called * `selfie.*` it will keep looking up the stack trace until a caller is not inside `selfie.*`. */ - @JvmStatic - @ExperimentalSelfieVcr - fun vcrTestLocator(sub: String = "") = VcrSelfie.TestLocator(sub, deferredDiskStorage) + @JvmStatic fun vcrTestLocator(sub: String = "") = VcrSelfie.TestLocator(sub, deferredDiskStorage) } - -@RequiresOptIn( - level = RequiresOptIn.Level.WARNING, - message = "This API is in beta and may change in the future.") -@Retention(AnnotationRetention.BINARY) -@Target(AnnotationTarget.CLASS, AnnotationTarget.FUNCTION, AnnotationTarget.PROPERTY) -annotation class ExperimentalSelfieVcr From 35921b07c239d4ef98e005a7b1b9c4498c701fc5 Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Tue, 4 Mar 2025 14:49:58 -0800 Subject: [PATCH 17/17] Update changelog. --- jvm/CHANGELOG.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/jvm/CHANGELOG.md b/jvm/CHANGELOG.md index ff4186d3..db5dc75e 100644 --- a/jvm/CHANGELOG.md +++ b/jvm/CHANGELOG.md @@ -11,6 +11,11 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). ## [Unreleased] +### Fixed +- Selfie VCR is now out of beta, no opt-in required. ([#525](https://github.com/diffplug/selfie/pull/525)) + - ArrayMap now sorts strings with multi-digit numbers as `1 2 ... 9 10 11` instead of `1 11 2 ...`. + - Improved VCR-specific error messages for determining why `//selfieonce` might not be working for a test rule. + - Fixed some bugs in VCR data storage (specifically concurrency and multiple frames). ## [2.5.0] - 2025-02-21 ### Added