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 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) } 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..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,13 +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 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) { @@ -70,11 +72,19 @@ enum class Mode { when (this) { interactive -> "$headline\n" + - (if (headline.startsWith("Snapshot ")) - "‣ update this snapshot by adding `_TODO` to the function name\n" - else "") + + "‣ 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/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 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..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 @@ -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 = "»" @@ -28,73 +31,98 @@ 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) } + private val state: State + + internal sealed class State { + class Read(val frames: List>) : State() { + fun currentFrameThenAdvance(): Int = cf.getAndUpdate { it + 1 } + fun framesReadSoFar(): Int = cf.get() + private val cf = atomic(0) + } - private class State(val readMode: Boolean) { - var currentFrame = 0 - val frames = mutableListOf>() + class Write : State() { + private val lock = reentrantLock() + private var frames: MutableList>? = + mutableListOf>() + fun add(key: String, value: SnapshotValue) { + lock.withLock { + frames?.apply { + val idx = size + 1 + add(entry("$OPEN$idx$CLOSE$key", value)) + } ?: throw IllegalStateException("This VCR was already closed.") + } + } + 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) - state = State(readMode = !canWrite) - if (state.readMode) { + if (canWrite) { + state = State.Write() + } else { val snapshot = disk.readDisk(sub, call) - ?: throw Selfie.system.fs.assertFailed(Selfie.system.mode.msgSnapshotNotFound()) + ?: throw Selfie.system.fs.assertFailed( + Selfie.system.mode.msgVcrSnapshotNotFound(call)) var idx = 1 + val frames = mutableListOf>() for ((key, value) in snapshot.facets) { check(key.startsWith(OPEN)) 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) + frames.add(keyAfterNum to value) } + state = State.Read(frames) } } override fun close() { - if (state.readMode) { - if (state.frames.size != state.currentFrame) { + if (state is State.Read) { + 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(), call)) } } else { - var snapshot = Snapshot.of("") - var idx = 1 - for ((key, value) in state.frames) { - snapshot = snapshot.plusFacet("$OPEN$idx$CLOSE$key", value) - } - disk.writeDisk(snapshot, sub, call) + disk.writeDisk((state as State.Write).closeAndGetSnapshot(), 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) { - throw fs.assertFailed(mode.msgVcrUnderflow(state.frames.size)) + val currentFrame = state.currentFrameThenAdvance() + if (state.frames.size <= currentFrame) { + throw fs.assertFailed(mode.msgVcrUnderflow(state.frames.size, call), call) } - 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, call), expected.first, key) } 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 } } @@ -107,11 +135,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 } } 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 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 {