Improve StringShrinker algorithm (#377)

The current string shrink algorithm generates candidates by dropping single characters from the end of the input. This doesn't produce the smallest case, since it doesn't drop characters from the start of the string. It is also linear on the size of the input, so it requires a potentially large number of tries to reach the result it does produce. This PR changes to algorithm to bisect the input string from both directions, producing a minimal output in log n tries. As an example of the current behavior, the test: `forAll { it: String -> !it.contains("#") }` produces output like this: ``` Attempting to shrink failed arg DPn CPQ7hBAY&LP;7MxPtN^Oy\$Fz;M#EC\M2yEcNn18+B*w,1x,L;6&k<}QQxU+!) e|Gr+ tri7jw{ Shrink #1: <empty string> pass Shrink #2: DPn CPQ7hBAY&LP;7MxPtN^Oy\$Fz;M#EC\M2yEcNn18+B*w,1x,L;6&k<}QQxU+!) e|Gr+ tri7 fail Shrink #3: DPn CPQ7hBAY&LP;7MxPtN^Oy\$Fz;M#EC\M2yEcNn18+B*w,1x,L;6&k<}QQxU+!) e|Gr+ fail Shrink #4: DPn CPQ7hBAY&LP;7MxPtN^Oy\$Fz;M#EC\M2yEcNn18+B*w,1x,L;6&k<}QQxU+!) fail Shrink #5: DPn CPQ7hBAY&LP;7MxPtN^Oy\$Fz;M#EC\M2yEcNn18+B*w,1x,L;6&k<}QQx fail Shrink #6: DPn CPQ7hBAY&LP;7MxPtN^Oy\$Fz;M#EC\M2yEcNn18+B*w,1x,L;6&k fail Shrink #7: DPn CPQ7hBAY&LP;7MxPtN^Oy\$Fz;M#EC\M2yEcNn18+B*w,1x, fail Shrink #8: DPn CPQ7hBAY&LP;7MxPtN^Oy\$Fz;M#EC\M2yEcNn18+B* fail Shrink #9: DPn CPQ7hBAY&LP;7MxPtN^Oy\$Fz;M#EC\M2yEcNn fail Shrink #10: DPn CPQ7hBAY&LP;7MxPtN^Oy\$Fz;M#EC\M2 fail Shrink #11: DPn CPQ7hBAY&LP;7MxPtN^Oy\$Fz;M# fail Shrink #12: DPn CPQ7hBAY&LP;7MxPtN^Oy\$ pass Shrink #13: DPn CPQ7hBAY&LP;7MxPtN^Oy\$F pass Shrink #14: DPn CPQ7hBAY&LP;7MxPtN^Oy\$Fz pass Shrink #15: DPn CPQ7hBAY&LP;7MxPtN^Oy\$Fz; pass Shrink #16: DPn CPQ7hBAY&LP;7MxPtN^Oy\$Fz;M pass Shrink #17: aaaaaDPn CPQ7hBAY&LP;7MxPtN^Oy\$ pass Shrink #18: aaaaDPn CPQ7hBAY&LP;7MxPtN^Oy\$F pass Shrink #19: aaaDPn CPQ7hBAY&LP;7MxPtN^Oy\$Fz pass Shrink #20: aaDPn CPQ7hBAY&LP;7MxPtN^Oy\$Fz; pass Shrink #21: aDPn CPQ7hBAY&LP;7MxPtN^Oy\$Fz;M pass Shrink result => DPn CPQ7hBAY&LP;7MxPtN^Oy\$Fz;M# ``` This is 21 attempts to produce a string 32x longer than optimal. The new algorithm produces output like this: ``` Attempting to shrink failed arg t ^j>t\o,x3?eb9#F'>g>vGQ-N}nkx Shrink #1: <empty string> pass Shrink #2: t ^j>t\o,x3?eb9 pass Shrink #3: #F'>g>vGQ-N}nkx fail Shrink #4: #F'>g>vG fail Shrink #5: #F'> fail Shrink #6: #F fail Shrink #7: # fail Shrink result => # ``` This is only 7 tries, and produces the correct output of `"#"`
kotest · Jul 15, 2018 · 477a293 · 477a293
1 parent 31786b5
commit 477a293
Show file tree

Hide file tree

Showing 4 changed files with 38 additions and 31 deletions.
diff --git a/kotlintest-assertions/src/main/kotlin/io/kotlintest/properties/shrinking/StringShrinker.kt b/kotlintest-assertions/src/main/kotlin/io/kotlintest/properties/shrinking/StringShrinker.kt
@@ -1,14 +1,14 @@
 package io.kotlintest.properties.shrinking
 
 object StringShrinker : Shrinker<String> {
-  override fun shrink(failure: String): List<String> {
-    // nothing more we can do with an empty string
-    return if (failure.isEmpty()) emptyList() else {
-      // 5 smaller strings and 5 strings with prefixes replaced
-      val smaller = (1..5).map { failure.dropLast(it) }.filter { it.isNotEmpty() }.reversed()
-      val prefixs = smaller.map { it.padStart(failure.length, 'a') }
+  override fun shrink(failure: String): List<String> = when (failure.length) {
+    0 -> emptyList()
+    1 -> listOf("", "a")
+    else -> {
+      val first = failure.take(failure.length / 2 + failure.length % 2)
+      val second = failure.takeLast(failure.length / 2)
       // always include empty string as the best shrink
-      listOf("") + smaller + prefixs
+     listOf("", first, first.padEnd(failure.length, 'a'), second, second.padStart(failure.length, 'a'))
     }
   }
-}
+}
diff --git a/kotlintest-assertions/src/main/kotlin/io/kotlintest/properties/shrinking/shrink.kt b/kotlintest-assertions/src/main/kotlin/io/kotlintest/properties/shrinking/shrink.kt
@@ -29,7 +29,6 @@ fun <T> shrink(t: T, shrinker: Shrinker<T>, test: (T) -> Unit): T {
       val next = candidates.firstOrNull {
         tested.add(it)
         count++
-        fun whitespace(str: String) = str.isBlank()
         try {
           test(it)
           sb.append("Shrink #$count: ${convertValueToString(it)} pass\n")

diff --git a/...est-tests-core/src/test/kotlin/com/sksamuel/kotlintest/properties/shrinking/ShrinkTest.kt b/...est-tests-core/src/test/kotlin/com/sksamuel/kotlintest/properties/shrinking/ShrinkTest.kt
@@ -3,6 +3,7 @@ package com.sksamuel.kotlintest.properties.shrinking
 import io.kotlintest.matchers.doubles.lt
 import io.kotlintest.matchers.lte
 import io.kotlintest.matchers.numerics.shouldBeLessThan
+import io.kotlintest.matchers.numerics.shouldBeLessThanOrEqual
 import io.kotlintest.matchers.string.shouldHaveLength
 import io.kotlintest.properties.Gen
 import io.kotlintest.properties.assertAll
@@ -33,7 +34,7 @@ class ShrinkTest : StringSpec({
       }
     }.message shouldBe "Property failed for\n" +
         "Arg 0: <empty string>\n" +
-        "Arg 1: aaaa (shrunk from \n" +
+        "Arg 1: aaaaa (shrunk from \n" +
         "abc\n" +
         "123\n" +
         ")\n" +
@@ -100,6 +101,6 @@ class ShrinkTest : StringSpec({
       assertAll(gen) { a ->
         a.padEnd(10, '*').shouldHaveLength(10)
       }
-    }.message shouldBe "Property failed for\nArg 0: aaaaaaaaaaa (shrunk from asjfiojoqiwehuoahsuidhqweqwe)\nafter 1 attempts\nCaused by: asjfiojoqiwehuoahsuidhqweqwe should have length 10"
+    }.message shouldBe "Property failed for\nArg 0: aaaaaaaaaaaaaa (shrunk from asjfiojoqiwehuoahsuidhqweqwe)\nafter 1 attempts\nCaused by: asjfiojoqiwehuoahsuidhqweqwe should have length 10"
   }
 })
diff --git a/...s-core/src/test/kotlin/com/sksamuel/kotlintest/properties/shrinking/StringShrinkerTest.kt b/...s-core/src/test/kotlin/com/sksamuel/kotlintest/properties/shrinking/StringShrinkerTest.kt
@@ -1,52 +1,59 @@
 package com.sksamuel.kotlintest.properties.shrinking
 
+import io.kotlintest.matchers.numerics.shouldBeLessThan
+import io.kotlintest.matchers.string.shouldEndWith
 import io.kotlintest.matchers.string.shouldHaveLength
+import io.kotlintest.matchers.string.shouldNotContain
 import io.kotlintest.matchers.string.shouldStartWith
 import io.kotlintest.properties.Gen
-import io.kotlintest.properties.shrinking.StringShrinker
 import io.kotlintest.properties.assertAll
+import io.kotlintest.properties.shrinking.StringShrinker
 import io.kotlintest.shouldBe
 import io.kotlintest.specs.StringSpec
 import shrink
 
 class StringShrinkerTest : StringSpec({
 
-  "StringShrinker shrinks should include empty string as the first candidate" {
+  "StringShrinker should include empty string as the first candidate" {
     assertAll { a: String ->
       if (a.isNotEmpty())
         StringShrinker.shrink(a)[0].shouldHaveLength(0)
     }
   }
 
-  "StringShrinker should include 5-1 sizes smaller as the 2nd to 6th candidates" {
+  "StringShrinker should bisect input as 2nd and 4th candidate" {
     assertAll { a: String ->
-      if (a.length > 5) {
+      if (a.length > 1) {
         val candidates = StringShrinker.shrink(a)
-        candidates[1].shouldHaveLength(a.length - 5)
-        candidates[2].shouldHaveLength(a.length - 4)
-        candidates[3].shouldHaveLength(a.length - 3)
-        candidates[4].shouldHaveLength(a.length - 2)
-        candidates[5].shouldHaveLength(a.length - 1)
+        candidates[1].shouldHaveLength(a.length / 2 + a.length % 2)
+        candidates[3].shouldHaveLength(a.length / 2)
       }
     }
   }
 
-  "StringShrinker should include 5-1 padded 'a's as the 7th to 11th candidates" {
+  "StringShrinker should include 2 padded 'a's as the 3rd to 5th candidates" {
     assertAll { a: String ->
-      if (a.length > 5) {
+      if (a.length > 1) {
         val candidates = StringShrinker.shrink(a)
-        candidates[6].shouldStartWith("aaaaa")
-        candidates[7].shouldStartWith("aaaa")
-        candidates[8].shouldStartWith("aaa")
-        candidates[9].shouldStartWith("aa")
-        candidates[10].shouldStartWith("a")
+        candidates[2].shouldEndWith("a".repeat(a.length / 2))
+        candidates[4].shouldStartWith("a".repeat(a.length / 2))
       }
     }
   }
 
   "StringShrinker should shrink to expected value" {
-    fun pad(str: String) = str.padEnd(12, '*')
-    shrink("97asd!@#ASD'''234)*safmasd", Gen.string(), { pad(it).shouldHaveLength(12) }) shouldBe "aaaaaaaaaaaaa"
-    shrink("97a", Gen.string(), { pad(it).shouldHaveLength(12) }) shouldBe "97a"
+    assertAll { it: String ->
+      val shrunk = shrink(it, Gen.string()) { it.shouldNotContain("#") }
+      if (it.contains("#")) {
+        shrunk shouldBe "#"
+      } else {
+        shrunk shouldBe it
+      }
+    }
+  }
+
+  "StringShrinker should prefer padded values" {
+    shrink("97asd!@#ASD'''234)*safmasd", Gen.string()) { it.length.shouldBeLessThan(13) } shouldBe "aaaaaaaaaaaaa"
+    shrink("97a", Gen.string()) { it.length.shouldBeLessThan(13) } shouldBe "97a"
   }
-})
+})