improves shrinking performance by streaming in order and taking the f…

…isrt instead of sorting them

NOTES.md

@@ -54,6 +54,9 @@ With no history it needs to use the same seed as ExUnit, which happens automatic
 - [x] Catch errors raised by `body_fn` so we can capture PRNG history and enter shrinking cycle.
 
 - [ ] Format raised error message to include generated values and shrinking statistics.
+Use Telemetry for metrics and stats?
+
+- [x] Include `value` field in `PropertyError`.
 
 - [ ] How do we get a meaningful stacktrace? Does that even matter?
 
@@ -63,7 +66,7 @@ With no history it needs to use the same seed as ExUnit, which happens automatic
 
 - [x] Change `History.shrink_length/1` to remove one item at a time and fix the resulting error and occasional timeout.
 
-- [x] Keep track of seen histories to avoid trying them again.
+- [x] Keep track of seen histories to avoid trying them again. (No longer needed after refactoring into `Shrinker` module.)
 
 - [x] Try a new implementation of shrinking. Create multiple histories from a given history. Test all of them against test_fn. 
 Keep best (shortlex smallest) that still fails the test and re-start the shrinking process with that one as the input.

lib/decorum/empty_history_error.ex

@@ -0,0 +1,4 @@
+defmodule Decorum.EmptyHistoryError do
+  @moduledoc false
+  defexception [:message]
+end

lib/decorum/history.ex

@@ -40,14 +40,18 @@ defmodule Decorum.History do
   end
 
   @spec replace_chunk(t(), Chunk.t(), t()) :: t()
-  def replace_chunk(history, chunk, new_chunk) do
-    {pre, _, post} = get_chunked_parts(history, chunk)
-
-    Enum.concat([
-      pre,
-      new_chunk,
-      post
-    ])
+  def replace_chunk(history, chunk, new_values) do
+    {pre, old_values, post} = get_chunked_parts(history, chunk)
+
+    if old_values != new_values do
+      Enum.concat([
+        pre,
+        new_values,
+        post
+      ])
+    else
+      history
+    end
   end
 
   @spec replace_chunk_with_zero(t(), Chunk.t()) :: t()

lib/decorum/prng.ex

@@ -14,11 +14,6 @@ defmodule Decorum.Prng do
   @type t :: prng
   @type prng :: __MODULE__.Random.t() | __MODULE__.Hardcoded.t()
 
-  defmodule EmptyHistoryError do
-    @moduledoc false
-    defexception [:message]
-  end
-
   defmodule Random do
     @moduledoc false
     @type t :: %__MODULE__{state: :rand.state(), history: Decorum.History.t()}
@@ -61,7 +56,7 @@ defmodule Decorum.Prng do
 
     @spec next!(prng :: t()) :: {non_neg_integer(), t()}
     def next!(%__MODULE__{unusedHistory: []} = _prng) do
-      raise EmptyHistoryError, "PRNG history is empty"
+      raise Decorum.EmptyHistoryError, "PRNG history is empty"
     end
 
     def next!(%__MODULE__{history: history, unusedHistory: [value | rest]} = prng) do

lib/decorum/property_error.ex

@@ -1,4 +1,4 @@
 defmodule Decorum.PropertyError do
   @moduledoc false
-  defexception [:message]
+  defexception [:message, :value]
 end

lib/decorum/shrinker.ex

@@ -20,6 +20,8 @@ defmodule Decorum.Shrinker do
 
   @type check_function(value) :: (value -> check_result())
 
