From 9aa0448cb415c76a5cdd4b151b1c92c6d50a3af8 Mon Sep 17 00:00:00 2001
From: Tobias Pfeiffer <pragtob@gmail.com>
Date: Mon, 15 Jan 2018 23:18:37 +0100
Subject: [PATCH] Tagged Save - implementation to avoid duplication

Not 100% happy with this implementation. Seems overcomplicated.
I changed the requirements a bit so that we don't create tag-1
but just tag and tag-2. Otherwise it'd be even more involved.

I considered making it more structure that a Tag is a structure
with `%Tag{name: "foo", count: 1}` or something like this.
But that also seemed like a bit of overkill.

So yeah, if you have anything simpler let me know. Otherwise
let me know if you think this is good enough :)
---
 lib/benchee/formatters/tagged_save.ex        | 33 ++++++++++-
 test/benchee/formatters/tagged_save_test.exs | 62 +++++++++++++++++---
 2 files changed, 86 insertions(+), 9 deletions(-)

diff --git a/lib/benchee/formatters/tagged_save.ex b/lib/benchee/formatters/tagged_save.ex
index a65c92a3..c35e92bb 100644
--- a/lib/benchee/formatters/tagged_save.ex
+++ b/lib/benchee/formatters/tagged_save.ex
@@ -17,13 +17,42 @@ defmodule Benchee.Formatters.TaggedSave do
   @spec format(Suite.t) :: {binary, String.t}
   def format(suite = %Suite{configuration: config, scenarios: scenarios}) do
     formatter_config = config.formatter_options.tagged_save
-    tagged_scenarios = tag_scenarios(scenarios, formatter_config)
+    tag = determine_tag(scenarios, formatter_config)
+    tagged_scenarios = tag_scenarios(scenarios, tag)
     tagged_suite = %Suite{suite | scenarios: tagged_scenarios}
 
     {:erlang.term_to_binary(tagged_suite), formatter_config.path}
   end
 
-  defp tag_scenarios(scenarios, %{tag: tag}) do
+  defp determine_tag(scenarios, %{tag: desired_tag}) do
+    scenarios
+    |> Enum.map(fn scenario -> scenario.tag end)
+    |> Enum.uniq()
+    |> Enum.filter(fn tag ->
+         tag != nil && tag =~ ~r/#{Regex.escape(desired_tag)}/
+       end)
+    |> choose_tag(desired_tag)
+  end
+
+  defp choose_tag([], desired_tag), do: desired_tag
+
+  defp choose_tag(tags, desired_tag) do
+    if Enum.all?(tags, fn tag -> tag == desired_tag end) do
+      "#{desired_tag}-2"
+    else
+      max = get_maximum_tag_increaser(tags, desired_tag)
+      "#{desired_tag}-#{max + 1}"
+    end
+  end
+
+  defp get_maximum_tag_increaser(tags, desired_tag) do
+    tags
+    |> Enum.map(fn tag -> String.replace(tag, desired_tag <> "-", "") end)
+    |> Enum.map(&String.to_integer/1)
+    |> Enum.max
+  end
+
+  defp tag_scenarios(scenarios, tag) do
     Enum.map scenarios, fn(scenario) ->
       scenario
       |> tagged_scenario(tag)
diff --git a/test/benchee/formatters/tagged_save_test.exs b/test/benchee/formatters/tagged_save_test.exs
index c243cfb1..1d8a0455 100644
--- a/test/benchee/formatters/tagged_save_test.exs
+++ b/test/benchee/formatters/tagged_save_test.exs
@@ -72,20 +72,68 @@ defmodule Benchee.Formatters.TaggedSaveTest do
       end
     end
 
-    test "doesn't tag scenarios that already had a tag" do
+    test "doesn't tag scenarios that already have a tag" do
       tagged_scenario = %Scenario{tag: "some-tag"}
       suite = %Suite{@suite | scenarios: [tagged_scenario | @suite.scenarios]}
 
+      tags =
+        suite
+        |> scenarios_from_formatted
+        |> sorted_tags
+
+      assert tags == [@benchee_tag, "some-tag"]
+    end
+
+    test "when duplicating tags for the same job the second gets -2" do
+      tagged_scenario = %Scenario{job_name: "foo", tag: @benchee_tag}
+      scenario = %Scenario{job_name: "foo"}
+      suite = %Suite{@suite | scenarios: [scenario, tagged_scenario]}
+
+      scenarios = scenarios_from_formatted(suite)
+      tags = sorted_tags(scenarios)
+      names = sorted_names(scenarios)
+
+      assert tags == [@benchee_tag, @benchee_tag <> "-2"]
+      assert names == ["foo (#{@benchee_tag})", "foo (#{@benchee_tag}-2)"]
+    end
+
+    test "when there's already a -2 and -3 tag we end up with -4" do
+      scenario_1 = %Scenario{job_name: "foo", tag: "#{@benchee_tag}-2"}
+      scenario_2 = %Scenario{job_name: "foo", tag: "#{@benchee_tag}-3"}
+      new_scenario = %Scenario{job_name: "foo"}
+
+      suite = %Suite{@suite | scenarios: [scenario_1, new_scenario, scenario_2]}
+
+      scenarios = scenarios_from_formatted(suite)
+      tags = sorted_tags(scenarios)
+      names = sorted_names(scenarios)
+
+      assert tags ==
+        [@benchee_tag <> "-2", @benchee_tag <> "-3", @benchee_tag <> "-4"]
+      assert names ==
+        ["foo (#{@benchee_tag}-2)",
+         "foo (#{@benchee_tag}-3)",
+         "foo (#{@benchee_tag}-4)"]
+    end
+
+    defp scenarios_from_formatted(suite) do
       {binary, _path} = format(suite)
       loaded_suite = :erlang.binary_to_term(binary)
-      loaded_scenarios = loaded_suite.scenarios
+      loaded_suite.scenarios
+    end
 
-      tags = loaded_scenarios
-             |> Enum.map(fn(scenario) -> scenario.tag end)
-             |> Enum.uniq
-             |> Enum.sort
+    defp sorted_tags(scenarios) do
+      scenarios
+      |> Enum.map(fn(scenario) -> scenario.tag end)
+      |> Enum.uniq
+      |> Enum.sort
+    end
 
-      assert tags == [@benchee_tag, "some-tag"]
+    defp sorted_names(scenarios) do
+      scenarios
+      |> Enum.map(fn(scenario) -> scenario.name end)
+      |> Enum.uniq
+      |> Enum.sort
     end
   end