Skip to content

Track double writes (prepare for literal snapshots). #15

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 9 commits into from
Sep 9, 2023
Original file line number Diff line number Diff line change
Expand Up @@ -127,9 +127,6 @@ class SnapshotFile {

var wasSetAtTestTime: Boolean = false
fun setAtTestTime(key: String, snapshot: Snapshot) {
// TODO: track whenever a snapshot is set, so that we can:
// - warn about duplicate snapshots when they are equal
// - give good errors when they are not
val newSnapshots = snapshots.plusOrReplace(key, snapshot)
if (newSnapshots !== snapshots) {
snapshots = newSnapshots
Expand Down
55 changes: 30 additions & 25 deletions selfie-runner-junit5/src/main/kotlin/com/diffplug/selfie/RW.kt
Original file line number Diff line number Diff line change
Expand Up @@ -21,34 +21,39 @@ package com.diffplug.selfie
* - if environment variable or system property named `ci` or `CI` with value `true` or `TRUE`
* - then Selfie is read-only and errors out on a snapshot mismatch
* - if environment variable or system property named `selfie` or `SELFIE`
* - its value should be either `read` or `write` (case-insensitive)
* - that will override the presence of `CI`
* - its value should be either `read`, `write`, or `writeonce` (case-insensitive)
* - `write` allows a single snapshot to be set multiple times within a test, so long as it is
* the same value. `writeonce` errors as soon as a snapshot is set twice even to the same
* value.
* - selfie, if set, will override the presence of `CI`
*/
internal object RW {
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
internal enum class RW {
read,
write,
writeonce;

companion object {
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
}
return null
}
private fun calcIsWrite(): Boolean {
val override = lowercaseFromEnvOrSys("selfie") ?: lowercaseFromEnvOrSys("SELFIE")
if (override != null) {
return when (override) {
"read" -> false
"write" -> true
else ->
throw IllegalArgumentException(
"Expected 'selfie' to be 'read' or 'write', but was '$override'")
private fun calcRW(): RW {
val override = lowercaseFromEnvOrSys("selfie") ?: lowercaseFromEnvOrSys("SELFIE")
if (override != null) {
return RW.valueOf(override)
}
val ci = lowercaseFromEnvOrSys("ci") ?: lowercaseFromEnvOrSys("CI")
return if (ci == "true") read else write
}
val ci = lowercaseFromEnvOrSys("ci") ?: lowercaseFromEnvOrSys("CI")
return ci != "true"
val mode = calcRW()
val isWrite = mode != read
val isWriteOnce = mode == writeonce
}
val isWrite = calcIsWrite()
}
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,7 @@ internal class ClassProgress(val className: String) {

private var file: SnapshotFile? = null
private var methods = ArrayMap.empty<String, MethodSnapshotGC>()
private var diskWriteTracker: DiskWriteTracker? = DiskWriteTracker()
// the methods below called by the TestExecutionListener on its runtime thread
@Synchronized fun startMethod(method: String) {
assertNotTerminated()
Expand Down Expand Up @@ -132,6 +133,7 @@ internal class ClassProgress(val className: String) {
}
// now that we are done, allow our contents to be GC'ed
methods = TERMINATED
diskWriteTracker = null
file = null
}
// the methods below are called from the test thread for I/O on snapshots
Expand All @@ -145,8 +147,10 @@ internal class ClassProgress(val className: String) {
}
@Synchronized fun write(method: String, suffix: String, snapshot: Snapshot) {
assertNotTerminated()
val key = "$method$suffix"
diskWriteTracker!!.record(key, snapshot, recordCall())
methods[method]!!.keepSuffix(suffix)
read().setAtTestTime("$method$suffix", snapshot)
read().setAtTestTime(key, snapshot)
}
@Synchronized fun read(
method: String,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
/*
* 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 com.diffplug.selfie.RW
import com.diffplug.selfie.Snapshot
import java.util.stream.Collectors

/** Represents the line at which user code called into Selfie. */
data class CallLocation(val subpath: String, val line: Int) : Comparable<CallLocation> {
override fun compareTo(other: CallLocation): Int {
val subpathCompare = subpath.compareTo(other.subpath)
return if (subpathCompare != 0) subpathCompare else line.compareTo(other.line)
}
override fun toString(): String = "$subpath:$line"
}
/** Represents the callstack above a given CallLocation. */
class CallStack(val location: CallLocation, val restOfStack: List<CallLocation>) {
override fun toString(): String = "$location"
}
/** Generates a CallLocation and the CallStack behind it. */
fun recordCall(): CallStack {
val calls =
StackWalker.getInstance().walk { frames ->
frames
.skip(1)
.map { CallLocation(it.className.replace('.', '/') + ".kt", it.lineNumber) }
.collect(Collectors.toList())
}
return CallStack(calls.removeAt(0), calls)
}
/** The first write at a given spot. */
internal class FirstWrite<T>(val snapshot: T, val callStack: CallStack)

/** For tracking the writes of disk snapshots literals. */
internal open class WriteTracker<K : Comparable<K>, V> {
val writes = mutableMapOf<K, FirstWrite<V>>()
protected fun recordInternal(key: K, snapshot: V, call: CallStack) {
val existing = writes.putIfAbsent(key, FirstWrite(snapshot, call))
if (existing != null) {
if (existing.snapshot != snapshot) {
throw org.opentest4j.AssertionFailedError(
"Snapshot was set to multiple values:\nfirst time:${existing.callStack}\n\nthis time:${call}",
existing.snapshot,
snapshot)
} else if (RW.isWriteOnce) {
throw org.opentest4j.AssertionFailedError(
"Snapshot was set to the same value multiple times.", existing.callStack, call)
}
}
}
}

internal class DiskWriteTracker : WriteTracker<String, Snapshot>() {
fun record(key: String, snapshot: Snapshot, call: CallStack) {
recordInternal(key, snapshot, call)
}
}

class LiteralValue {
// TODO: String, Int, Long, Boolean, etc
}

internal class InlineWriteTracker : WriteTracker<CallLocation, LiteralValue>() {
fun record(call: CallStack, snapshot: LiteralValue) {
recordInternal(call.location, snapshot, call)
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
/*
* 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 io.kotest.matchers.string.shouldStartWith
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

/** Simplest test for verifying read/write of a snapshot. */
@TestMethodOrder(MethodOrderer.OrderAnnotation::class)
@DisableIfTestFails
class DuplicateWriteTest : Harness("undertest-junit5") {
@Test @Order(1)
fun noSelfie() {
ut_snapshot().deleteIfExists()
ut_snapshot().assertDoesNotExist()
}

@Test @Order(2)
fun cannot_write_multiple_things_to_one_snapshot() {
ut_mirror().linesFrom("fun shouldFail()").toFirst("}").uncomment()
ut_mirror().linesFrom("fun shouldPass()").toFirst("}").commentOut()
gradlew("underTest", "-Pselfie=write")!!.message shouldStartWith
"Snapshot was set to multiple values"
}

@Test @Order(3)
fun can_write_one_thing_multiple_times_to_one_snapshot() {
ut_mirror().linesFrom("fun shouldFail()").toFirst("}").commentOut()
ut_mirror().linesFrom("fun shouldPass()").toFirst("}").uncomment()
gradlew("underTest", "-Pselfie=write") shouldBe null
}

@Test @Order(4)
fun can_read_one_thing_multiple_times_from_one_snapshot() {
ut_mirror().linesFrom("fun shouldFail()").toFirst("}").commentOut()
ut_mirror().linesFrom("fun shouldPass()").toFirst("}").uncomment()
gradlew("underTest", "-Pselfie=read") shouldBe null
}

@Test @Order(5)
fun writeonce_mode() {
ut_mirror().linesFrom("fun shouldFail()").toFirst("}").commentOut()
ut_mirror().linesFrom("fun shouldPass()").toFirst("}").uncomment()
gradlew("underTest", "-Pselfie=writeonce")!!.message shouldStartWith
"Snapshot was set to the same value multiple times"
}

@Test @Order(6)
fun deleteSelfie() {
ut_snapshot().deleteIfExists()
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,20 @@
package com.diffplug.selfie.junit5

import io.kotest.matchers.shouldBe
import java.io.StringReader
import java.util.regex.Matcher
import java.util.regex.Pattern
import javax.xml.parsers.DocumentBuilderFactory
import javax.xml.xpath.XPathConstants
import javax.xml.xpath.XPathFactory
import okio.FileSystem
import okio.Path
import okio.Path.Companion.toPath
import org.gradle.internal.impldep.junit.framework.AssertionFailedError
import org.gradle.tooling.BuildException
import org.gradle.tooling.GradleConnector
import org.w3c.dom.NodeList
import org.xml.sax.InputSource

open class Harness(subproject: String) {
val subprojectFolder: Path
Expand Down Expand Up @@ -165,7 +174,7 @@ open class Harness(subproject: String) {
}
}
}
fun gradlew(task: String, vararg args: String): BuildException? {
fun gradlew(task: String, vararg args: String): AssertionFailedError? {
val runner =
GradleConnector.newConnector()
.forProjectDirectory(subprojectFolder.parent!!.toFile())
Expand All @@ -185,9 +194,56 @@ open class Harness(subproject: String) {
buildLauncher.run()
return null
} catch (e: BuildException) {
return e
return parseBuildException(task)
}
}

/**
* Parse build exception from gradle by looking into <code>build</code> directory to the matching
* test.
*
* Parses the exception message as well as stacktrace.
*/
private fun parseBuildException(task: String): AssertionFailedError {
val xmlFile =
subprojectFolder
.resolve("build")
.resolve("test-results")
.resolve(task)
.resolve("TEST-" + task.lowercase() + ".junit5.UT_" + javaClass.simpleName + ".xml")
val xml = FileSystem.SYSTEM.read(xmlFile) { readUtf8() }
val dbFactory = DocumentBuilderFactory.newInstance()
val dBuilder = dbFactory.newDocumentBuilder()
val xmlInput = InputSource(StringReader(xml))
val doc = dBuilder.parse(xmlInput)
val xpFactory = XPathFactory.newInstance()
val xPath = xpFactory.newXPath()

// <item type="T1" count="1">Value1</item>
val xpath = "/testsuite/testcase/failure"
val failures = xPath.evaluate(xpath, doc, XPathConstants.NODESET) as NodeList
val failure = failures.item(0)
val type = failure.attributes.getNamedItem("type").nodeValue
val message = failure.attributes.getNamedItem("message").nodeValue.replace("&#10;", "\n")
val lines = failure.textContent.replace(message, "").trim().split("\n")
val stacktrace: MutableList<StackTraceElement> = ArrayList()
val tracePattern =
Pattern.compile("\\s*at\\s+([\\w]+)//([\\w\\.]+)\\.([\\w]+)(\\(.*kt)?:(\\d+)\\)")
lines.forEach {
val traceMatcher: Matcher = tracePattern.matcher(it)
while (traceMatcher.find()) {
val module: String = traceMatcher.group(1)
val className: String = module + "//" + traceMatcher.group(2)
val methodName: String = traceMatcher.group(3)
val sourceFile: String = traceMatcher.group(4)
val lineNum: Int = traceMatcher.group(5).toInt()
stacktrace.add(StackTraceElement(className, methodName, sourceFile, lineNum))
}
}
val error = AssertionFailedError(message.replace("$type: ", "").trim())
error.stackTrace = stacktrace.toTypedArray()
return error
}
fun gradleWriteSS() {
gradlew("underTest", "-Pselfie=write")?.let {
throw AssertionError("Expected write snapshots to succeed, but it failed", it)
Expand All @@ -198,7 +254,7 @@ open class Harness(subproject: String) {
throw AssertionError("Expected read snapshots to succeed, but it failed", it)
}
}
fun gradleReadSSFail(): BuildException {
fun gradleReadSSFail(): AssertionFailedError {
val failure = gradlew("underTest", "-Pselfie=read")
if (failure == null) {
throw AssertionError("Expected read snapshots to fail, but it succeeded.")
Expand Down
30 changes: 30 additions & 0 deletions selfie-runner-junit5/src/test/kotlin/testpkg/RecordCallTest.kt
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
/*
* 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 testpkg

import com.diffplug.selfie.junit5.recordCall
import io.kotest.matchers.ints.shouldBeGreaterThan
import io.kotest.matchers.shouldBe
import org.junit.jupiter.api.Test

class RecordCallTest {
@Test
fun testRecordCall() {
val stack = recordCall()
stack.location.toString() shouldBe "testpkg/RecordCallTest.kt:26"
stack.restOfStack.size shouldBeGreaterThan 0
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
package undertest.junit5

import com.diffplug.selfie.expectSelfie
import org.junit.jupiter.api.Test

class UT_DuplicateWriteTest {
// @Test fun shouldFail() {
// expectSelfie("apples").toMatchDisk()
// expectSelfie("oranges").toMatchDisk()
// }
@Test fun shouldPass() {
expectSelfie("twins").toMatchDisk()
expectSelfie("twins").toMatchDisk()
}
}