+  @type check_history_result(value) :: :fail | {:pass, History.t(), value, String.t()}
+
   @doc """
   Takes a PRNG history and shrinks it to smaller values
   using a number of chunk manipulation strategies.
@@ -35,48 +37,47 @@ defmodule Decorum.Shrinker do
         ) :: :ok
   def shrink(_, _, value, [], message) do
     # TODO: figure out why this function gets called with an empty history.
-    raise Decorum.PropertyError, Enum.join([inspect(value), message], "\n\n")
+    raise Decorum.EmptyHistoryError,
+      message: Enum.join([inspect(value), message], "\n\n"),
+      value: value
   end
 
   def shrink(check_function, generator, value, history, message) do
-    history_length = Enum.count(history)
+    case shrink_by_chunks(check_function, generator, history) do
+      {:pass, history, value, message} ->
+        shrink(check_function, generator, value, history, message)
+
+      :fail ->
+        {new_value, new_history, new_message} =
+          binary_search(check_function, generator, value, message, history)
 
-    valid_histories =
-      1..min(history_length, 4)
-      |> Enum.flat_map(fn chunk_length ->
-        Chunk.chunks(history_length, chunk_length)
-      end)
-      |> Enum.flat_map(fn chunk ->
-        [
-          History.delete_chunk(history, chunk),
-          History.replace_chunk_with_zero(history, chunk)
-        ]
-      end)
-      |> Enum.reduce([], fn hist, valid_histories ->
-        if History.compare(hist, history) == :lt do
-          case check_history(hist, generator, check_function) do
-            :fail -> valid_histories
-            {:pass, value, message} -> [{hist, value, message} | valid_histories]
-          end
+        if new_history != history do
+          shrink(check_function, generator, new_value, new_history, new_message)
         else
-          valid_histories
+          raise Decorum.PropertyError,
+            message: Enum.join([inspect(value), message], "\n\n"),
+            value: value
         end
-      end)
-      |> Enum.sort_by(fn {hist, _, _} -> hist end, History)
-
-    if Enum.any?(valid_histories) do
-      {history, value, message} = List.first(valid_histories)
-      shrink(check_function, generator, value, history, message)
-    else
-      {new_value, new_history, new_message} =
-        binary_search(check_function, generator, value, message, history)
-
-      if new_history != history do
-        shrink(check_function, generator, new_value, new_history, new_message)
-      end
     end
+  end
+
+  @spec shrink_by_chunks(check_function(value), Decorum.generator_fun(value), History.t()) ::
+          check_history_result(value)
+  defp shrink_by_chunks(check_function, generator, history) do
+    history_length = Enum.count(history)
 
-    raise Decorum.PropertyError, Enum.join([inspect(value), message], "\n\n")
+    chunks = min(history_length, 4)..1
+    |> Enum.flat_map(fn chunk_length ->
+      Chunk.chunks(history_length, chunk_length)
+    end)
+
+    Stream.concat([
+      Stream.map(chunks, &(History.delete_chunk(history, &1))),
+      Stream.map(chunks, &(History.replace_chunk_with_zero(history, &1)))
+    ])
+    |> Enum.filter(&(&1 != history))
+    |> Enum.map(&check_history(&1, generator, check_function))
+    |> Enum.find(:fail, &(&1 != :fail))
   end
 
   defp binary_search(check_function, generator, value, message, [first_value | post_history]) do
@@ -137,7 +138,7 @@ defmodule Decorum.Shrinker do
           high
         )
 
-      {:pass, value, message} ->
+      {:pass, _, value, message} ->
         binary_search(
           check_function,
           generator,
@@ -152,7 +153,7 @@ defmodule Decorum.Shrinker do
   end
 
   @spec check_history(History.t(), Decorum.generator_fun(value), check_function(value)) ::
-          :fail | {:pass, value, String.t()}
+          check_history_result(value)
   defp check_history([], _, _), do: :fail
 
   defp check_history(history, generator, check_function) do
@@ -163,10 +164,10 @@ defmodule Decorum.Shrinker do
 
       case check_function.(value) do
         :ok -> :fail
-        {:error, message} -> {:pass, value, message}
+        {:error, message} -> {:pass, history, value, message}
       end
     rescue
-      Prng.EmptyHistoryError -> :fail
+      Decorum.EmptyHistoryError -> :fail
     end
   end
 end

test/prng_test.exs

@@ -33,7 +33,7 @@ defmodule PrngTest do
 
     prng = Prng.hardcoded(history)
     {1, prng} = Prng.next!(prng)
-    assert_raise Decorum.Prng.EmptyHistoryError, fn -> Prng.next!(prng) end
+    assert_raise Decorum.EmptyHistoryError, fn -> Prng.next!(prng) end
   end
 
   test "hardcoded Prng keeps track of the history it uses" do

test/shrinking_challenge_test.exs

@@ -7,13 +7,10 @@ defmodule ShrinkingChallengeTest do
   Implementing these tests to exercise the Decorum shrinker.
   """
 
-  @tag :skip
   test "bound5" do
-    # runs for over 5 seconds with mix test --seed 558117
-    # takes over 38 seconds with seed 631266
-    # fails with seed 299458 because empty lists are not together.
-    assert_raise Decorum.PropertyError,
-                 ~r/\[\], \[\]/,
+    # runs for about 4 seconds with mix test --seed 558117
+    # takes about 21 seconds with seed 631266
+    %Decorum.PropertyError{value: value} = assert_raise Decorum.PropertyError,
                  fn ->
                    Decorum.integer(-32768..32767)
                    |> Decorum.list_of()
@@ -28,6 +25,8 @@ defmodule ShrinkingChallengeTest do
                               5 * 256
                    end)
                  end
+    #assert that 3 of the lists are empty
+    assert Enum.count(value, &(&1 == [])) == 3
   end
 
   defp normalize(n) do