Skip to content
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

Implement toBe(LITERAL), not just toBe_TODO() #49

Merged
merged 6 commits into from
Dec 27, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 36 additions & 10 deletions selfie-lib/src/commonMain/kotlin/com/diffplug/selfie/SourceFile.kt
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,8 @@ class SourceFile(filename: String, content: String) {
* 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) {
inner class ToBeLiteral
internal constructor(private val slice: Slice, private val valueStart: Int) {
/**
* Modifies the parent [SourceFile] to set the value within the `toBe` call, and returns the net
* change in newline count.
Expand All @@ -48,7 +49,7 @@ class SourceFile(filename: String, content: String) {
throw Error(
"There is an error in " +
literalValue.format::class.simpleName +
", the following value isn't roundtripping.\n" +
", the following value isn't round tripping.\n" +
"Please this error and the data below at https://github.com/diffplug/selfie/issues/new\n" +
"```\n" +
"ORIGINAL\n" +
Expand All @@ -73,20 +74,45 @@ class SourceFile(filename: String, content: String) {
* `toBe_TODO()`.
*/
fun <T : Any> parseLiteral(literalFormat: LiteralFormat<T>): T {
// this won't work, because we need to find the `.toBe` and parens
TODO("return literalFormat.parse(slice.toString())")
return literalFormat.parse(
slice.subSequence(valueStart, slice.length - 1).toString(), language)
}
}
fun parseToBe_TODO(lineOneIndexed: Int): ToBeLiteral {
return parseToBeLike(".toBe_TODO(", lineOneIndexed)
}
fun parseToBe(lineOneIndexed: Int): ToBeLiteral {
return parseToBeLike(".toBe(", lineOneIndexed)
}
private fun parseToBeLike(prefix: String, lineOneIndexed: Int): ToBeLiteral {
val lineContent = contentSlice.unixLine(lineOneIndexed)
val idx = lineContent.indexOf(".toBe_TODO()")
val idx = lineContent.indexOf(prefix)
if (idx == -1) {
throw AssertionError(
"Expected to find `.toBe_TODO()` on line $lineOneIndexed, but there was only `${lineContent}`")
"Expected to find `$prefix)` 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")
var opened = 0
val startIndex = idx + prefix.length
var endIndex = -1
// TODO: do we need to detect paired parenthesis ( ( ) )?
Copy link
Member Author

@nedtwigg nedtwigg Dec 27, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cases we need to handle:

.toBe("""
(((
"
(((
""")
// or
.toBe(
  "(((\n" +
  "\"\n" +
   "(((")

Cases we do not need to handle

.toBe(("a" + "b") + "c")

We should add support for these in future PRs, there are many hard cases, we should do the easy stuff first (""" literals) and then solve problems as they come.

for (i in startIndex ..< lineContent.length) {
val ch = lineContent[i]
// TODO: handle () inside string literal
if (ch == '(') {
opened += 1
} else if (ch == ')') {
if (opened == 0) {
endIndex = i
break
} else {
opened -= 1
}
}
}
if (endIndex == -1) {
throw AssertionError(
"Expected to find `$prefix)` on line $lineOneIndexed, but there was only `${lineContent}`")
}
return ToBeLiteral(lineContent.subSequence(idx, endIndex + 1), prefix.length)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import java.nio.file.Files
import java.nio.file.Path
import java.util.stream.Collectors
import kotlin.io.path.name
import org.opentest4j.AssertionFailedError

/** Represents the line at which user code called into Selfie. */
data class CallLocation(val clazz: String, val method: String, val file: String?, val line: Int) :
Expand Down Expand Up @@ -100,9 +101,21 @@ internal class DiskWriteTracker : WriteTracker<String, Snapshot>() {
internal class InlineWriteTracker : WriteTracker<CallLocation, LiteralValue<*>>() {
fun record(call: CallStack, literalValue: LiteralValue<*>, layout: SnapshotFileLayout) {
recordInternal(call.location, literalValue, call, layout)
// assert that the value passed at runtime matches the value we parse at compile time
// because if that assert fails, we've got no business modifying test code
val file =
layout.sourcecodeForCall(call.location)
?: throw Error("Unable to find source file for ${call.location.ideLink(layout)}")
if (literalValue.expected != null) {
throw UnsupportedOperationException(
"`.toBe() didn't match! Change to `toBe_TODO()` to record a new value until https://github.com/diffplug/selfie/pull/49 is merged.")
// if expected == null, it's a `toBe_TODO()`, so there's nothing to check
val content = SourceFile(file.name, Files.readString(file))
val parsedValue = content.parseToBe(call.location.line).parseLiteral(literalValue.format)
if (parsedValue != literalValue.expected) {
throw AssertionFailedError(
"There is likely a bug in Selfie's literal parsing.",
literalValue.expected,
parsedValue)
}
}
}
fun hasWrites(): Boolean = writes.isNotEmpty()
Expand Down Expand Up @@ -141,18 +154,7 @@ internal class InlineWriteTracker : WriteTracker<CallLocation, LiteralValue<*>>(
if (write.literal.expected == null) {
content.parseToBe_TODO(line)
} else {
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)
}
}
content.parseToBe(line)
}
deltaLineNumbers += toBe.setLiteralAndGetNewlineDelta(write.literal)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,9 +33,20 @@ class InlineIntTest : Harness("undertest-junit5") {
}

@Test @Order(2)
fun toBe_write() {
fun toBe_writeTODO() {
ut_mirror().lineWith("expectSelfie").setContent(" expectSelfie(1234).toBe_TODO()")
gradleReadSSFail()
gradleWriteSS()
ut_mirror().lineWith("expectSelfie").content() shouldBe " expectSelfie(1234).toBe(1234)"
gradleReadSS()
}

@Test @Order(3)
fun toBe_writeLiteral() {
ut_mirror().lineWith("expectSelfie").setContent(" expectSelfie(7777).toBe(1234)")
gradleReadSSFail()
gradleWriteSS()
ut_mirror().lineWith("expectSelfie").content() shouldBe " expectSelfie(7777).toBe(7777)"
gradleReadSS()
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,6 @@ import kotlin.test.Test

class UT_InlineIntTest {
@Test fun singleInt() {
expectSelfie(1234).toBe(1234)
expectSelfie(7777).toBe(7777)
}
}