From ef934a6e1e0407fbd7efe2239dbe7d2eeef728d6 Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Thu, 25 Jan 2024 10:33:41 -0800 Subject: [PATCH 1/7] Add a test that exercises all java visibility levels. --- .../selfie/junit5/TestMethodVisibility.kt | 49 +++++++++++++++++++ .../junit5/UT_TestMethodVisibility.java | 28 +++++++++++ .../junit5/UT_TestMethodVisibility.ss | 7 +++ 3 files changed, 84 insertions(+) create mode 100644 selfie-runner-junit5/src/test/kotlin/com/diffplug/selfie/junit5/TestMethodVisibility.kt create mode 100644 undertest-junit5/src/test/java/undertest/junit5/UT_TestMethodVisibility.java create mode 100644 undertest-junit5/src/test/java/undertest/junit5/UT_TestMethodVisibility.ss diff --git a/selfie-runner-junit5/src/test/kotlin/com/diffplug/selfie/junit5/TestMethodVisibility.kt b/selfie-runner-junit5/src/test/kotlin/com/diffplug/selfie/junit5/TestMethodVisibility.kt new file mode 100644 index 00000000..173197c3 --- /dev/null +++ b/selfie-runner-junit5/src/test/kotlin/com/diffplug/selfie/junit5/TestMethodVisibility.kt @@ -0,0 +1,49 @@ +/* + * Copyright (C) 2024 DiffPlug + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.diffplug.selfie.junit5 + +import 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 + +@TestMethodOrder(MethodOrderer.OrderAnnotation::class) +@DisableIfTestFails +class TestMethodVisibility : Harness("undertest-junit5") { + @Test @Order(1) + fun writeSelfie() { + gradleWriteSS() + } + + @Test @Order(2) + fun readSelfie() { + gradleReadSS() + ut_snapshot() + .assertContent( + """ + ╔═ isPackage ═╗ + package + ╔═ isProtected ═╗ + protected + ╔═ isPublic ═╗ + public + ╔═ [end of file] ═╗ + + """ + .trimIndent()) + } +} diff --git a/undertest-junit5/src/test/java/undertest/junit5/UT_TestMethodVisibility.java b/undertest-junit5/src/test/java/undertest/junit5/UT_TestMethodVisibility.java new file mode 100644 index 00000000..a7575035 --- /dev/null +++ b/undertest-junit5/src/test/java/undertest/junit5/UT_TestMethodVisibility.java @@ -0,0 +1,28 @@ +package undertest.junit5; + +import static com.diffplug.selfie.Selfie.expectSelfie; + +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.Test; + +public class UT_TestMethodVisibility { + @Test + public void isPublic() { + expectSelfie("public").toMatchDisk(); + } + + @Test + void isPackage() { + expectSelfie("package").toMatchDisk(); + } + + @Test + protected void isProtected() { + expectSelfie("protected").toMatchDisk(); + } + + @Test + private void isPrivate() { + Assertions.fail("Test methods can't be private"); + } +} diff --git a/undertest-junit5/src/test/java/undertest/junit5/UT_TestMethodVisibility.ss b/undertest-junit5/src/test/java/undertest/junit5/UT_TestMethodVisibility.ss new file mode 100644 index 00000000..b83e21bd --- /dev/null +++ b/undertest-junit5/src/test/java/undertest/junit5/UT_TestMethodVisibility.ss @@ -0,0 +1,7 @@ +╔═ isPackage ═╗ +package +╔═ isProtected ═╗ +protected +╔═ isPublic ═╗ +public +╔═ [end of file] ═╗ From 2bc084f77f02014b94a48788e11b413bb6cb9027 Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Thu, 25 Jan 2024 13:20:59 -0800 Subject: [PATCH 2/7] Put SelfieTestExecutionListener into its own file. --- .../junit5/SelfieTestExecutionListener.kt | 309 ----------------- .../selfie/junit5/SnapshotStorageJUnit5.kt | 327 ++++++++++++++++++ 2 files changed, 327 insertions(+), 309 deletions(-) create mode 100644 selfie-runner-junit5/src/main/kotlin/com/diffplug/selfie/junit5/SnapshotStorageJUnit5.kt 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 59659735..94fdecf6 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,312 +15,13 @@ */ package com.diffplug.selfie.junit5 -import com.diffplug.selfie.* -import com.diffplug.selfie.guts.CallStack -import com.diffplug.selfie.guts.CommentTracker -import com.diffplug.selfie.guts.DiskWriteTracker -import com.diffplug.selfie.guts.FS -import com.diffplug.selfie.guts.InlineWriteTracker -import com.diffplug.selfie.guts.LiteralValue -import com.diffplug.selfie.guts.Path -import com.diffplug.selfie.guts.SnapshotFileLayout -import com.diffplug.selfie.guts.SnapshotStorage -import com.diffplug.selfie.guts.SourceFile -import java.nio.charset.StandardCharsets -import java.nio.file.Files -import java.util.concurrent.ConcurrentSkipListSet -import java.util.concurrent.atomic.AtomicReference -import kotlin.streams.asSequence import org.junit.platform.engine.TestExecutionResult import org.junit.platform.engine.support.descriptor.ClassSource import org.junit.platform.engine.support.descriptor.MethodSource import org.junit.platform.launcher.TestExecutionListener import org.junit.platform.launcher.TestIdentifier import org.junit.platform.launcher.TestPlan -import org.opentest4j.AssertionFailedError -internal object FSJava : FS { - override fun fileWrite(file: Path, content: String) = file.writeText(content) - override fun fileRead(file: Path) = file.readText() - /** Walks the files (not directories) which are children and grandchildren of the given path. */ - override fun fileWalk(file: Path, walk: (Sequence) -> T): T = - Files.walk(file.toPath()).use { - walk(it.asSequence().mapNotNull { if (Files.isRegularFile(it)) it.toFile() else null }) - } - override fun name(file: Path): String = file.name - override fun assertFailed(message: String, expected: Any?, actual: Any?): Error = - if (expected == null && actual == null) AssertionFailedError(message) - else AssertionFailedError(message, expected, actual) -} -private fun lowercaseFromEnvOrSys(key: String): String? { - val env = System.getenv(key)?.lowercase() - if (!env.isNullOrEmpty()) { - return env - } - val system = System.getProperty(key)?.lowercase() - if (!system.isNullOrEmpty()) { - return system - } - return null -} -private fun calcMode(): Mode { - val override = lowercaseFromEnvOrSys("selfie") ?: lowercaseFromEnvOrSys("SELFIE") - if (override != null) { - return Mode.valueOf(override) - } - val ci = lowercaseFromEnvOrSys("ci") ?: lowercaseFromEnvOrSys("CI") - return if (ci == "true") Mode.readonly else Mode.interactive -} - -/** Routes between `toMatchDisk()` calls and the snapshot file / pruning machinery. */ -internal object SnapshotStorageJUnit5 : SnapshotStorage { - @JvmStatic fun initStorage(): SnapshotStorage = this - override val fs = FSJava - override val mode = calcMode() - override val layout: SnapshotFileLayout - get() = classAndMethod().clazz.parent.layout - - private class ClassMethod(val clazz: ClassProgress, val method: String) - private val threadCtx = ThreadLocal() - private fun classAndMethod() = - threadCtx.get() - ?: throw AssertionError( - "Selfie `toMatchDisk` must be called only on the original thread.") - override fun sourceFileHasWritableComment(call: CallStack): Boolean { - val cm = classAndMethod() - return cm.clazz.parent.commentTracker!!.hasWritableComment(call, cm.clazz.parent.layout) - } - private fun suffix(sub: String) = if (sub == "") "" else "/$sub" - override fun readDisk(sub: String, call: CallStack): Snapshot? { - val cm = classAndMethod() - val suffix = suffix(sub) - return cm.clazz.read(cm.method, suffix) - } - override fun writeDisk(actual: Snapshot, sub: String, call: CallStack) { - val cm = classAndMethod() - val suffix = suffix(sub) - cm.clazz.write(cm.method, suffix, actual, call, cm.clazz.parent.layout) - } - override fun keep(subOrKeepAll: String?) { - val cm = classAndMethod() - if (subOrKeepAll == null) { - cm.clazz.keep(cm.method, null) - } else { - cm.clazz.keep(cm.method, suffix(subOrKeepAll)) - } - } - override fun writeInline(literalValue: LiteralValue<*>, call: CallStack) { - val cm = - threadCtx.get() - ?: throw AssertionError("Selfie `toBe` must be called only on the original thread.") - cm.clazz.writeInline(call, literalValue, cm.clazz.parent.layout) - } - internal fun start(clazz: ClassProgress, method: String) { - val current = threadCtx.get() - check(current == null) { - "THREAD ERROR: ${current!!.clazz.className}#${current.method} is in progress, cannot start $clazz#$method" - } - threadCtx.set(ClassMethod(clazz, method)) - } - internal fun finish(clazz: ClassProgress, method: String) { - val current = threadCtx.get() - check(current != null) { - "THREAD ERROR: no method is in progress, cannot finish $clazz#$method" - } - check(current.clazz == clazz && current.method == method) { - "THREAD ERROR: ${current.clazz.className}#${current.method} is in progress, cannot finish ${clazz.className}#$method" - } - threadCtx.set(null) - } -} - -/** Tracks the progress of test runs within a single class, so that snapshots can be pruned. */ -internal class ClassProgress(val parent: Progress, val className: String) { - companion object { - val TERMINATED = - ArrayMap.empty().plus(" ~ f!n1shed ~ ", MethodSnapshotGC()) - } - private fun assertNotTerminated() { - assert(methods !== TERMINATED) { "Cannot call methods on a terminated ClassProgress" } - } - - 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, isTest: Boolean) { - assertNotTerminated() - if (isTest) { - SnapshotStorageJUnit5.start(this, method) - } - assert(method.indexOf('/') == -1) { "Method name cannot contain '/', was $method" } - methods = methods.plusOrNoOp(method, MethodSnapshotGC()) - } - @Synchronized fun finishedMethodWithSuccess(method: String, isTest: Boolean, success: Boolean) { - assertNotTerminated() - if (isTest) { - SnapshotStorageJUnit5.finish(this, method) - } - if (!success) { - methods[method]!!.keepAll() - } - } - @Synchronized fun finishedClassWithSuccess(success: Boolean) { - assertNotTerminated() - if (inlineWriteTracker!!.hasWrites()) { - inlineWriteTracker!!.persistWrites(parent.layout) - } - if (file != null) { - val staleSnapshotIndices = - MethodSnapshotGC.findStaleSnapshotsWithin(className, file!!.snapshots, methods) - if (staleSnapshotIndices.isNotEmpty() || file!!.wasSetAtTestTime) { - file!!.removeAllIndices(staleSnapshotIndices) - val snapshotPath = parent.layout.snapshotPathForClass(className) - if (file!!.snapshots.isEmpty()) { - deleteFileAndParentDirIfEmpty(snapshotPath) - } else { - parent.markPathAsWritten(parent.layout.snapshotPathForClass(className)) - Files.createDirectories(snapshotPath.toPath().parent) - Files.newBufferedWriter(snapshotPath.toPath(), StandardCharsets.UTF_8).use { writer -> - file!!.serialize(writer) - } - } - } - } else { - // we never read or wrote to the file - val isStale = MethodSnapshotGC.isUnusedSnapshotFileStale(className, methods, success) - if (isStale) { - val snapshotFile = parent.layout.snapshotPathForClass(className) - deleteFileAndParentDirIfEmpty(snapshotFile) - } - } - // 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 - @Synchronized fun keep(method: String, suffixOrAll: String?) { - assertNotTerminated() - if (suffixOrAll == null) { - methods[method]!!.keepAll() - } else { - methods[method]!!.keepSuffix(suffixOrAll) - } - } - @Synchronized fun writeInline(call: CallStack, literalValue: LiteralValue<*>, layout: SnapshotFileLayout) { - inlineWriteTracker!!.record(call, literalValue, layout) - } - @Synchronized fun write( - method: String, - suffix: String, - snapshot: Snapshot, - callStack: CallStack, - layout: SnapshotFileLayout - ) { - assertNotTerminated() - val key = "$method$suffix" - diskWriteTracker!!.record(key, snapshot, callStack, layout) - methods[method]!!.keepSuffix(suffix) - read().setAtTestTime(key, snapshot) - } - @Synchronized fun read(method: String, suffix: String): Snapshot? { - assertNotTerminated() - val snapshot = read().snapshots["$method$suffix"] - if (snapshot != null) { - methods[method]!!.keepSuffix(suffix) - } - return snapshot - } - private fun read(): SnapshotFile { - if (file == null) { - val snapshotPath = parent.layout.snapshotPathForClass(className).toPath() - file = - if (Files.exists(snapshotPath) && Files.isRegularFile(snapshotPath)) { - val content = Files.readAllBytes(snapshotPath) - SnapshotFile.parse(SnapshotValueReader.of(content)) - } else { - SnapshotFile.createEmptyWithUnixNewlines(parent.layout.unixNewlines) - } - } - return file!! - } -} -/** - * Responsible for - * - lazily determining the snapshot file layout - * - pruning unused snapshot files - */ -internal class Progress { - val settings = SelfieSettingsAPI.initialize() - val layout = SnapshotFileLayoutJUnit5(settings, SnapshotStorageJUnit5.fs) - var commentTracker: CommentTracker? = CommentTracker() - - private var progressPerClass = ArrayMap.empty() - private fun forClass(className: String) = synchronized(this) { progressPerClass[className]!! } - - // TestExecutionListener - fun start(className: String, method: String?, isTest: Boolean) { - if (method == null) { - synchronized(this) { - progressPerClass = progressPerClass.plus(className, ClassProgress(this, className)) - } - } else { - forClass(className).startMethod(method, isTest) - } - } - fun skip(className: String, method: String?, isTest: Boolean) { - if (method == null) { - start(className, null, isTest) - finishWithSuccess(className, null, isTest, false) - } else { - // thanks to reflection, we don't have to rely on method skip events - } - } - fun finishWithSuccess(className: String, method: String?, isTest: Boolean, isSuccess: Boolean) { - forClass(className).let { - if (method != null) { - it.finishedMethodWithSuccess(method, isTest, isSuccess) - } else { - it.finishedClassWithSuccess(isSuccess) - } - } - } - - private var checkForInvalidStale: AtomicReference?> = - AtomicReference(ConcurrentSkipListSet()) - internal fun markPathAsWritten(path: Path) { - val written = - checkForInvalidStale.get() - ?: throw AssertionError("Snapshot file is being written after all tests were finished.") - written.add(path) - } - fun finishedAllTests() { - val pathsWithOnce = commentTracker!!.pathsWithOnce() - commentTracker = null - if (SnapshotStorageJUnit5.mode != Mode.readonly) { - for (path in pathsWithOnce) { - val source = SourceFile(layout.fs.name(path), layout.fs.fileRead(path)) - source.removeSelfieOnceComments() - layout.fs.fileWrite(path, source.asString) - } - } - val snapshotsFilesWrittenToDisk = - checkForInvalidStale.getAndSet(null) - ?: throw AssertionError("finishedAllTests() was called more than once.") - for (stale in findStaleSnapshotFiles(layout)) { - val staleFile = layout.snapshotPathForClass(stale) - if (snapshotsFilesWrittenToDisk.contains(staleFile)) { - throw AssertionError( - "Selfie wrote a snapshot and then marked it stale for deletion it in the same run: $staleFile\nSelfie will delete this snapshot on the next run, which is bad! Why is Selfie marking this snapshot as stale?") - } else { - deleteFileAndParentDirIfEmpty(staleFile) - } - } - } -} /** This is automatically registered at runtime thanks to `META-INF/services`. */ class SelfieTestExecutionListener : TestExecutionListener { private val progress = Progress() @@ -369,13 +70,3 @@ class SelfieTestExecutionListener : TestExecutionListener { } } } -private fun deleteFileAndParentDirIfEmpty(snapshotFile: Path) { - if (Files.isRegularFile(snapshotFile.toPath())) { - Files.delete(snapshotFile.toPath()) - // if the parent folder is now empty, delete it - val parent = snapshotFile.toPath().parent!! - if (Files.list(parent).use { !it.findAny().isPresent }) { - Files.delete(parent) - } - } -} diff --git a/selfie-runner-junit5/src/main/kotlin/com/diffplug/selfie/junit5/SnapshotStorageJUnit5.kt b/selfie-runner-junit5/src/main/kotlin/com/diffplug/selfie/junit5/SnapshotStorageJUnit5.kt new file mode 100644 index 00000000..d132c340 --- /dev/null +++ b/selfie-runner-junit5/src/main/kotlin/com/diffplug/selfie/junit5/SnapshotStorageJUnit5.kt @@ -0,0 +1,327 @@ +/* + * Copyright (C) 2023-2024 DiffPlug + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.diffplug.selfie.junit5 + +import com.diffplug.selfie.* +import com.diffplug.selfie.guts.CallStack +import com.diffplug.selfie.guts.CommentTracker +import com.diffplug.selfie.guts.DiskWriteTracker +import com.diffplug.selfie.guts.FS +import com.diffplug.selfie.guts.InlineWriteTracker +import com.diffplug.selfie.guts.LiteralValue +import com.diffplug.selfie.guts.Path +import com.diffplug.selfie.guts.SnapshotFileLayout +import com.diffplug.selfie.guts.SnapshotStorage +import com.diffplug.selfie.guts.SourceFile +import java.nio.charset.StandardCharsets +import java.nio.file.Files +import java.util.concurrent.ConcurrentSkipListSet +import java.util.concurrent.atomic.AtomicReference +import kotlin.streams.asSequence +import org.opentest4j.AssertionFailedError + +internal object FSJava : FS { + override fun fileWrite(file: Path, content: String) = file.writeText(content) + override fun fileRead(file: Path) = file.readText() + /** Walks the files (not directories) which are children and grandchildren of the given path. */ + override fun fileWalk(file: Path, walk: (Sequence) -> T): T = + Files.walk(file.toPath()).use { + walk(it.asSequence().mapNotNull { if (Files.isRegularFile(it)) it.toFile() else null }) + } + override fun name(file: Path): String = file.name + override fun assertFailed(message: String, expected: Any?, actual: Any?): Error = + if (expected == null && actual == null) AssertionFailedError(message) + else AssertionFailedError(message, expected, actual) +} +private fun lowercaseFromEnvOrSys(key: String): String? { + val env = System.getenv(key)?.lowercase() + if (!env.isNullOrEmpty()) { + return env + } + val system = System.getProperty(key)?.lowercase() + if (!system.isNullOrEmpty()) { + return system + } + return null +} +private fun calcMode(): Mode { + val override = lowercaseFromEnvOrSys("selfie") ?: lowercaseFromEnvOrSys("SELFIE") + if (override != null) { + return Mode.valueOf(override) + } + val ci = lowercaseFromEnvOrSys("ci") ?: lowercaseFromEnvOrSys("CI") + return if (ci == "true") Mode.readonly else Mode.interactive +} + +/** Routes between `toMatchDisk()` calls and the snapshot file / pruning machinery. */ +internal object SnapshotStorageJUnit5 : SnapshotStorage { + @JvmStatic fun initStorage(): SnapshotStorage = this + override val fs = FSJava + override val mode = calcMode() + override val layout: SnapshotFileLayout + get() = classAndMethod().clazz.parent.layout + + private class ClassMethod(val clazz: ClassProgress, val method: String) + private val threadCtx = ThreadLocal() + private fun classAndMethod() = + threadCtx.get() + ?: throw AssertionError( + "Selfie `toMatchDisk` must be called only on the original thread.") + override fun sourceFileHasWritableComment(call: CallStack): Boolean { + val cm = classAndMethod() + return cm.clazz.parent.commentTracker!!.hasWritableComment(call, cm.clazz.parent.layout) + } + private fun suffix(sub: String) = if (sub == "") "" else "/$sub" + override fun readDisk(sub: String, call: CallStack): Snapshot? { + val cm = classAndMethod() + val suffix = suffix(sub) + return cm.clazz.read(cm.method, suffix) + } + override fun writeDisk(actual: Snapshot, sub: String, call: CallStack) { + val cm = classAndMethod() + val suffix = suffix(sub) + cm.clazz.write(cm.method, suffix, actual, call, cm.clazz.parent.layout) + } + override fun keep(subOrKeepAll: String?) { + val cm = classAndMethod() + if (subOrKeepAll == null) { + cm.clazz.keep(cm.method, null) + } else { + cm.clazz.keep(cm.method, suffix(subOrKeepAll)) + } + } + override fun writeInline(literalValue: LiteralValue<*>, call: CallStack) { + val cm = + threadCtx.get() + ?: throw AssertionError("Selfie `toBe` must be called only on the original thread.") + cm.clazz.writeInline(call, literalValue, cm.clazz.parent.layout) + } + internal fun start(clazz: ClassProgress, method: String) { + val current = threadCtx.get() + check(current == null) { + "THREAD ERROR: ${current!!.clazz.className}#${current.method} is in progress, cannot start $clazz#$method" + } + threadCtx.set(ClassMethod(clazz, method)) + } + internal fun finish(clazz: ClassProgress, method: String) { + val current = threadCtx.get() + check(current != null) { + "THREAD ERROR: no method is in progress, cannot finish $clazz#$method" + } + check(current.clazz == clazz && current.method == method) { + "THREAD ERROR: ${current.clazz.className}#${current.method} is in progress, cannot finish ${clazz.className}#$method" + } + threadCtx.set(null) + } +} + +/** Tracks the progress of test runs within a single class, so that snapshots can be pruned. */ +internal class ClassProgress(val parent: Progress, val className: String) { + companion object { + val TERMINATED = + ArrayMap.empty().plus(" ~ f!n1shed ~ ", MethodSnapshotGC()) + } + private fun assertNotTerminated() { + assert(methods !== TERMINATED) { "Cannot call methods on a terminated ClassProgress" } + } + + 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, isTest: Boolean) { + assertNotTerminated() + if (isTest) { + SnapshotStorageJUnit5.start(this, method) + } + assert(method.indexOf('/') == -1) { "Method name cannot contain '/', was $method" } + methods = methods.plusOrNoOp(method, MethodSnapshotGC()) + } + @Synchronized fun finishedMethodWithSuccess(method: String, isTest: Boolean, success: Boolean) { + assertNotTerminated() + if (isTest) { + SnapshotStorageJUnit5.finish(this, method) + } + if (!success) { + methods[method]!!.keepAll() + } + } + @Synchronized fun finishedClassWithSuccess(success: Boolean) { + assertNotTerminated() + if (inlineWriteTracker!!.hasWrites()) { + inlineWriteTracker!!.persistWrites(parent.layout) + } + if (file != null) { + val staleSnapshotIndices = + MethodSnapshotGC.findStaleSnapshotsWithin(className, file!!.snapshots, methods) + if (staleSnapshotIndices.isNotEmpty() || file!!.wasSetAtTestTime) { + file!!.removeAllIndices(staleSnapshotIndices) + val snapshotPath = parent.layout.snapshotPathForClass(className) + if (file!!.snapshots.isEmpty()) { + deleteFileAndParentDirIfEmpty(snapshotPath) + } else { + parent.markPathAsWritten(parent.layout.snapshotPathForClass(className)) + Files.createDirectories(snapshotPath.toPath().parent) + Files.newBufferedWriter(snapshotPath.toPath(), StandardCharsets.UTF_8).use { writer -> + file!!.serialize(writer) + } + } + } + } else { + // we never read or wrote to the file + val isStale = MethodSnapshotGC.isUnusedSnapshotFileStale(className, methods, success) + if (isStale) { + val snapshotFile = parent.layout.snapshotPathForClass(className) + deleteFileAndParentDirIfEmpty(snapshotFile) + } + } + // 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 + @Synchronized fun keep(method: String, suffixOrAll: String?) { + assertNotTerminated() + if (suffixOrAll == null) { + methods[method]!!.keepAll() + } else { + methods[method]!!.keepSuffix(suffixOrAll) + } + } + @Synchronized fun writeInline(call: CallStack, literalValue: LiteralValue<*>, layout: SnapshotFileLayout) { + inlineWriteTracker!!.record(call, literalValue, layout) + } + @Synchronized fun write( + method: String, + suffix: String, + snapshot: Snapshot, + callStack: CallStack, + layout: SnapshotFileLayout + ) { + assertNotTerminated() + val key = "$method$suffix" + diskWriteTracker!!.record(key, snapshot, callStack, layout) + methods[method]!!.keepSuffix(suffix) + read().setAtTestTime(key, snapshot) + } + @Synchronized fun read(method: String, suffix: String): Snapshot? { + assertNotTerminated() + val snapshot = read().snapshots["$method$suffix"] + if (snapshot != null) { + methods[method]!!.keepSuffix(suffix) + } + return snapshot + } + private fun read(): SnapshotFile { + if (file == null) { + val snapshotPath = parent.layout.snapshotPathForClass(className).toPath() + file = + if (Files.exists(snapshotPath) && Files.isRegularFile(snapshotPath)) { + val content = Files.readAllBytes(snapshotPath) + SnapshotFile.parse(SnapshotValueReader.of(content)) + } else { + SnapshotFile.createEmptyWithUnixNewlines(parent.layout.unixNewlines) + } + } + return file!! + } +} +/** + * Responsible for + * - lazily determining the snapshot file layout + * - pruning unused snapshot files + */ +internal class Progress { + val settings = SelfieSettingsAPI.initialize() + val layout = SnapshotFileLayoutJUnit5(settings, SnapshotStorageJUnit5.fs) + var commentTracker: CommentTracker? = CommentTracker() + + private var progressPerClass = ArrayMap.empty() + private fun forClass(className: String) = synchronized(this) { progressPerClass[className]!! } + + // TestExecutionListener + fun start(className: String, method: String?, isTest: Boolean) { + if (method == null) { + synchronized(this) { + progressPerClass = progressPerClass.plus(className, ClassProgress(this, className)) + } + } else { + forClass(className).startMethod(method, isTest) + } + } + fun skip(className: String, method: String?, isTest: Boolean) { + if (method == null) { + start(className, null, isTest) + finishWithSuccess(className, null, isTest, false) + } else { + // thanks to reflection, we don't have to rely on method skip events + } + } + fun finishWithSuccess(className: String, method: String?, isTest: Boolean, isSuccess: Boolean) { + forClass(className).let { + if (method != null) { + it.finishedMethodWithSuccess(method, isTest, isSuccess) + } else { + it.finishedClassWithSuccess(isSuccess) + } + } + } + + private var checkForInvalidStale: AtomicReference?> = + AtomicReference(ConcurrentSkipListSet()) + internal fun markPathAsWritten(path: Path) { + val written = + checkForInvalidStale.get() + ?: throw AssertionError("Snapshot file is being written after all tests were finished.") + written.add(path) + } + fun finishedAllTests() { + val pathsWithOnce = commentTracker!!.pathsWithOnce() + commentTracker = null + if (SnapshotStorageJUnit5.mode != Mode.readonly) { + for (path in pathsWithOnce) { + val source = SourceFile(layout.fs.name(path), layout.fs.fileRead(path)) + source.removeSelfieOnceComments() + layout.fs.fileWrite(path, source.asString) + } + } + val snapshotsFilesWrittenToDisk = + checkForInvalidStale.getAndSet(null) + ?: throw AssertionError("finishedAllTests() was called more than once.") + for (stale in findStaleSnapshotFiles(layout)) { + val staleFile = layout.snapshotPathForClass(stale) + if (snapshotsFilesWrittenToDisk.contains(staleFile)) { + throw AssertionError( + "Selfie wrote a snapshot and then marked it stale for deletion it in the same run: $staleFile\nSelfie will delete this snapshot on the next run, which is bad! Why is Selfie marking this snapshot as stale?") + } else { + deleteFileAndParentDirIfEmpty(staleFile) + } + } + } +} +private fun deleteFileAndParentDirIfEmpty(snapshotFile: Path) { + if (Files.isRegularFile(snapshotFile.toPath())) { + Files.delete(snapshotFile.toPath()) + // if the parent folder is now empty, delete it + val parent = snapshotFile.toPath().parent!! + if (Files.list(parent).use { !it.findAny().isPresent }) { + Files.delete(parent) + } + } +} From 9fda01ff36b5c4f60c0064db15fa41a69de6a46b Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Thu, 25 Jan 2024 14:49:56 -0800 Subject: [PATCH 3/7] Fix typo. --- .../kotlin/com/diffplug/selfie/junit5/SnapshotStorageJUnit5.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/selfie-runner-junit5/src/main/kotlin/com/diffplug/selfie/junit5/SnapshotStorageJUnit5.kt b/selfie-runner-junit5/src/main/kotlin/com/diffplug/selfie/junit5/SnapshotStorageJUnit5.kt index d132c340..4624d6c6 100644 --- a/selfie-runner-junit5/src/main/kotlin/com/diffplug/selfie/junit5/SnapshotStorageJUnit5.kt +++ b/selfie-runner-junit5/src/main/kotlin/com/diffplug/selfie/junit5/SnapshotStorageJUnit5.kt @@ -308,7 +308,7 @@ internal class Progress { val staleFile = layout.snapshotPathForClass(stale) if (snapshotsFilesWrittenToDisk.contains(staleFile)) { throw AssertionError( - "Selfie wrote a snapshot and then marked it stale for deletion it in the same run: $staleFile\nSelfie will delete this snapshot on the next run, which is bad! Why is Selfie marking this snapshot as stale?") + "Selfie wrote a snapshot and then marked it stale for deletion in the same run: $staleFile\nSelfie will delete this snapshot on the next run, which is bad! Why is Selfie marking this snapshot as stale?") } else { deleteFileAndParentDirIfEmpty(staleFile) } From 8e59325231f8ebf78c0f1053305019bc8f00c678 Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Thu, 25 Jan 2024 14:53:06 -0800 Subject: [PATCH 4/7] Aha, as long as there was at least one `isPublic` then the file wasn't marked stale. Now it should fail... --- .../com/diffplug/selfie/junit5/TestMethodVisibility.kt | 2 -- .../test/java/undertest/junit5/UT_TestMethodVisibility.java | 5 ----- .../test/java/undertest/junit5/UT_TestMethodVisibility.ss | 2 -- 3 files changed, 9 deletions(-) diff --git a/selfie-runner-junit5/src/test/kotlin/com/diffplug/selfie/junit5/TestMethodVisibility.kt b/selfie-runner-junit5/src/test/kotlin/com/diffplug/selfie/junit5/TestMethodVisibility.kt index 173197c3..2d3c775c 100644 --- a/selfie-runner-junit5/src/test/kotlin/com/diffplug/selfie/junit5/TestMethodVisibility.kt +++ b/selfie-runner-junit5/src/test/kotlin/com/diffplug/selfie/junit5/TestMethodVisibility.kt @@ -39,8 +39,6 @@ class TestMethodVisibility : Harness("undertest-junit5") { package ╔═ isProtected ═╗ protected - ╔═ isPublic ═╗ - public ╔═ [end of file] ═╗ """ diff --git a/undertest-junit5/src/test/java/undertest/junit5/UT_TestMethodVisibility.java b/undertest-junit5/src/test/java/undertest/junit5/UT_TestMethodVisibility.java index a7575035..49ba8dce 100644 --- a/undertest-junit5/src/test/java/undertest/junit5/UT_TestMethodVisibility.java +++ b/undertest-junit5/src/test/java/undertest/junit5/UT_TestMethodVisibility.java @@ -6,11 +6,6 @@ import org.junit.jupiter.api.Test; public class UT_TestMethodVisibility { - @Test - public void isPublic() { - expectSelfie("public").toMatchDisk(); - } - @Test void isPackage() { expectSelfie("package").toMatchDisk(); diff --git a/undertest-junit5/src/test/java/undertest/junit5/UT_TestMethodVisibility.ss b/undertest-junit5/src/test/java/undertest/junit5/UT_TestMethodVisibility.ss index b83e21bd..0f6d1bc6 100644 --- a/undertest-junit5/src/test/java/undertest/junit5/UT_TestMethodVisibility.ss +++ b/undertest-junit5/src/test/java/undertest/junit5/UT_TestMethodVisibility.ss @@ -2,6 +2,4 @@ package ╔═ isProtected ═╗ protected -╔═ isPublic ═╗ -public ╔═ [end of file] ═╗ From 6756a5fdc3586e871e3d3867197e45f78721a40d Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Thu, 25 Jan 2024 14:58:56 -0800 Subject: [PATCH 5/7] And now its fixed. --- .../kotlin/com/diffplug/selfie/junit5/SelfieGC.kt | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/selfie-runner-junit5/src/main/kotlin/com/diffplug/selfie/junit5/SelfieGC.kt b/selfie-runner-junit5/src/main/kotlin/com/diffplug/selfie/junit5/SelfieGC.kt index 68c19a2b..c91b26a9 100644 --- a/selfie-runner-junit5/src/main/kotlin/com/diffplug/selfie/junit5/SelfieGC.kt +++ b/selfie-runner-junit5/src/main/kotlin/com/diffplug/selfie/junit5/SelfieGC.kt @@ -52,9 +52,16 @@ internal fun findStaleSnapshotFiles(layout: SnapshotFileLayoutJUnit5): List - testAnnotations.any { method.isAnnotationPresent(it) } + var clazz: Class<*> = Class.forName(key) + while (clazz != Object::class.java) { + if (clazz.declaredMethods.any { method -> + testAnnotations.any { method.isAnnotationPresent(it) } + }) { + return true + } + clazz = clazz.superclass } + return false } catch (e: ClassNotFoundException) { false } From f95f19393e59e52aeee96f41d149e4189867d48f Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Thu, 25 Jan 2024 15:01:10 -0800 Subject: [PATCH 6/7] And it stays fixed even if the class itself doesn't declare any tests. --- .../junit5/UT_TestMethodVisibility.java | 13 ++--------- .../UT_TestMethodVisibilityParentClass.java | 23 +++++++++++++++++++ 2 files changed, 25 insertions(+), 11 deletions(-) create mode 100644 undertest-junit5/src/test/java/undertest/junit5/UT_TestMethodVisibilityParentClass.java diff --git a/undertest-junit5/src/test/java/undertest/junit5/UT_TestMethodVisibility.java b/undertest-junit5/src/test/java/undertest/junit5/UT_TestMethodVisibility.java index 49ba8dce..3e4b2432 100644 --- a/undertest-junit5/src/test/java/undertest/junit5/UT_TestMethodVisibility.java +++ b/undertest-junit5/src/test/java/undertest/junit5/UT_TestMethodVisibility.java @@ -5,17 +5,8 @@ import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.Test; -public class UT_TestMethodVisibility { - @Test - void isPackage() { - expectSelfie("package").toMatchDisk(); - } - - @Test - protected void isProtected() { - expectSelfie("protected").toMatchDisk(); - } - +public class UT_TestMethodVisibility extends UT_TestMethodVisibilityParentClass { + // get test methods from parent class @Test private void isPrivate() { Assertions.fail("Test methods can't be private"); diff --git a/undertest-junit5/src/test/java/undertest/junit5/UT_TestMethodVisibilityParentClass.java b/undertest-junit5/src/test/java/undertest/junit5/UT_TestMethodVisibilityParentClass.java new file mode 100644 index 00000000..a799467c --- /dev/null +++ b/undertest-junit5/src/test/java/undertest/junit5/UT_TestMethodVisibilityParentClass.java @@ -0,0 +1,23 @@ +package undertest.junit5; + +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.Test; + +import static com.diffplug.selfie.Selfie.expectSelfie; + +class UT_TestMethodVisibilityParentClass { + @Test + void isPackage() { + expectSelfie("package").toMatchDisk(); + } + + @Test + protected void isProtected() { + expectSelfie("protected").toMatchDisk(); + } + + @Test + private void isPrivate() { + Assertions.fail("Test methods can't be private"); + } +} From 7ce5d55fc1ec2f240ee29f6c66c3f0baaf119372 Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Thu, 25 Jan 2024 15:04:19 -0800 Subject: [PATCH 7/7] Fix changelog. --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index e891b9c4..803aea21 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,8 @@ 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 was erroneously garbage collecting snapshots when a test class contained no `@Test public` methods. ([#124](https://github.com/diffplug/selfie/pull/124) fixes [#123](https://github.com/diffplug/selfie/issues/123)) ## [1.1.0] - 2024-01-21 ### Added