From 82b611f428034226f0eef697886874122623be59 Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Thu, 11 Jan 2024 11:31:25 -0800 Subject: [PATCH 1/3] Improve SelfieSettings javadoc. --- .../selfie/junit5/SelfieSettingsAPI.kt | 21 +++++++++++-------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/selfie-runner-junit5/src/main/kotlin/com/diffplug/selfie/junit5/SelfieSettingsAPI.kt b/selfie-runner-junit5/src/main/kotlin/com/diffplug/selfie/junit5/SelfieSettingsAPI.kt index 94e9de11..e4ba8da7 100644 --- a/selfie-runner-junit5/src/main/kotlin/com/diffplug/selfie/junit5/SelfieSettingsAPI.kt +++ b/selfie-runner-junit5/src/main/kotlin/com/diffplug/selfie/junit5/SelfieSettingsAPI.kt @@ -20,23 +20,26 @@ import java.io.File open class SelfieSettingsAPI { /** * It's possible that multiple codepaths from multiple tests can end up writing a single snapshot - * to a single location. If these snapshots are different, you get a "snapshot error" within a - * single invocation, so it can't be resolved by updating the snapshot. - * - * But if they're all writing the same value, it could be okay. By default, we allow this, but you - * can disable it if you want to be more strict. + * to a single location. If all these codepaths are writing the same value, it's fine. But it's a + * bit of a problem waiting to happen, because if they start writing different values, we'll have + * a "snapshot error" even within a single invocation, so it can't be resolved by updating the + * snapshot. By default we let this happen and give a nice error message if it goes wrong, but you + * can disallow it in the first place if you want. */ open val allowMultipleEquivalentWritesToOneLocation: Boolean get() = true /** - * Defaults to `__snapshot__`, null means that snapshots are stored at the same folder location as - * the test that created them. + * Defaults to null, which means that snapshots are stored right next to the test that created + * them. Set to `__snapshots__` to mimic Jest behavior. */ open val snapshotFolderName: String? get() = null - /** By default, the root folder is the first of the standard test directories. */ + /** + * By default, the root folder is the first of the standard test directories. All snapshots are + * stored within the root folder. + */ open val rootFolder: File get() { val userDir = File(System.getProperty("user.dir")) @@ -52,7 +55,7 @@ open class SelfieSettingsAPI { /** * If Selfie should look for test sourcecode in places other than the rootFolder, you can specify - * them here. + * them here. Selfie will not store snapshots in these folders. */ open val otherSourceRoots: List get() { From 72827d39bbe09dc147698e33ea1d037e81228fa7 Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Thu, 11 Jan 2024 11:46:40 -0800 Subject: [PATCH 2/3] Create a mechanism for smuggling initialization errors into the snapshots so they're not hidden. --- .../selfie/junit5/SelfieSettingsAPI.kt | 28 +++++++++++-------- .../selfie/junit5/SnapshotFileLayoutJUnit5.kt | 5 ++++ 2 files changed, 22 insertions(+), 11 deletions(-) diff --git a/selfie-runner-junit5/src/main/kotlin/com/diffplug/selfie/junit5/SelfieSettingsAPI.kt b/selfie-runner-junit5/src/main/kotlin/com/diffplug/selfie/junit5/SelfieSettingsAPI.kt index e4ba8da7..70e1948e 100644 --- a/selfie-runner-junit5/src/main/kotlin/com/diffplug/selfie/junit5/SelfieSettingsAPI.kt +++ b/selfie-runner-junit5/src/main/kotlin/com/diffplug/selfie/junit5/SelfieSettingsAPI.kt @@ -80,20 +80,24 @@ open class SelfieSettingsAPI { "src/test/scala", "src/test/resources") internal fun initialize(): SelfieSettingsAPI { - val settings = System.getProperty("selfie.settings") - if (settings != null && settings.isNotBlank()) { + try { + val settings = System.getProperty("selfie.settings") + if (settings != null && settings.isNotBlank()) { + try { + return instantiate(Class.forName(settings)) + } catch (e: ClassNotFoundException) { + throw Error( + "The system property selfie.settings was set to $settings, but that class could not be found.", + e) + } + } try { - return instantiate(Class.forName(settings)) + return instantiate(Class.forName("selfie.SelfieSettings")) } catch (e: ClassNotFoundException) { - throw Error( - "The system property selfie.settings was set to $settings, but that class could not be found.", - e) + return SelfieSettingsAPI() } - } - try { - return instantiate(Class.forName("selfie.SelfieSettings")) - } catch (e: ClassNotFoundException) { - return SelfieSettingsAPI() + } catch (e: Throwable) { + return SelfieSettingsSmuggleError(e) } } private fun instantiate(clazz: Class<*>): SelfieSettingsAPI { @@ -106,3 +110,5 @@ open class SelfieSettingsAPI { } } } + +class SelfieSettingsSmuggleError(val error: Throwable) : SelfieSettingsAPI() {} diff --git a/selfie-runner-junit5/src/main/kotlin/com/diffplug/selfie/junit5/SnapshotFileLayoutJUnit5.kt b/selfie-runner-junit5/src/main/kotlin/com/diffplug/selfie/junit5/SnapshotFileLayoutJUnit5.kt index 8185bd58..20983805 100644 --- a/selfie-runner-junit5/src/main/kotlin/com/diffplug/selfie/junit5/SnapshotFileLayoutJUnit5.kt +++ b/selfie-runner-junit5/src/main/kotlin/com/diffplug/selfie/junit5/SnapshotFileLayoutJUnit5.kt @@ -22,6 +22,8 @@ import com.diffplug.selfie.guts.SnapshotFileLayout class SnapshotFileLayoutJUnit5(settings: SelfieSettingsAPI, override val fs: FS) : SnapshotFileLayout { + private val smuggledError: Throwable? = + if (settings is SelfieSettingsSmuggleError) settings.error else null override val rootFolder = settings.rootFolder private val otherSourceRoots = settings.otherSourceRoots override val allowMultipleEquivalentWritesToOneLocation = @@ -31,6 +33,9 @@ class SnapshotFileLayoutJUnit5(settings: SelfieSettingsAPI, override val fs: FS) val extension: String = ".ss" private val cache = ThreadLocal?>() override fun sourcePathForCall(call: CallLocation): Path { + if (smuggledError != null) { + throw smuggledError + } val nonNull = sourcePathForCallMaybe(call) ?: throw fs.assertFailed( From a09fe973c3d009965635846dae65f3ea6ea29d79 Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Thu, 11 Jan 2024 12:12:15 -0800 Subject: [PATCH 3/3] More javadoc improvements for `SelfieSettingsAPI`. --- .../kotlin/com/diffplug/selfie/junit5/SelfieSettingsAPI.kt | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/selfie-runner-junit5/src/main/kotlin/com/diffplug/selfie/junit5/SelfieSettingsAPI.kt b/selfie-runner-junit5/src/main/kotlin/com/diffplug/selfie/junit5/SelfieSettingsAPI.kt index 70e1948e..c2429b4e 100644 --- a/selfie-runner-junit5/src/main/kotlin/com/diffplug/selfie/junit5/SelfieSettingsAPI.kt +++ b/selfie-runner-junit5/src/main/kotlin/com/diffplug/selfie/junit5/SelfieSettingsAPI.kt @@ -17,6 +17,12 @@ package com.diffplug.selfie.junit5 import java.io.File +/** + * If you create a class named `SelfieSettings` in the package `selfie`, it must extend this class, + * and you can override the methods below to customize various behaviors of selfie. You can also put + * the settings class somewhere else if you set the `selfie.settings` system property to the fully + * qualified name of the class you want selfie to use. + */ open class SelfieSettingsAPI { /** * It's possible that multiple codepaths from multiple tests can end up writing a single snapshot