From 9fa1065929acd9b33eedfea55a1fc0b66d4e1b35 Mon Sep 17 00:00:00 2001 From: Edgar Espina Date: Mon, 18 Sep 2023 10:17:40 -0300 Subject: [PATCH 01/15] WIP: Int Selfie - Attemp to implement int selfie --- .../src/main/kotlin/com/diffplug/selfie/Selfie.kt | 10 ++++++++-- .../test/kotlin/undertest/junit5/UT_IntSelfieTest.kt | 12 ++++++++++++ .../junit5/__snapshots__/UT_IntSelfieTest.ss | 3 +++ 3 files changed, 23 insertions(+), 2 deletions(-) create mode 100644 undertest-junit5/src/test/kotlin/undertest/junit5/UT_IntSelfieTest.kt create mode 100644 undertest-junit5/src/test/kotlin/undertest/junit5/__snapshots__/UT_IntSelfieTest.ss diff --git a/selfie-runner-junit5/src/main/kotlin/com/diffplug/selfie/Selfie.kt b/selfie-runner-junit5/src/main/kotlin/com/diffplug/selfie/Selfie.kt index 993ec0a5..7d620e20 100644 --- a/selfie-runner-junit5/src/main/kotlin/com/diffplug/selfie/Selfie.kt +++ b/selfie-runner-junit5/src/main/kotlin/com/diffplug/selfie/Selfie.kt @@ -16,6 +16,7 @@ package com.diffplug.selfie import com.diffplug.selfie.junit5.Router +import org.junit.jupiter.api.Assertions.assertEquals import org.opentest4j.AssertionFailedError /** @@ -68,8 +69,13 @@ class BinarySelfie(private val actual: ByteArray) : DiskSelfie(Snapshot.of(actua } fun expectSelfie(actual: ByteArray) = BinarySelfie(actual) -class IntSelfie(private val actual: Int) { - fun toBe(expected: Int): Int = TODO() +class IntSelfie(private val actual: Int) : DiskSelfie(Snapshot.of(actual.toString())) { + infix fun toBe(expected: Int): Int { + // TODO: Is this right? + val snapshot = toMatchDisk() + assertEquals(expected, snapshot.value.valueString().toInt()) + return expected + } fun toBe_TODO(): Int = TODO() } fun expectSelfie(actual: Int) = IntSelfie(actual) diff --git a/undertest-junit5/src/test/kotlin/undertest/junit5/UT_IntSelfieTest.kt b/undertest-junit5/src/test/kotlin/undertest/junit5/UT_IntSelfieTest.kt new file mode 100644 index 00000000..6a600a95 --- /dev/null +++ b/undertest-junit5/src/test/kotlin/undertest/junit5/UT_IntSelfieTest.kt @@ -0,0 +1,12 @@ +package undertest.junit5 +// spotless:off +import com.diffplug.selfie.expectSelfie +import kotlin.test.Test + +// spotless:on + +class UT_IntSelfieTest { + @Test fun singleInt() { + expectSelfie(678) toBe 678 + } +} diff --git a/undertest-junit5/src/test/kotlin/undertest/junit5/__snapshots__/UT_IntSelfieTest.ss b/undertest-junit5/src/test/kotlin/undertest/junit5/__snapshots__/UT_IntSelfieTest.ss new file mode 100644 index 00000000..7e438d52 --- /dev/null +++ b/undertest-junit5/src/test/kotlin/undertest/junit5/__snapshots__/UT_IntSelfieTest.ss @@ -0,0 +1,3 @@ +╔═ singleInt ═╗ +678 +╔═ [end of file] ═╗ From 4b9ea21b69f34bd6284d542d0d091bd658629e63 Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Tue, 3 Oct 2023 22:30:12 -0700 Subject: [PATCH 02/15] Add modeling for Literals. --- .../kotlin/com/diffplug/selfie/Literals.kt | 50 +++++++++++++++++++ 1 file changed, 50 insertions(+) create mode 100644 selfie-lib/src/commonMain/kotlin/com/diffplug/selfie/Literals.kt diff --git a/selfie-lib/src/commonMain/kotlin/com/diffplug/selfie/Literals.kt b/selfie-lib/src/commonMain/kotlin/com/diffplug/selfie/Literals.kt new file mode 100644 index 00000000..4495d154 --- /dev/null +++ b/selfie-lib/src/commonMain/kotlin/com/diffplug/selfie/Literals.kt @@ -0,0 +1,50 @@ +/* + * Copyright (C) 2023 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 + +class LiteralValue(val expected: T?, val actual: T, val format: LiteralFormat) { + fun encodedActual(): String = format.encode(actual) +} + +interface LiteralFormat { + fun encode(value: T): String + fun parse(str: String): T +} + +class IntFormat : LiteralFormat { + override fun encode(value: Int): String { + // TODO: 1000000 is hard to read, 1_000_000 is much much better + return value.toString() + } + override fun parse(str: String): Int { + return str.toInt() + } +} + +class StrFormat : LiteralFormat { + override fun encode(value: String): String { + if (!value.contains("\n")) { + // TODO: replace \t, maybe others... + return "\"" + value.replace("\"", "\\\"") + "\"" + } else { + // TODO: test! It's okay to assume Java 15+ for now + return "\"\"\"\n" + value + "\"\"\"" + } + } + override fun parse(str: String): String { + TODO("Harder than it seems!") + } +} From d1c70a55c3eca519de8e41d8c1c4165f2e3e8ff1 Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Tue, 3 Oct 2023 22:35:47 -0700 Subject: [PATCH 03/15] Selfie should pass inline writes along to the Router. --- .../main/kotlin/com/diffplug/selfie/Selfie.kt | 27 ++++++++++++++----- 1 file changed, 20 insertions(+), 7 deletions(-) diff --git a/selfie-runner-junit5/src/main/kotlin/com/diffplug/selfie/Selfie.kt b/selfie-runner-junit5/src/main/kotlin/com/diffplug/selfie/Selfie.kt index 7d620e20..aa217835 100644 --- a/selfie-runner-junit5/src/main/kotlin/com/diffplug/selfie/Selfie.kt +++ b/selfie-runner-junit5/src/main/kotlin/com/diffplug/selfie/Selfie.kt @@ -16,7 +16,7 @@ package com.diffplug.selfie import com.diffplug.selfie.junit5.Router -import org.junit.jupiter.api.Assertions.assertEquals +import com.diffplug.selfie.junit5.recordCall import org.opentest4j.AssertionFailedError /** @@ -70,13 +70,26 @@ class BinarySelfie(private val actual: ByteArray) : DiskSelfie(Snapshot.of(actua fun expectSelfie(actual: ByteArray) = BinarySelfie(actual) class IntSelfie(private val actual: Int) : DiskSelfie(Snapshot.of(actual.toString())) { - infix fun toBe(expected: Int): Int { - // TODO: Is this right? - val snapshot = toMatchDisk() - assertEquals(expected, snapshot.value.valueString().toInt()) - return expected + fun toBe_TODO(): Int = toBeDidntMatch(null) + infix fun toBe(expected: Int): Int = + if (actual == expected) expected + else { + toBeDidntMatch(expected) + } + private fun toBeDidntMatch(expected: Int?): Int { + if (RW.isWrite) { + Router.writeInline(recordCall(), LiteralValue(expected, actual, IntFormat())) + return actual + } else { + if (expected == null) { + throw AssertionFailedError( + "`.toBe_TODO()` was called in `read` mode, try again with selfie in write mode") + } else { + throw AssertionFailedError( + "Inline literal did not match the actual value", expected, actual) + } + } } - fun toBe_TODO(): Int = TODO() } fun expectSelfie(actual: Int) = IntSelfie(actual) From 338759ef9441e46e412cebff7639f582f2f834ee Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Tue, 3 Oct 2023 22:41:38 -0700 Subject: [PATCH 04/15] Router should pass inline writes to WriteTracker. --- .../junit5/SelfieTestExecutionListener.kt | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/selfie-runner-junit5/src/main/kotlin/com/diffplug/selfie/junit5/SelfieTestExecutionListener.kt b/selfie-runner-junit5/src/main/kotlin/com/diffplug/selfie/junit5/SelfieTestExecutionListener.kt index d5587532..cab185df 100644 --- a/selfie-runner-junit5/src/main/kotlin/com/diffplug/selfie/junit5/SelfieTestExecutionListener.kt +++ b/selfie-runner-junit5/src/main/kotlin/com/diffplug/selfie/junit5/SelfieTestExecutionListener.kt @@ -15,7 +15,12 @@ */ package com.diffplug.selfie.junit5 -import com.diffplug.selfie.* +import com.diffplug.selfie.ArrayMap +import com.diffplug.selfie.LiteralValue +import com.diffplug.selfie.RW +import com.diffplug.selfie.Snapshot +import com.diffplug.selfie.SnapshotFile +import com.diffplug.selfie.SnapshotValueReader import java.nio.charset.StandardCharsets import java.nio.file.Files import java.nio.file.Path @@ -30,6 +35,12 @@ import org.junit.platform.launcher.TestPlan internal object Router { private class ClassMethod(val clazz: ClassProgress, val method: String) private val threadCtx = ThreadLocal() + fun writeInline(call: CallStack, literalValue: LiteralValue<*>) { + val classMethod = + threadCtx.get() + ?: throw AssertionError("Selfie `toBe` must be called only on the original thread.") + classMethod.clazz.writeInline(call, literalValue) + } fun readOrWriteOrKeep(snapshot: Snapshot?, subOrKeepAll: String?): Snapshot? { val classMethod = threadCtx.get() @@ -94,6 +105,7 @@ internal class ClassProgress(val className: String) { private var file: SnapshotFile? = null private var methods = ArrayMap.empty() private var diskWriteTracker: DiskWriteTracker? = DiskWriteTracker() + private var inlineWriteTracker: InlineWriteTracker? = InlineWriteTracker() // the methods below called by the TestExecutionListener on its runtime thread @Synchronized fun startMethod(method: String) { assertNotTerminated() @@ -108,6 +120,7 @@ internal class ClassProgress(val className: String) { } @Synchronized fun finishedClassWithSuccess(success: Boolean) { assertNotTerminated() + inlineWriteTracker!!.persistWrites() if (file != null) { val staleSnapshotIndices = MethodSnapshotGC.findStaleSnapshotsWithin(className, file!!.snapshots, methods) @@ -134,6 +147,7 @@ internal class ClassProgress(val className: String) { // now that we are done, allow our contents to be GC'ed methods = TERMINATED diskWriteTracker = null + inlineWriteTracker = null file = null } // the methods below are called from the test thread for I/O on snapshots @@ -145,6 +159,9 @@ internal class ClassProgress(val className: String) { methods[method]!!.keepSuffix(suffixOrAll) } } + @Synchronized fun writeInline(call: CallStack, literalValue: LiteralValue<*>) { + inlineWriteTracker!!.record(call, literalValue) + } @Synchronized fun write(method: String, suffix: String, snapshot: Snapshot) { assertNotTerminated() val key = "$method$suffix" From a6245aa680c1d07274bda8a3278b2ae64299ad5e Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Tue, 3 Oct 2023 22:42:29 -0700 Subject: [PATCH 05/15] WriteTracker should persist inline snapshots. --- .../diffplug/selfie/junit5/WriteTracker.kt | 27 ++++++++++++++----- 1 file changed, 20 insertions(+), 7 deletions(-) diff --git a/selfie-runner-junit5/src/main/kotlin/com/diffplug/selfie/junit5/WriteTracker.kt b/selfie-runner-junit5/src/main/kotlin/com/diffplug/selfie/junit5/WriteTracker.kt index ba481592..cf2ad5a9 100644 --- a/selfie-runner-junit5/src/main/kotlin/com/diffplug/selfie/junit5/WriteTracker.kt +++ b/selfie-runner-junit5/src/main/kotlin/com/diffplug/selfie/junit5/WriteTracker.kt @@ -15,6 +15,7 @@ */ package com.diffplug.selfie.junit5 +import com.diffplug.selfie.LiteralValue import com.diffplug.selfie.RW import com.diffplug.selfie.Snapshot import java.util.stream.Collectors @@ -69,13 +70,25 @@ internal class DiskWriteTracker : WriteTracker() { recordInternal(key, snapshot, call) } } +private fun String.countNewlines(): Int = TODO() -class LiteralValue { - // TODO: String, Int, Long, Boolean, etc -} - -internal class InlineWriteTracker : WriteTracker() { - fun record(call: CallStack, snapshot: LiteralValue) { - recordInternal(call.location, snapshot, call) +internal class InlineWriteTracker : WriteTracker>() { + fun record(call: CallStack, literalValue: LiteralValue<*>) { + recordInternal(call.location, literalValue, call) + } + fun persistWrites() { + val locations = writes.toList().sortedBy { it.first } + var deltaLineNumbers = 0 + for (location in locations) { + val currentlyInFile = "TODO" + val literalValue = location.second.snapshot + val parsedInFile = literalValue.format.parse(currentlyInFile) + if (parsedInFile != literalValue.expected) { + // warn that the parsing wasn't quite as expected + } + val toInjectIntoFile = literalValue.encodedActual() + deltaLineNumbers += (toInjectIntoFile.countNewlines() - currentlyInFile.countNewlines()) + // TODO: inject the encoded thing into the file + } } } From 7f4de6045960a74b4fd144bd5bd24d11ba19c01f Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Tue, 3 Oct 2023 23:09:58 -0700 Subject: [PATCH 06/15] Add testing for the IntFormat (found a bug!) --- .../kotlin/com/diffplug/selfie/Literals.kt | 2 +- .../com/diffplug/selfie/IntFormatTest.kt | 56 +++++++++++++++++++ 2 files changed, 57 insertions(+), 1 deletion(-) create mode 100644 selfie-lib/src/commonTest/kotlin/com/diffplug/selfie/IntFormatTest.kt diff --git a/selfie-lib/src/commonMain/kotlin/com/diffplug/selfie/Literals.kt b/selfie-lib/src/commonMain/kotlin/com/diffplug/selfie/Literals.kt index 4495d154..1bee1185 100644 --- a/selfie-lib/src/commonMain/kotlin/com/diffplug/selfie/Literals.kt +++ b/selfie-lib/src/commonMain/kotlin/com/diffplug/selfie/Literals.kt @@ -30,7 +30,7 @@ class IntFormat : LiteralFormat { return value.toString() } override fun parse(str: String): Int { - return str.toInt() + return str.replace("_", "").toInt() } } diff --git a/selfie-lib/src/commonTest/kotlin/com/diffplug/selfie/IntFormatTest.kt b/selfie-lib/src/commonTest/kotlin/com/diffplug/selfie/IntFormatTest.kt new file mode 100644 index 00000000..3772b5e5 --- /dev/null +++ b/selfie-lib/src/commonTest/kotlin/com/diffplug/selfie/IntFormatTest.kt @@ -0,0 +1,56 @@ +/* + * Copyright (C) 2023 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 + +import io.kotest.matchers.shouldBe +import kotlin.test.Test + +class IntFormatTest { + @Test + fun encode() { + encode(0, "0") + encode(1, "1") + encode(-1, "-1") + encode(999, "999") + encode(-999, "-999") + // TODO: add underscores + encode(1_000, "1000") + encode(-1_000, "-1000") + encode(1_000_000, "1000000") + encode(-1_000_000, "-1000000") + } + private fun encode(value: Int, expected: String) { + val actual = IntFormat().encode(value) + actual shouldBe expected + } + + @Test + fun decode() { + decode("0", 0) + decode("1", 1) + decode("-1", -1) + decode("999", 999) + decode("9_99", 999) + decode("9_9_9", 999) + decode("-999", -999) + decode("-9_99", -999) + decode("-9_9_9", -999) + } + private fun decode(value: String, expected: Int) { + val actual = IntFormat().parse(value) + actual shouldBe expected + } +} From 002129dc37d1f7ffe1001caa4e219b189384f302 Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Wed, 4 Oct 2023 00:18:10 -0700 Subject: [PATCH 07/15] Pipe the filesystem into InlineWriteTracker.persistWrites() --- .../diffplug/selfie/junit5/SelfieConfig.kt | 8 ++++++ .../junit5/SelfieTestExecutionListener.kt | 9 ++++--- .../diffplug/selfie/junit5/WriteTracker.kt | 26 ++++++++++++++++--- 3 files changed, 37 insertions(+), 6 deletions(-) diff --git a/selfie-runner-junit5/src/main/kotlin/com/diffplug/selfie/junit5/SelfieConfig.kt b/selfie-runner-junit5/src/main/kotlin/com/diffplug/selfie/junit5/SelfieConfig.kt index d6f8a0be..3f5e9da7 100644 --- a/selfie-runner-junit5/src/main/kotlin/com/diffplug/selfie/junit5/SelfieConfig.kt +++ b/selfie-runner-junit5/src/main/kotlin/com/diffplug/selfie/junit5/SelfieConfig.kt @@ -57,6 +57,14 @@ internal class SnapshotFileLayout( } return classnameWithSlashes.replace('/', '.') } + fun testSourceFile(location: CallLocation): Path { + // we should have a way to have multiple source roots to check against + val path = rootFolder.resolve(location.subpath) + if (!Files.exists(path)) { + throw AssertionError("Unable to find ${location.subpath} at ${path.toAbsolutePath()}") + } + return path + } companion object { private const val DEFAULT_SNAPSHOT_DIR = "__snapshots__" diff --git a/selfie-runner-junit5/src/main/kotlin/com/diffplug/selfie/junit5/SelfieTestExecutionListener.kt b/selfie-runner-junit5/src/main/kotlin/com/diffplug/selfie/junit5/SelfieTestExecutionListener.kt index cab185df..65175502 100644 --- a/selfie-runner-junit5/src/main/kotlin/com/diffplug/selfie/junit5/SelfieTestExecutionListener.kt +++ b/selfie-runner-junit5/src/main/kotlin/com/diffplug/selfie/junit5/SelfieTestExecutionListener.kt @@ -82,11 +82,12 @@ internal object Router { } threadCtx.set(null) } - fun fileLocationFor(className: String): Path { + fun fileLocationFor(className: String): Path = layout(className).snapshotPathForClass(className) + fun layout(className: String): SnapshotFileLayout { if (layout == null) { layout = SnapshotFileLayout.initialize(className) } - return layout!!.snapshotPathForClass(className) + return layout!! } var layout: SnapshotFileLayout? = null @@ -120,7 +121,9 @@ internal class ClassProgress(val className: String) { } @Synchronized fun finishedClassWithSuccess(success: Boolean) { assertNotTerminated() - inlineWriteTracker!!.persistWrites() + if (inlineWriteTracker!!.hasWrites()) { + inlineWriteTracker!!.persistWrites(Router.layout(className)) + } if (file != null) { val staleSnapshotIndices = MethodSnapshotGC.findStaleSnapshotsWithin(className, file!!.snapshots, methods) diff --git a/selfie-runner-junit5/src/main/kotlin/com/diffplug/selfie/junit5/WriteTracker.kt b/selfie-runner-junit5/src/main/kotlin/com/diffplug/selfie/junit5/WriteTracker.kt index cf2ad5a9..624e5b2e 100644 --- a/selfie-runner-junit5/src/main/kotlin/com/diffplug/selfie/junit5/WriteTracker.kt +++ b/selfie-runner-junit5/src/main/kotlin/com/diffplug/selfie/junit5/WriteTracker.kt @@ -18,6 +18,8 @@ package com.diffplug.selfie.junit5 import com.diffplug.selfie.LiteralValue import com.diffplug.selfie.RW import com.diffplug.selfie.Snapshot +import java.nio.file.Files +import java.nio.file.Path import java.util.stream.Collectors /** Represents the line at which user code called into Selfie. */ @@ -76,19 +78,37 @@ internal class InlineWriteTracker : WriteTracker>( fun record(call: CallStack, literalValue: LiteralValue<*>) { recordInternal(call.location, literalValue, call) } - fun persistWrites() { + fun hasWrites(): Boolean = writes.isNotEmpty() + fun persistWrites(layout: SnapshotFileLayout) { val locations = writes.toList().sortedBy { it.first } + var subpath = "" var deltaLineNumbers = 0 + var source = "" + var path: Path? = null + // If I was implementing this, I would use Slice https://github.com/diffplug/selfie/pull/22 + // as the type of source, but that is by no means a requirement for (location in locations) { - val currentlyInFile = "TODO" + if (location.first.subpath != subpath) { + path?.let { Files.writeString(it, source) } + subpath = location.first.subpath + deltaLineNumbers = 0 + path = layout.testSourceFile(location.first) + source = Files.readString(path) + } + // parse the location within the file + val currentlyInFile = "TODO parse using ${location.first.line + deltaLineNumbers}" val literalValue = location.second.snapshot val parsedInFile = literalValue.format.parse(currentlyInFile) if (parsedInFile != literalValue.expected) { - // warn that the parsing wasn't quite as expected + // warn that the parsing wasn't as expected + // TODO: we can't report failures to the user very well + // someday, we should verify that the parse works in the `record()` and + // throw an `AssertionFail` there so that the user sees it early } val toInjectIntoFile = literalValue.encodedActual() deltaLineNumbers += (toInjectIntoFile.countNewlines() - currentlyInFile.countNewlines()) // TODO: inject the encoded thing into the file } + path?.let { Files.writeString(it, source) } } } From 689936c855e362125dada3fb96e2f74f52f10bd1 Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Wed, 4 Oct 2023 00:53:41 -0700 Subject: [PATCH 08/15] Bump spotless and improve the test harness. --- gradle/spotless.gradle | 2 +- .../com/diffplug/selfie/junit5/SelfieConfig.kt | 3 +-- .../kotlin/com/diffplug/selfie/junit5/Harness.kt | 15 +++++++++++++++ 3 files changed, 17 insertions(+), 3 deletions(-) diff --git a/gradle/spotless.gradle b/gradle/spotless.gradle index ae931332..ccce86de 100644 --- a/gradle/spotless.gradle +++ b/gradle/spotless.gradle @@ -15,7 +15,7 @@ spotless { kotlin { target 'src/**/*.kt' licenseHeaderFile 干.file("spotless/license-${license}.java") - ktfmt() + ktfmt('0.46') for (modifier in ['', 'override ', 'public ', 'protected ', 'private ', 'internal ', 'infix ', 'expected ', 'actual ']) { for (key in ['inline', 'fun', 'abstract fun', 'val', 'override']) { String toCheck = "$modifier$key" diff --git a/selfie-runner-junit5/src/main/kotlin/com/diffplug/selfie/junit5/SelfieConfig.kt b/selfie-runner-junit5/src/main/kotlin/com/diffplug/selfie/junit5/SelfieConfig.kt index 3f5e9da7..a3d4fdf7 100644 --- a/selfie-runner-junit5/src/main/kotlin/com/diffplug/selfie/junit5/SelfieConfig.kt +++ b/selfie-runner-junit5/src/main/kotlin/com/diffplug/selfie/junit5/SelfieConfig.kt @@ -102,8 +102,7 @@ internal class SnapshotFileLayout( } } .firstOrNull() - ?.let { it.indexOf('\r') == -1 } - ?: true // if we didn't find any files, assume unix + ?.let { it.indexOf('\r') == -1 } ?: true // if we didn't find any files, assume unix } private fun snapshotFolderName(snapshotDir: String?): String? { if (snapshotDir == null) { diff --git a/selfie-runner-junit5/src/test/kotlin/com/diffplug/selfie/junit5/Harness.kt b/selfie-runner-junit5/src/test/kotlin/com/diffplug/selfie/junit5/Harness.kt index 56301dd2..ae65e4ee 100644 --- a/selfie-runner-junit5/src/test/kotlin/com/diffplug/selfie/junit5/Harness.kt +++ b/selfie-runner-junit5/src/test/kotlin/com/diffplug/selfie/junit5/Harness.kt @@ -171,6 +171,21 @@ open class Harness(subproject: String) { } } } + fun content() = lines.subList(startInclusive, endInclusive).joinToString("\n") + fun setContent(mustBe: String) { + FileSystem.SYSTEM.write(subprojectFolder.resolve(subpath)) { + for (i in 0 ..< startInclusive) { + writeUtf8(lines[i]) + writeUtf8("\n") + } + writeUtf8(mustBe) + writeUtf8("\n") + for (i in endInclusive + 1 ..< lines.size) { + writeUtf8(lines[i]) + writeUtf8("\n") + } + } + } } } fun gradlew(task: String, vararg args: String): AssertionFailedError? { From 465a08d7adb340efa1efec255bcaa458531fc40f Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Wed, 4 Oct 2023 00:53:51 -0700 Subject: [PATCH 09/15] Add the inline test. --- .../diffplug/selfie/junit5/InlineIntTest.kt | 46 +++++++++++++++++++ ...T_IntSelfieTest.kt => UT_InlineIntTest.kt} | 4 +- .../junit5/__snapshots__/UT_IntSelfieTest.ss | 3 -- 3 files changed, 48 insertions(+), 5 deletions(-) create mode 100644 selfie-runner-junit5/src/test/kotlin/com/diffplug/selfie/junit5/InlineIntTest.kt rename undertest-junit5/src/test/kotlin/undertest/junit5/{UT_IntSelfieTest.kt => UT_InlineIntTest.kt} (71%) delete mode 100644 undertest-junit5/src/test/kotlin/undertest/junit5/__snapshots__/UT_IntSelfieTest.ss diff --git a/selfie-runner-junit5/src/test/kotlin/com/diffplug/selfie/junit5/InlineIntTest.kt b/selfie-runner-junit5/src/test/kotlin/com/diffplug/selfie/junit5/InlineIntTest.kt new file mode 100644 index 00000000..b1b3292b --- /dev/null +++ b/selfie-runner-junit5/src/test/kotlin/com/diffplug/selfie/junit5/InlineIntTest.kt @@ -0,0 +1,46 @@ +/* + * Copyright (C) 2023 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 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 + +/** Write-only test which asserts adding and removing snapshots results in same-class GC. */ +@TestMethodOrder(MethodOrderer.OrderAnnotation::class) +@DisableIfTestFails +class InlineIntTest : Harness("undertest-junit5") { + @Test @Order(1) + fun toBe_TODO() { + ut_mirror().lineWith("expectSelfie").setContent(" expectSelfie(1234).toBe_TODO()") + gradleReadSSFail() + } + + @Test @Order(2) + fun toBe_write() { + gradleWriteSS() + ut_mirror().lineWith("expectSelfie").content() shouldBe " expectSelfie(1234).toBe(1234)" + gradleReadSS() + } + + @Test @Order(3) + fun cleanip() { + ut_mirror().lineWith("expectSelfie").setContent(" expectSelfie(1234).toBe_TODO()") + } +} diff --git a/undertest-junit5/src/test/kotlin/undertest/junit5/UT_IntSelfieTest.kt b/undertest-junit5/src/test/kotlin/undertest/junit5/UT_InlineIntTest.kt similarity index 71% rename from undertest-junit5/src/test/kotlin/undertest/junit5/UT_IntSelfieTest.kt rename to undertest-junit5/src/test/kotlin/undertest/junit5/UT_InlineIntTest.kt index 6a600a95..ebe9d00b 100644 --- a/undertest-junit5/src/test/kotlin/undertest/junit5/UT_IntSelfieTest.kt +++ b/undertest-junit5/src/test/kotlin/undertest/junit5/UT_InlineIntTest.kt @@ -5,8 +5,8 @@ import kotlin.test.Test // spotless:on -class UT_IntSelfieTest { +class UT_InlineIntTest { @Test fun singleInt() { - expectSelfie(678) toBe 678 + expectSelfie(1234).toBe_TODO() } } diff --git a/undertest-junit5/src/test/kotlin/undertest/junit5/__snapshots__/UT_IntSelfieTest.ss b/undertest-junit5/src/test/kotlin/undertest/junit5/__snapshots__/UT_IntSelfieTest.ss deleted file mode 100644 index 7e438d52..00000000 --- a/undertest-junit5/src/test/kotlin/undertest/junit5/__snapshots__/UT_IntSelfieTest.ss +++ /dev/null @@ -1,3 +0,0 @@ -╔═ singleInt ═╗ -678 -╔═ [end of file] ═╗ From a1d9a5264534a738cf27c270798a990a402f45a0 Mon Sep 17 00:00:00 2001 From: Edgar Espina Date: Sun, 8 Oct 2023 21:42:14 -0300 Subject: [PATCH 10/15] Literal: parse literal and inject encoded value - parse single line literal - inject encoded value - TODO: tests are still failing - TODO: add Slice - TODO: support multi-line parsing --- .../diffplug/selfie/junit5/WriteTracker.kt | 48 +++++++++++++++++-- .../com/diffplug/selfie/junit5/Harness.kt | 3 +- .../diffplug/selfie/junit5/InlineIntTest.kt | 2 +- 3 files changed, 48 insertions(+), 5 deletions(-) diff --git a/selfie-runner-junit5/src/main/kotlin/com/diffplug/selfie/junit5/WriteTracker.kt b/selfie-runner-junit5/src/main/kotlin/com/diffplug/selfie/junit5/WriteTracker.kt index 624e5b2e..e7129f9e 100644 --- a/selfie-runner-junit5/src/main/kotlin/com/diffplug/selfie/junit5/WriteTracker.kt +++ b/selfie-runner-junit5/src/main/kotlin/com/diffplug/selfie/junit5/WriteTracker.kt @@ -20,6 +20,8 @@ import com.diffplug.selfie.RW import com.diffplug.selfie.Snapshot import java.nio.file.Files import java.nio.file.Path +import java.util.regex.Matcher +import java.util.regex.Pattern import java.util.stream.Collectors /** Represents the line at which user code called into Selfie. */ @@ -72,7 +74,19 @@ internal class DiskWriteTracker : WriteTracker() { recordInternal(key, snapshot, call) } } -private fun String.countNewlines(): Int = TODO() +private fun String.countNewlines(): Int = lineOffset { true }.size +private fun String.lineOffset(filter: (Int) -> Boolean): List { + val lineTerminator = "\n" + var offset = 0 + var next = indexOf(lineTerminator, offset) + val offsets = mutableListOf() + while (next != -1 && filter(offsets.size)) { + offsets.add(offset) + offset = next + lineTerminator.length + next = indexOf(lineTerminator, offset) + } + return offsets +} internal class InlineWriteTracker : WriteTracker>() { fun record(call: CallStack, literalValue: LiteralValue<*>) { @@ -96,7 +110,18 @@ internal class InlineWriteTracker : WriteTracker>( source = Files.readString(path) } // parse the location within the file - val currentlyInFile = "TODO parse using ${location.first.line + deltaLineNumbers}" + val line = location.first.line + deltaLineNumbers + val offsets = source.lineOffset { it <= line + 1 } + val startOffset = offsets[line] + // TODO: multi-line support + val endOffset = + if (line + 1 < offsets.size) { + offsets[line + 1] + } else { + source.length + } + val matcher = parseExpectSelfie(source.substring(startOffset, endOffset)) + val currentlyInFile = matcher.group(2) val literalValue = location.second.snapshot val parsedInFile = literalValue.format.parse(currentlyInFile) if (parsedInFile != literalValue.expected) { @@ -107,8 +132,25 @@ internal class InlineWriteTracker : WriteTracker>( } val toInjectIntoFile = literalValue.encodedActual() deltaLineNumbers += (toInjectIntoFile.countNewlines() - currentlyInFile.countNewlines()) - // TODO: inject the encoded thing into the file + source = + source.replaceRange(startOffset, endOffset, matcher.replaceAll("$1$toInjectIntoFile$3")) } path?.let { Files.writeString(it, source) } } + private fun replaceLiteral(matcher: Matcher, toInjectIntoFile: String): CharSequence { + val sb = StringBuilder() + matcher.appendReplacement(sb, toInjectIntoFile) + matcher.appendTail(sb) + return sb + } + private fun parseExpectSelfie(source: String): Matcher { + // TODO: support multi-line parsing + val pattern = Pattern.compile("^(\\s*expectSelfie\\()([^)]*)(\\))", Pattern.MULTILINE) + val matcher = pattern.matcher(source) + if (matcher.find()) { + return matcher + } else { + TODO("Unexpected line: $source") + } + } } diff --git a/selfie-runner-junit5/src/test/kotlin/com/diffplug/selfie/junit5/Harness.kt b/selfie-runner-junit5/src/test/kotlin/com/diffplug/selfie/junit5/Harness.kt index ae65e4ee..c9f3b3d4 100644 --- a/selfie-runner-junit5/src/test/kotlin/com/diffplug/selfie/junit5/Harness.kt +++ b/selfie-runner-junit5/src/test/kotlin/com/diffplug/selfie/junit5/Harness.kt @@ -98,7 +98,8 @@ open class Harness(subproject: String) { } val matchingLines = allLines.mapIndexedNotNull() { index, line -> - if (line.contains(start)) "L$index: $line" else null + // TODO: probably need more than ignore import?? + if (line.contains(start) && !line.contains("import ")) "L$index: $line" else null } if (matchingLines.size == 1) { val idx = matchingLines[0].substringAfter("L").substringBefore(":").toInt() diff --git a/selfie-runner-junit5/src/test/kotlin/com/diffplug/selfie/junit5/InlineIntTest.kt b/selfie-runner-junit5/src/test/kotlin/com/diffplug/selfie/junit5/InlineIntTest.kt index b1b3292b..d58f894d 100644 --- a/selfie-runner-junit5/src/test/kotlin/com/diffplug/selfie/junit5/InlineIntTest.kt +++ b/selfie-runner-junit5/src/test/kotlin/com/diffplug/selfie/junit5/InlineIntTest.kt @@ -40,7 +40,7 @@ class InlineIntTest : Harness("undertest-junit5") { } @Test @Order(3) - fun cleanip() { + fun cleanup() { ut_mirror().lineWith("expectSelfie").setContent(" expectSelfie(1234).toBe_TODO()") } } From 0899b4091afc23f51c758644e6a49562072bed7b Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Sun, 17 Dec 2023 09:00:55 -0800 Subject: [PATCH 11/15] Mark `UT_InlineIntTest` as ignored so it doesn't kill other tests. --- .../test/kotlin/com/diffplug/selfie/junit5/InlineIntTest.kt | 6 ++++-- .../src/test/kotlin/undertest/junit5/UT_InlineIntTest.kt | 3 ++- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/selfie-runner-junit5/src/test/kotlin/com/diffplug/selfie/junit5/InlineIntTest.kt b/selfie-runner-junit5/src/test/kotlin/com/diffplug/selfie/junit5/InlineIntTest.kt index d58f894d..93cc4501 100644 --- a/selfie-runner-junit5/src/test/kotlin/com/diffplug/selfie/junit5/InlineIntTest.kt +++ b/selfie-runner-junit5/src/test/kotlin/com/diffplug/selfie/junit5/InlineIntTest.kt @@ -17,17 +17,18 @@ package com.diffplug.selfie.junit5 import io.kotest.matchers.shouldBe import kotlin.test.Test +import org.junit.jupiter.api.AfterAll import org.junit.jupiter.api.MethodOrderer import org.junit.jupiter.api.Order import org.junit.jupiter.api.TestMethodOrder -import org.junitpioneer.jupiter.DisableIfTestFails /** Write-only test which asserts adding and removing snapshots results in same-class GC. */ @TestMethodOrder(MethodOrderer.OrderAnnotation::class) -@DisableIfTestFails +//@DisableIfTestFails don't disable if test fails because we *have* to run cleanup class InlineIntTest : Harness("undertest-junit5") { @Test @Order(1) fun toBe_TODO() { + ut_mirror().lineWith("@Ignore").setContent("//@Ignore") ut_mirror().lineWith("expectSelfie").setContent(" expectSelfie(1234).toBe_TODO()") gradleReadSSFail() } @@ -42,5 +43,6 @@ class InlineIntTest : Harness("undertest-junit5") { @Test @Order(3) fun cleanup() { ut_mirror().lineWith("expectSelfie").setContent(" expectSelfie(1234).toBe_TODO()") + ut_mirror().lineWith("//@Ignore").setContent("@Ignore") } } diff --git a/undertest-junit5/src/test/kotlin/undertest/junit5/UT_InlineIntTest.kt b/undertest-junit5/src/test/kotlin/undertest/junit5/UT_InlineIntTest.kt index ebe9d00b..8a1751ae 100644 --- a/undertest-junit5/src/test/kotlin/undertest/junit5/UT_InlineIntTest.kt +++ b/undertest-junit5/src/test/kotlin/undertest/junit5/UT_InlineIntTest.kt @@ -1,10 +1,11 @@ package undertest.junit5 // spotless:off import com.diffplug.selfie.expectSelfie +import kotlin.test.Ignore import kotlin.test.Test - // spotless:on +@Ignore class UT_InlineIntTest { @Test fun singleInt() { expectSelfie(1234).toBe_TODO() From 0d13147cc71a3c2ed3e4434f09c4e7c627084273 Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Sun, 17 Dec 2023 21:08:47 -0800 Subject: [PATCH 12/15] Give Slice a clean way to find a given line (and make it module-private). --- .../kotlin/com/diffplug/selfie/Slice.kt | 47 ++++++++++++++----- .../kotlin/com/diffplug/selfie/SliceTest.kt | 12 +++++ .../kotlin/com/diffplug/selfie/Slice.jvm.kt | 2 +- 3 files changed, 48 insertions(+), 13 deletions(-) diff --git a/selfie-lib/src/commonMain/kotlin/com/diffplug/selfie/Slice.kt b/selfie-lib/src/commonMain/kotlin/com/diffplug/selfie/Slice.kt index 6aac3bdc..2b00b6e3 100644 --- a/selfie-lib/src/commonMain/kotlin/com/diffplug/selfie/Slice.kt +++ b/selfie-lib/src/commonMain/kotlin/com/diffplug/selfie/Slice.kt @@ -26,8 +26,8 @@ import kotlin.math.min */ internal expect fun groupImpl(slice: Slice, matchResult: MatchResult, group: Int): Slice -class Slice private constructor(val base: CharSequence, val startIndex: Int, val endIndex: Int) : - CharSequence { +internal class Slice +private constructor(val base: CharSequence, val startIndex: Int, val endIndex: Int) : CharSequence { init { require(base is StringBuilder || base is String) require(0 <= startIndex) @@ -174,22 +174,34 @@ class Slice private constructor(val base: CharSequence, val startIndex: Int, val } return true } - fun indexOf(lookingFor: String): Int { + fun indexOf(lookingFor: String, startOffset: Int = 0): Int { val result = - if (base is String) base.indexOf(lookingFor, startIndex) - else { - (base as StringBuilder).indexOf(lookingFor, startIndex) - } + if (base is String) base.indexOf(lookingFor, startIndex + startOffset) + else (base as StringBuilder).indexOf(lookingFor, startIndex + startOffset) return if (result == -1 || result >= endIndex) -1 else result - startIndex } - fun indexOf(lookingFor: Char): Int { + fun indexOf(lookingFor: Char, startOffset: Int = 0): Int { val result = - if (base is String) base.indexOf(lookingFor, startIndex) - else { - (base as StringBuilder).indexOf(lookingFor.toString(), startIndex) - } + if (base is String) base.indexOf(lookingFor, startIndex + startOffset) + else (base as StringBuilder).indexOf(lookingFor, startIndex + startOffset) return if (result == -1 || result >= endIndex) -1 else result - startIndex } + /** Returns a slice at the nth line. Handy for expanding the slice from there. */ + fun unixLine(count: Int): Slice { + check(count > 0) + var lineStart = 0 + for (i in 1 until count) { + lineStart = indexOf('\n', lineStart) + require(lineStart >= 0) { "This string has only ${i - 1} lines, not $count" } + ++lineStart + } + var lineEnd = indexOf('\n', lineStart) + return if (lineEnd == -1) { + Slice(base, startIndex + lineStart, endIndex) + } else { + Slice(base, startIndex + lineStart, startIndex + lineEnd) + } + } /** * Returns a Slice which represents everything from the start of this string until `lookingFor` is @@ -264,6 +276,17 @@ class Slice private constructor(val base: CharSequence, val startIndex: Int, val return endIndex == other.endIndex } + /** Returns the underlying string with this slice replaced by the given string. */ + fun replaceSelfWith(s: String): String { + check(base is String) + val deltaLength = s.length - length + val builder = StringBuilder(base.length + deltaLength) + builder.appendRange(base, 0, startIndex) + builder.append(s) + builder.appendRange(base, endIndex, base.length) + return builder.toString() + } + companion object { @JvmStatic fun of(base: String, startIndex: Int = 0, endIndex: Int = base.length): Slice { diff --git a/selfie-lib/src/commonTest/kotlin/com/diffplug/selfie/SliceTest.kt b/selfie-lib/src/commonTest/kotlin/com/diffplug/selfie/SliceTest.kt index 965280e6..4ffef5d8 100644 --- a/selfie-lib/src/commonTest/kotlin/com/diffplug/selfie/SliceTest.kt +++ b/selfie-lib/src/commonTest/kotlin/com/diffplug/selfie/SliceTest.kt @@ -35,4 +35,16 @@ class SliceTest { untilZ.toString() shouldBe "abcdef" abcdef.after(untilZ).toString() shouldBe "" } + + @Test + fun unixLine() { + Slice.of("A single line").unixLine(1).toString() shouldBe "A single line" + val oneTwoThree = Slice.of("\nI am the first\nI, the second\n\nFOURTH\n") + oneTwoThree.unixLine(1).toString() shouldBe "" + oneTwoThree.unixLine(2).toString() shouldBe "I am the first" + oneTwoThree.unixLine(3).toString() shouldBe "I, the second" + oneTwoThree.unixLine(4).toString() shouldBe "" + oneTwoThree.unixLine(5).toString() shouldBe "FOURTH" + oneTwoThree.unixLine(6).toString() shouldBe "" + } } diff --git a/selfie-lib/src/jvmMain/kotlin/com/diffplug/selfie/Slice.jvm.kt b/selfie-lib/src/jvmMain/kotlin/com/diffplug/selfie/Slice.jvm.kt index d4edd889..e3b421fd 100644 --- a/selfie-lib/src/jvmMain/kotlin/com/diffplug/selfie/Slice.jvm.kt +++ b/selfie-lib/src/jvmMain/kotlin/com/diffplug/selfie/Slice.jvm.kt @@ -24,7 +24,7 @@ package com.diffplug.selfie * Would be cool to have PoolString.Root which differentiates the String-based ones from * StringBuilder-based ones. */ -actual fun groupImpl(slice: Slice, matchResult: MatchResult, group: Int): Slice { +internal actual fun groupImpl(slice: Slice, matchResult: MatchResult, group: Int): Slice { val group = matchResult.groups[group]!! return slice.subSequence(group.range.start, group.range.endInclusive - 1) } From dc245b1128f30c09f30b0915d150d945bfd8eeab Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Mon, 18 Dec 2023 00:00:38 -0800 Subject: [PATCH 13/15] Fix a bug in `Harness` related to reading lines from test source files. --- .../src/test/kotlin/com/diffplug/selfie/junit5/Harness.kt | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/selfie-runner-junit5/src/test/kotlin/com/diffplug/selfie/junit5/Harness.kt b/selfie-runner-junit5/src/test/kotlin/com/diffplug/selfie/junit5/Harness.kt index 001712e8..9e3220e3 100644 --- a/selfie-runner-junit5/src/test/kotlin/com/diffplug/selfie/junit5/Harness.kt +++ b/selfie-runner-junit5/src/test/kotlin/com/diffplug/selfie/junit5/Harness.kt @@ -172,7 +172,7 @@ open class Harness(subproject: String) { } } } - fun content() = lines.subList(startInclusive, endInclusive).joinToString("\n") + fun content() = lines.subList(startInclusive, endInclusive + 1).joinToString("\n") fun setContent(mustBe: String) { FileSystem.SYSTEM.write(subprojectFolder.resolve(subpath)) { for (i in 0 ..< startInclusive) { @@ -198,8 +198,8 @@ open class Harness(subproject: String) { val buildLauncher = connection .newBuild() - // .setStandardError(System.err) - // .setStandardOutput(System.out) + .setStandardError(System.err) + .setStandardOutput(System.out) .forTasks(":${subprojectFolder.name}:$task") .withArguments( buildList { From 1b6d2711bf940601f48ee3ea5286fc31a7f9beee Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Mon, 18 Dec 2023 00:19:03 -0800 Subject: [PATCH 14/15] Created a new `SourceFile` which is now used to write inline snapshots. --- .../com/diffplug/selfie/SnapshotFile.kt | 2 +- .../kotlin/com/diffplug/selfie/SourceFile.kt | 72 +++++++++++++++++ .../diffplug/selfie/junit5/WriteTracker.kt | 79 +++++-------------- 3 files changed, 94 insertions(+), 59 deletions(-) create mode 100644 selfie-lib/src/commonMain/kotlin/com/diffplug/selfie/SourceFile.kt diff --git a/selfie-lib/src/commonMain/kotlin/com/diffplug/selfie/SnapshotFile.kt b/selfie-lib/src/commonMain/kotlin/com/diffplug/selfie/SnapshotFile.kt index 550dfe28..3e6b63a8 100644 --- a/selfie-lib/src/commonMain/kotlin/com/diffplug/selfie/SnapshotFile.kt +++ b/selfie-lib/src/commonMain/kotlin/com/diffplug/selfie/SnapshotFile.kt @@ -80,7 +80,7 @@ data class Snapshot( interface Snapshotter { fun snapshot(value: T): Snapshot } -private fun String.efficientReplace(find: String, replaceWith: String): String { +internal fun String.efficientReplace(find: String, replaceWith: String): String { val idx = this.indexOf(find) return if (idx == -1) this else this.replace(find, replaceWith) } diff --git a/selfie-lib/src/commonMain/kotlin/com/diffplug/selfie/SourceFile.kt b/selfie-lib/src/commonMain/kotlin/com/diffplug/selfie/SourceFile.kt new file mode 100644 index 00000000..44a25519 --- /dev/null +++ b/selfie-lib/src/commonMain/kotlin/com/diffplug/selfie/SourceFile.kt @@ -0,0 +1,72 @@ +/* + * Copyright (C) 2023 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 + +/** + * @param filename The filename (not full path, but the extension is used for language-specific + * parsing). + * @param content The exact content of the file, unix or windows newlines will be preserved + */ +class SourceFile(val filename: String, content: String) { + private val unixNewlines = content.indexOf('\r') == -1 + private var contentSlice = Slice.of(content.efficientReplace("\r\n", "\n")) + /** + * Returns the content of the file, possibly modified by + * [ToBeLiteral.setLiteralAndGetNewlineDelta]. + */ + val asString: String + get() = + if (unixNewlines) contentSlice.toString() else contentSlice.toString().replace("\r\n", "\n") + + /** + * Represents a section of the sourcecode which is a `.toBe(LITERAL)` call. It might also be + * `.toBe_TODO()` or ` toBe LITERAL` (infix notation). + */ + inner class ToBeLiteral internal constructor(private val slice: Slice) { + /** + * Modifies the parent [SourceFile] to set the value within the `toBe` call, and returns the net + * change in newline count. + */ + fun setLiteralAndGetNewlineDelta(literalValue: LiteralValue): Int { + val encoded = literalValue.format.encode(literalValue.actual) + val existingNewlines = slice.count { it == '\n' } + val newNewlines = encoded.count { it == '\n' } + contentSlice = Slice.of(slice.replaceSelfWith(".toBe($encoded)")) + return newNewlines - existingNewlines + } + + /** + * Parses the current value of the value within `.toBe()`. This method should not be called on + * `toBe_TODO()`. + */ + fun parseLiteral(literalFormat: LiteralFormat): T { + // this won't work, because we need to find the `.toBe` and parens + TODO("return literalFormat.parse(slice.toString())") + } + } + fun parseToBe_TODO(lineOneIndexed: Int): ToBeLiteral { + val lineContent = contentSlice.unixLine(lineOneIndexed) + val idx = lineContent.indexOf(".toBe_TODO()") + if (idx == -1) { + throw AssertionError( + "Expected to find `.toBe_TODO()` on line $lineOneIndexed, but there was only `${lineContent}`") + } + return ToBeLiteral(lineContent.subSequence(idx, idx + ".toBe_TODO()".length)) + } + fun parseToBe(lineOneIndexed: Int): ToBeLiteral { + TODO("More complicated because we have to actually parse the literal") + } +} diff --git a/selfie-runner-junit5/src/main/kotlin/com/diffplug/selfie/junit5/WriteTracker.kt b/selfie-runner-junit5/src/main/kotlin/com/diffplug/selfie/junit5/WriteTracker.kt index d6717259..b7424ea3 100644 --- a/selfie-runner-junit5/src/main/kotlin/com/diffplug/selfie/junit5/WriteTracker.kt +++ b/selfie-runner-junit5/src/main/kotlin/com/diffplug/selfie/junit5/WriteTracker.kt @@ -18,10 +18,9 @@ package com.diffplug.selfie.junit5 import com.diffplug.selfie.LiteralValue import com.diffplug.selfie.RW import com.diffplug.selfie.Snapshot +import com.diffplug.selfie.SourceFile import java.nio.file.Files import java.nio.file.Path -import java.util.regex.Matcher -import java.util.regex.Pattern import java.util.stream.Collectors import kotlin.io.path.name @@ -98,19 +97,6 @@ internal class DiskWriteTracker : WriteTracker() { recordInternal(key, snapshot, call, layout) } } -private fun String.countNewlines(): Int = lineOffset { true }.size -private fun String.lineOffset(filter: (Int) -> Boolean): List { - val lineTerminator = "\n" - var offset = 0 - var next = indexOf(lineTerminator, offset) - val offsets = mutableListOf() - while (next != -1 && filter(offsets.size)) { - offsets.add(offset) - offset = next + lineTerminator.length - next = indexOf(lineTerminator, offset) - } - return offsets -} internal class InlineWriteTracker : WriteTracker>() { fun record(call: CallStack, literalValue: LiteralValue<*>, layout: SnapshotFileLayout) { @@ -137,59 +123,36 @@ internal class InlineWriteTracker : WriteTracker>( .sorted() var file = writes.first().file - var contentOfFile = Files.readString(file) + var content = SourceFile(file.name, Files.readString(file)) var deltaLineNumbers = 0 - // If I was implementing this, I would use Slice https://github.com/diffplug/selfie/pull/22 - // as the type of source, but that is by no means a requirement for (write in writes) { if (write.file != file) { - Files.writeString(file, contentOfFile) + Files.writeString(file, content.asString) file = write.file deltaLineNumbers = 0 - contentOfFile = Files.readString(file) + content = SourceFile(file.name, Files.readString(file)) } // parse the location within the file val line = write.line + deltaLineNumbers - val offsets = contentOfFile.lineOffset { it <= line + 1 } - val startOffset = offsets[line] - // TODO: multi-line support - val endOffset = - if (line + 1 < offsets.size) { - offsets[line + 1] + val toBe = + if (write.literal.expected == null) { + content.parseToBe_TODO(line) } else { - contentOfFile.length + content.parseToBe(line).also { + val currentValue = it.parseLiteral(write.literal.format) + if (currentValue != write.literal.expected) { + // TODO: this shouldn't happen here, it should happen + // when the write is recorded so that we can fail eagerly, + // exceptions thrown here are easy to miss + throw org.opentest4j.AssertionFailedError( + "There is likely a bug in Selfie's literal parsing.", + write.literal.expected, + currentValue) + } + } } - val matcher = parseExpectSelfie(contentOfFile.substring(startOffset, endOffset)) - val currentlyInFile = matcher.group(2) - val parsedInFile = write.literal.format.parse(currentlyInFile) - if (parsedInFile != write.literal.expected) { - // warn that the parsing wasn't as expected - // TODO: we can't report failures to the user very well - // someday, we should verify that the parse works in the `record()` and - // throw an `AssertionFail` there so that the user sees it early - } - val toInjectIntoFile = write.literal.encodedActual() - deltaLineNumbers += (toInjectIntoFile.countNewlines() - currentlyInFile.countNewlines()) - contentOfFile = - contentOfFile.replaceRange( - startOffset, endOffset, matcher.replaceAll("$1$toInjectIntoFile$3")) - } - file?.let { Files.writeString(it, contentOfFile) } - } - private fun replaceLiteral(matcher: Matcher, toInjectIntoFile: String): CharSequence { - val sb = StringBuilder() - matcher.appendReplacement(sb, toInjectIntoFile) - matcher.appendTail(sb) - return sb - } - private fun parseExpectSelfie(source: String): Matcher { - // TODO: support multi-line parsing - val pattern = Pattern.compile("^(\\s*expectSelfie\\()([^)]*)(\\))", Pattern.MULTILINE) - val matcher = pattern.matcher(source) - if (matcher.find()) { - return matcher - } else { - TODO("Unexpected line: $source") + deltaLineNumbers += toBe.setLiteralAndGetNewlineDelta(write.literal) } + Files.writeString(file, content.asString) } } From 1d77e6f4640025b96ac5711159b0edf03914539a Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Mon, 18 Dec 2023 00:31:01 -0800 Subject: [PATCH 15/15] Simplify Slice to be as small as possible. --- .../kotlin/com/diffplug/selfie/Slice.kt | 249 +----------------- .../kotlin/com/diffplug/selfie/SourceFile.kt | 4 +- .../kotlin/com/diffplug/selfie/SliceTest.kt | 21 +- .../kotlin/com/diffplug/selfie/Slice.js.kt | 29 -- .../kotlin/com/diffplug/selfie/Slice.jvm.kt | 30 --- 5 files changed, 18 insertions(+), 315 deletions(-) delete mode 100644 selfie-lib/src/jsMain/kotlin/com/diffplug/selfie/Slice.js.kt delete mode 100644 selfie-lib/src/jvmMain/kotlin/com/diffplug/selfie/Slice.jvm.kt diff --git a/selfie-lib/src/commonMain/kotlin/com/diffplug/selfie/Slice.kt b/selfie-lib/src/commonMain/kotlin/com/diffplug/selfie/Slice.kt index 2b00b6e3..47c4f726 100644 --- a/selfie-lib/src/commonMain/kotlin/com/diffplug/selfie/Slice.kt +++ b/selfie-lib/src/commonMain/kotlin/com/diffplug/selfie/Slice.kt @@ -15,21 +15,9 @@ */ package com.diffplug.selfie -import kotlin.jvm.JvmStatic -import kotlin.math.min - -/** - * A [CharSequence] which can efficiently subdivide and append itself. - * - * Equal only to other [Slice] with the same [Slice.toString]. Use [Slice.sameAs] to compare with - * other kinds of [CharSequence]. - */ -internal expect fun groupImpl(slice: Slice, matchResult: MatchResult, group: Int): Slice - -internal class Slice -private constructor(val base: CharSequence, val startIndex: Int, val endIndex: Int) : CharSequence { +internal class Slice(val base: String, val startIndex: Int = 0, val endIndex: Int = base.length) : + CharSequence { init { - require(base is StringBuilder || base is String) require(0 <= startIndex) require(startIndex <= endIndex) require(endIndex <= base.length) @@ -37,12 +25,8 @@ private constructor(val base: CharSequence, val startIndex: Int, val endIndex: I override val length: Int get() = endIndex - startIndex override fun get(index: Int): Char = base[startIndex + index] - override fun subSequence(start: Int, end: Int): Slice { - return Slice(base, startIndex + start, startIndex + end) - } - - /** Returns a Slice representing the given group within the given match */ - fun group(matchResult: MatchResult, group: Int): Slice = groupImpl(this, matchResult, group) + override fun subSequence(start: Int, end: Int): Slice = + Slice(base, startIndex + start, startIndex + end) /** Same behavior as [String.trim]. */ fun trim(): Slice { @@ -57,89 +41,6 @@ private constructor(val base: CharSequence, val startIndex: Int, val endIndex: I return if (start > 0 || end < length) subSequence(start, end) else this } override fun toString() = base.subSequence(startIndex, endIndex).toString() - fun concat(other: Slice): Slice = - if (this.isEmpty()) { - other - } else if (other.isEmpty()) { - this - } else if (base === other.base && endIndex == other.startIndex) { - Slice(base, startIndex, other.endIndex) - } else { - val builder: StringBuilder - val start: Int - val end: Int - if (base is StringBuilder && endIndex == base.length) { - builder = base - start = startIndex - end = endIndex + other.length - } else { - builder = StringBuilder(length + other.length) - builder.append(this) - start = 0 - end = length + other.length - } - other.appendThisTo(builder) - Slice(builder, start, end) - } - - /** append(this) but taking advantage of fastpath where possible */ - private fun appendThisTo(builder: StringBuilder) { - if (startIndex == 0 && endIndex == base.length) { - // there is a fastpath for adding a full string and for adding a full StringBuilder - if (base is String) { - builder.append(base) - } else { - builder.append(base as StringBuilder) - } - } else { - builder.append(this) - } - } - fun concat(other: String): Slice { - if (base is String && endIndex + other.length <= base.length) { - for (i in other.indices) { - if (base[i + endIndex] != other[i]) { - return concat(of(other)) - } - } - return Slice(base, startIndex, endIndex + other.length) - } - return concat(of(other)) - } - fun concatAnchored(other: Slice): Slice { - val result = concat(other) - if (result.base !== base) { - throw concatRootFailure(other) - } - return result - } - fun concatAnchored(other: String): Slice { - val result = concat(other) - if (result.base !== base) { - throw concatRootFailure(other) - } - return result - } - private fun concatRootFailure(other: CharSequence): IllegalArgumentException { - val maxChange = min(other.length, base.length - endIndex) - if (maxChange == 0) { - return IllegalArgumentException( - "Could not perform anchored concat because we are already at the end of the root ${visualize(base)}") - } - var firstChange = 0 - while (firstChange < maxChange) { - if (base[endIndex + firstChange] != other[firstChange]) { - break - } - ++firstChange - } - return IllegalArgumentException( - """ - This ends with '${visualize(base.subSequence(endIndex, endIndex + firstChange + 1))}' - cannot concat '${visualize(other.subSequence(firstChange, firstChange + 1))} - """ - .trimIndent()) - } fun sameAs(other: CharSequence): Boolean { if (length != other.length) { return false @@ -151,39 +52,12 @@ private constructor(val base: CharSequence, val startIndex: Int, val endIndex: I } return true } - fun startsWith(prefix: CharSequence): Boolean { - if (length < prefix.length) { - return false - } - for (i in 0 until prefix.length) { - if (get(i) != prefix[i]) { - return false - } - } - return true - } - fun endsWith(suffix: CharSequence): Boolean { - if (length < suffix.length) { - return false - } - val offset = length - suffix.length - for (i in 0 until suffix.length) { - if (get(i + offset) != suffix[i]) { - return false - } - } - return true - } fun indexOf(lookingFor: String, startOffset: Int = 0): Int { - val result = - if (base is String) base.indexOf(lookingFor, startIndex + startOffset) - else (base as StringBuilder).indexOf(lookingFor, startIndex + startOffset) + val result = base.indexOf(lookingFor, startIndex + startOffset) return if (result == -1 || result >= endIndex) -1 else result - startIndex } fun indexOf(lookingFor: Char, startOffset: Int = 0): Int { - val result = - if (base is String) base.indexOf(lookingFor, startIndex + startOffset) - else (base as StringBuilder).indexOf(lookingFor, startIndex + startOffset) + val result = base.indexOf(lookingFor, startIndex + startOffset) return if (result == -1 || result >= endIndex) -1 else result - startIndex } /** Returns a slice at the nth line. Handy for expanding the slice from there. */ @@ -195,75 +69,21 @@ private constructor(val base: CharSequence, val startIndex: Int, val endIndex: I require(lineStart >= 0) { "This string has only ${i - 1} lines, not $count" } ++lineStart } - var lineEnd = indexOf('\n', lineStart) + val lineEnd = indexOf('\n', lineStart) return if (lineEnd == -1) { Slice(base, startIndex + lineStart, endIndex) } else { Slice(base, startIndex + lineStart, startIndex + lineEnd) } } - - /** - * Returns a Slice which represents everything from the start of this string until `lookingFor` is - * found. If the string is never found, returns this. - */ - fun until(lookingFor: String): Slice { - val idx = indexOf(lookingFor) - return if (idx == -1) this else subSequence(0, idx) - } - - /** - * Asserts that the other string was generated from a call to [.until], and then returns a new - * Slice representing everything after that. - */ - fun after(other: Slice): Slice { - if (other.isEmpty()) { - return this - } - require(other.base === base && other.startIndex == startIndex && other.endIndex <= endIndex) { - "'${visualize(other)}' was not generated by `until` on '${visualize(this)}'" - } - return Slice(base, other.endIndex, endIndex) - } - - /** - * Returns the line number of the start of this string. Throws an exception if this isn't based on - * a string any longer, because non-contiguous StringPools have been concatenated. - */ - fun baseLineNumberStart(): Int { - return baseLineNumberOfOffset(startIndex) - } - - /** - * Returns the line number of the end of this string. Throws an exception if this isn't based on a - * string any longer, because non-contiguous Slices have been concatenated. - */ - fun baseLineNumberEnd(): Int { - return baseLineNumberOfOffset(endIndex) - } - private fun baseLineNumberOfOffset(idx: Int): Int { - assertStringBased() - var lineNumber = 1 - for (i in 0 until base.length) { - if (base[i] == '\n') { - ++lineNumber + override fun equals(other: Any?) = + if (this === other) { + true + } else if (other is Slice) { + sameAs(other) + } else { + false } - } - return lineNumber - } - private fun assertStringBased() { - check(base is String) { - "When you call concat on non-contiguous parts, you lose the connection to the original String." - } - } - override fun equals(anObject: Any?): Boolean { - if (this === anObject) { - return true - } else if (anObject is Slice) { - return sameAs(anObject) - } - return false - } override fun hashCode(): Int { var h = 0 for (i in indices) { @@ -271,14 +91,8 @@ private constructor(val base: CharSequence, val startIndex: Int, val endIndex: I } return h } - fun endsAtSamePlace(other: Slice): Boolean { - check(base === other.base) - return endIndex == other.endIndex - } - /** Returns the underlying string with this slice replaced by the given string. */ fun replaceSelfWith(s: String): String { - check(base is String) val deltaLength = s.length - length val builder = StringBuilder(base.length + deltaLength) builder.appendRange(base, 0, startIndex) @@ -286,39 +100,4 @@ private constructor(val base: CharSequence, val startIndex: Int, val endIndex: I builder.appendRange(base, endIndex, base.length) return builder.toString() } - - companion object { - @JvmStatic - fun of(base: String, startIndex: Int = 0, endIndex: Int = base.length): Slice { - return Slice(base, startIndex, endIndex) - } - fun concatAll(vararg poolStringsOrStrings: CharSequence): Slice { - if (poolStringsOrStrings.isEmpty()) { - return empty() - } - var total = asPool(poolStringsOrStrings[0]) - for (i in 1 until poolStringsOrStrings.size) { - val next = poolStringsOrStrings[i] - total = if (next is String) total.concat(next) else total.concat(next as Slice) - } - return total - } - private fun visualize(input: CharSequence): String { - return input - .toString() - .replace("\n", "␊") - .replace("\r", "␍") - .replace(" ", "·") - .replace("\t", "»") - } - private fun asPool(sequence: CharSequence): Slice { - return if (sequence is Slice) sequence else of(sequence as String) - } - - /** Returns the empty Slice. */ - @JvmStatic - fun empty(): Slice { - return Slice("", 0, 0) - } - } } diff --git a/selfie-lib/src/commonMain/kotlin/com/diffplug/selfie/SourceFile.kt b/selfie-lib/src/commonMain/kotlin/com/diffplug/selfie/SourceFile.kt index 44a25519..91c777fb 100644 --- a/selfie-lib/src/commonMain/kotlin/com/diffplug/selfie/SourceFile.kt +++ b/selfie-lib/src/commonMain/kotlin/com/diffplug/selfie/SourceFile.kt @@ -22,7 +22,7 @@ package com.diffplug.selfie */ class SourceFile(val filename: String, content: String) { private val unixNewlines = content.indexOf('\r') == -1 - private var contentSlice = Slice.of(content.efficientReplace("\r\n", "\n")) + private var contentSlice = Slice(content.efficientReplace("\r\n", "\n")) /** * Returns the content of the file, possibly modified by * [ToBeLiteral.setLiteralAndGetNewlineDelta]. @@ -44,7 +44,7 @@ class SourceFile(val filename: String, content: String) { val encoded = literalValue.format.encode(literalValue.actual) val existingNewlines = slice.count { it == '\n' } val newNewlines = encoded.count { it == '\n' } - contentSlice = Slice.of(slice.replaceSelfWith(".toBe($encoded)")) + contentSlice = Slice(slice.replaceSelfWith(".toBe($encoded)")) return newNewlines - existingNewlines } diff --git a/selfie-lib/src/commonTest/kotlin/com/diffplug/selfie/SliceTest.kt b/selfie-lib/src/commonTest/kotlin/com/diffplug/selfie/SliceTest.kt index 4ffef5d8..a54a8c8e 100644 --- a/selfie-lib/src/commonTest/kotlin/com/diffplug/selfie/SliceTest.kt +++ b/selfie-lib/src/commonTest/kotlin/com/diffplug/selfie/SliceTest.kt @@ -19,27 +19,10 @@ import io.kotest.matchers.shouldBe import kotlin.test.Test class SliceTest { - @Test - fun afterTest() { - val abcdef = Slice.of("abcdef") - val untilA = abcdef.until("a") - untilA.toString() shouldBe "" - abcdef.after(untilA).toString() shouldBe "abcdef" - val untilC = abcdef.until("c") - untilC.toString() shouldBe "ab" - abcdef.after(untilC).toString() shouldBe "cdef" - val untilF = abcdef.until("f") - untilF.toString() shouldBe "abcde" - abcdef.after(untilF).toString() shouldBe "f" - val untilZ = abcdef.until("z") - untilZ.toString() shouldBe "abcdef" - abcdef.after(untilZ).toString() shouldBe "" - } - @Test fun unixLine() { - Slice.of("A single line").unixLine(1).toString() shouldBe "A single line" - val oneTwoThree = Slice.of("\nI am the first\nI, the second\n\nFOURTH\n") + Slice("A single line").unixLine(1).toString() shouldBe "A single line" + val oneTwoThree = Slice("\nI am the first\nI, the second\n\nFOURTH\n") oneTwoThree.unixLine(1).toString() shouldBe "" oneTwoThree.unixLine(2).toString() shouldBe "I am the first" oneTwoThree.unixLine(3).toString() shouldBe "I, the second" diff --git a/selfie-lib/src/jsMain/kotlin/com/diffplug/selfie/Slice.js.kt b/selfie-lib/src/jsMain/kotlin/com/diffplug/selfie/Slice.js.kt deleted file mode 100644 index faf65040..00000000 --- a/selfie-lib/src/jsMain/kotlin/com/diffplug/selfie/Slice.js.kt +++ /dev/null @@ -1,29 +0,0 @@ -/* - * Copyright (C) 2023 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 - -/** - * A CharSequence which can efficiently subdivide and append itself. - * - * Equal only to other PoolString with the same `toString()`. Use [.sameAs] to compare with other - * kinds of [CharSequence]. - * - * Would be cool to have PoolString.Root which differentiates the String-based ones from - * StringBuilder-based ones. - */ -actual fun groupImpl(slice: Slice, matchResult: MatchResult, group: Int): Slice { - TODO("Not yet implemented") -} diff --git a/selfie-lib/src/jvmMain/kotlin/com/diffplug/selfie/Slice.jvm.kt b/selfie-lib/src/jvmMain/kotlin/com/diffplug/selfie/Slice.jvm.kt deleted file mode 100644 index e3b421fd..00000000 --- a/selfie-lib/src/jvmMain/kotlin/com/diffplug/selfie/Slice.jvm.kt +++ /dev/null @@ -1,30 +0,0 @@ -/* - * Copyright (C) 2023 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 - -/** - * A CharSequence which can efficiently subdivide and append itself. - * - * Equal only to other PoolString with the same `toString()`. Use [.sameAs] to compare with other - * kinds of [CharSequence]. - * - * Would be cool to have PoolString.Root which differentiates the String-based ones from - * StringBuilder-based ones. - */ -internal actual fun groupImpl(slice: Slice, matchResult: MatchResult, group: Int): Slice { - val group = matchResult.groups[group]!! - return slice.subSequence(group.range.start, group.range.endInclusive - 1) -}