Skip to content

Commit

Permalink
Better error messages (#477)
Browse files Browse the repository at this point in the history
  • Loading branch information
nedtwigg authored Oct 1, 2024
2 parents 91a7865 + 97de81d commit 5c0c779
Show file tree
Hide file tree
Showing 6 changed files with 213 additions and 13 deletions.
16 changes: 16 additions & 0 deletions jvm/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,22 @@ 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]
### Added
- Snapshot mismatch error messages now show a diff of the first mismatched line. ([#477](https://github.com/diffplug/selfie/pull/477))
- before
```
Snapshot mismatch
- update this snapshot by adding `_TODO` to the function name
- update all snapshots in this file by adding `//selfieonce` or `//SELFIEWRITE`
```
- after
```
Snapshot mismatch at L7:C9
-Testing 123
+Testing ABC
‣ update this snapshot by adding `_TODO` to the function name
‣ update all snapshots in this file by adding `//selfieonce` or `//SELFIEWRITE`
```
## [2.3.0] - 2024-07-11
### Added
Expand Down
22 changes: 19 additions & 3 deletions jvm/selfie-lib/src/commonMain/kotlin/com/diffplug/selfie/Mode.kt
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ package com.diffplug.selfie

import com.diffplug.selfie.guts.CallStack
import com.diffplug.selfie.guts.CommentTracker
import com.diffplug.selfie.guts.SnapshotNotEqualErrorMsg
import com.diffplug.selfie.guts.SnapshotSystem
import com.diffplug.selfie.guts.TypedPath

Expand All @@ -42,13 +43,28 @@ enum class Mode {
internal fun msgSnapshotNotFound() = msg("Snapshot not found")
internal fun msgSnapshotNotFoundNoSuchFile(file: TypedPath) =
msg("Snapshot not found: no such file $file")
internal fun msgSnapshotMismatch() = msg("Snapshot mismatch")
internal fun msgSnapshotMismatch(expected: String, actual: String) =
msg(SnapshotNotEqualErrorMsg.forUnequalStrings(expected, actual))
internal fun msgSnapshotMismatchBinary(expected: ByteArray, actual: ByteArray) =
msgSnapshotMismatch(expected.toQuotedPrintable(), actual.toQuotedPrintable())
private fun ByteArray.toQuotedPrintable(): String {
val sb = StringBuilder()
for (byte in this) {
val b = byte.toInt() and 0xFF // Make sure byte is treated as unsigned
if (b in 33..126 && b != 61) { // Printable ASCII, except '='
sb.append(b.toChar())
} else {
sb.append("=").append(b.toString(16).uppercase().padStart(2, '0')) // Convert to hex and pad
}
}
return sb.toString()
}
private fun msg(headline: String) =
when (this) {
interactive ->
"$headline\n" +
"- update this snapshot by adding `_TODO` to the function name\n" +
"- update all snapshots in this file by adding `//selfieonce` or `//SELFIEWRITE`"
" update this snapshot by adding `_TODO` to the function name\n" +
" update all snapshots in this file by adding `//selfieonce` or `//SELFIEWRITE`"
readonly -> headline
overwrite -> "$headline\n(didn't expect this to ever happen in overwrite mode)"
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,9 @@ class BinarySelfie(actual: Snapshot, disk: DiskStorage, private val onlyFacet: S
return actualBytes
} else {
throw Selfie.system.fs.assertFailed(
Selfie.system.mode.msgSnapshotMismatch(), expected, actualBytes)
Selfie.system.mode.msgSnapshotMismatchBinary(expected, actualBytes),
expected,
actualBytes)
}
}
}
Expand Down Expand Up @@ -241,7 +243,9 @@ private fun <T : Any> toBeDidntMatch(expected: T?, actual: T, format: LiteralFor
throw Selfie.system.fs.assertFailed("Can't call `toBe_TODO` in ${Mode.readonly} mode!")
} else {
throw Selfie.system.fs.assertFailed(
Selfie.system.mode.msgSnapshotMismatch(), expected, actual)
Selfie.system.mode.msgSnapshotMismatch(expected.toString(), actual.toString()),
expected,
actual)
}
}
}
Expand All @@ -263,10 +267,12 @@ private fun assertEqual(expected: Snapshot?, actual: Snapshot, storage: Snapshot
.filter { expected.subjectOrFacetMaybe(it) != actual.subjectOrFacetMaybe(it) }
.toList()
.sorted()
val expectedFacets = serializeOnlyFacets(expected, mismatchedKeys)
val actualFacets = serializeOnlyFacets(actual, mismatchedKeys)
throw storage.fs.assertFailed(
storage.mode.msgSnapshotMismatch(),
serializeOnlyFacets(expected, mismatchedKeys),
serializeOnlyFacets(actual, mismatchedKeys))
storage.mode.msgSnapshotMismatch(expectedFacets, actualFacets),
expectedFacets,
actualFacets)
}
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
/*
* 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.guts

object SnapshotNotEqualErrorMsg {
const val REASONABLE_LINE_LENGTH = 120
fun forUnequalStrings(expected: String, actual: String): String {
var lineNumber = 1
var columnNumber = 1
var index = 0

while (index < expected.length && index < actual.length) {
val expectedChar = expected[index]
val actualChar = actual[index]
if (expectedChar != actualChar) {
val endOfLineExpected =
expected.indexOf('\n', index).let { if (it == -1) expected.length else it }
val endOfLineActual =
actual.indexOf('\n', index).let { if (it == -1) actual.length else it }
val expectedLine = expected.substring(index - columnNumber + 1, endOfLineExpected)
val actualLine = actual.substring(index - columnNumber + 1, endOfLineActual)
return "Snapshot mismatch at L$lineNumber:C$columnNumber\n-$expectedLine\n+$actualLine"
}
if (expectedChar == '\n') {
lineNumber++
columnNumber = 1
} else {
columnNumber++
}
index++
}
val endOfLineExpected =
expected.indexOf('\n', index).let { if (it == -1) expected.length else it }
val endOfLineActual = actual.indexOf('\n', index).let { if (it == -1) actual.length else it }

if (endOfLineActual == endOfLineExpected) {
// it ended at a line break
val longer = if (actual.length > expected.length) actual else expected
val added = if (actual.length > expected.length) "+" else "-"
val endIdx =
longer.indexOf('\n', endOfLineActual + 1).let { if (it == -1) longer.length else it }
val line = longer.substring(endOfLineActual + 1, endIdx)
return "Snapshot mismatch at L${lineNumber+1}:C1 - line(s) ${if (added == "+") "added" else "removed"}\n${added}$line"
} else {
val expectedLine = expected.substring(index - columnNumber + 1, endOfLineExpected)
val actualLine = actual.substring(index - columnNumber + 1, endOfLineActual)
return "Snapshot mismatch at L$lineNumber:C$columnNumber\n-$expectedLine\n+$actualLine"
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,97 @@
/*
* 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.guts

import io.kotest.matchers.shouldBe
import kotlin.test.Test

class SnapshotNotEqualErrorMsgTest {
@Test
fun errorLine1() {
SnapshotNotEqualErrorMsg.forUnequalStrings("Testing 123", "Testing ABC") shouldBe
"""Snapshot mismatch at L1:C9
-Testing 123
+Testing ABC"""

SnapshotNotEqualErrorMsg.forUnequalStrings("123 Testing", "ABC Testing") shouldBe
"""Snapshot mismatch at L1:C1
-123 Testing
+ABC Testing"""
}

@Test
fun errorLine2() {
SnapshotNotEqualErrorMsg.forUnequalStrings("Line\nTesting 123", "Line\nTesting ABC") shouldBe
"""Snapshot mismatch at L2:C9
-Testing 123
+Testing ABC"""

SnapshotNotEqualErrorMsg.forUnequalStrings("Line\n123 Testing", "Line\nABC Testing") shouldBe
"""Snapshot mismatch at L2:C1
-123 Testing
+ABC Testing"""
}

@Test
fun extraLine1() {
SnapshotNotEqualErrorMsg.forUnequalStrings("123", "123ABC") shouldBe
"""Snapshot mismatch at L1:C4
-123
+123ABC"""
SnapshotNotEqualErrorMsg.forUnequalStrings("123ABC", "123") shouldBe
"""Snapshot mismatch at L1:C4
-123ABC
+123"""
}

@Test
fun extraLine2() {
SnapshotNotEqualErrorMsg.forUnequalStrings("line\n123", "line\n123ABC") shouldBe
"""Snapshot mismatch at L2:C4
-123
+123ABC"""
SnapshotNotEqualErrorMsg.forUnequalStrings("line\n123ABC", "line\n123") shouldBe
"""Snapshot mismatch at L2:C4
-123ABC
+123"""
}

@Test
fun extraLine() {
SnapshotNotEqualErrorMsg.forUnequalStrings("line", "line\nnext") shouldBe
"""Snapshot mismatch at L2:C1 - line(s) added
+next"""
SnapshotNotEqualErrorMsg.forUnequalStrings("line\nnext", "line") shouldBe
"""Snapshot mismatch at L2:C1 - line(s) removed
-next"""
}

@Test
fun extraNewline() {
SnapshotNotEqualErrorMsg.forUnequalStrings("line", "line\n") shouldBe
"""Snapshot mismatch at L2:C1 - line(s) added
+"""
SnapshotNotEqualErrorMsg.forUnequalStrings("line\n", "line") shouldBe
"""Snapshot mismatch at L2:C1 - line(s) removed
-"""
SnapshotNotEqualErrorMsg.forUnequalStrings("", "\n") shouldBe
"""Snapshot mismatch at L2:C1 - line(s) added
+"""
SnapshotNotEqualErrorMsg.forUnequalStrings("\n", "") shouldBe
"""Snapshot mismatch at L2:C1 - line(s) removed
-"""
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -37,9 +37,11 @@ class InteractiveTest : HarnessJUnit() {
fun inlineMismatch() {
ut_mirrorKt().lineWith("expectSelfie(").setContent(" expectSelfie(5).toBe(10)")
gradleInteractiveFail().message shouldBe
"Snapshot mismatch\n" +
"- update this snapshot by adding `_TODO` to the function name\n" +
"- update all snapshots in this file by adding `//selfieonce` or `//SELFIEWRITE`"
"""Snapshot mismatch at L1:C1
-10
+5
‣ update this snapshot by adding `_TODO` to the function name
‣ update all snapshots in this file by adding `//selfieonce` or `//SELFIEWRITE`"""
}

@Test @Order(3)
Expand Down Expand Up @@ -72,8 +74,8 @@ class InteractiveTest : HarnessJUnit() {
ut_mirrorKt().lineWith("expectSelfie(").setContent(" expectSelfie(\"5\").toMatchDisk()")
gradleInteractiveFail().message shouldBe
"Snapshot not found\n" +
"- update this snapshot by adding `_TODO` to the function name\n" +
"- update all snapshots in this file by adding `//selfieonce` or `//SELFIEWRITE`"
" update this snapshot by adding `_TODO` to the function name\n" +
" update all snapshots in this file by adding `//selfieonce` or `//SELFIEWRITE`"
}

@Test @Order(7)
Expand Down

0 comments on commit 5c0c779

Please sign in to comment.