Optimize ExUnit runner by short-circuiting logic and merging list passes

Author: josevalim

lib/ex_unit/lib/ex_unit/filters.ex

@@ -1,9 +1,8 @@
 defmodule ExUnit.Filters do
-  alias ExUnit.FailuresManifest
-
   @moduledoc """
   Conveniences for parsing and evaluating filters.
   """
+  alias ExUnit.FailuresManifest
 
   @type t :: list({atom, Regex.t() | String.Chars.t()} | atom)
   @type location :: {:location, {String.t(), pos_integer | [pos_integer, ...]}}
@@ -195,26 +194,32 @@ defmodule ExUnit.Filters do
   @spec eval(t, t, map, [ExUnit.Test.t()]) ::
           :ok | {:excluded, String.t()} | {:skipped, String.t()}
   def eval(include, exclude, tags, collection) when is_map(tags) do
-    excluded = Enum.find_value(exclude, &has_tag(&1, tags, collection))
-    excluded? = excluded != nil
-    included? = Enum.any?(include, &has_tag(&1, tags, collection))
+    cond do
+      Enum.any?(include, &has_tag(&1, tags, collection)) ->
+        maybe_skipped(include, tags, collection)
+
+      excluded = Enum.find_value(exclude, &has_tag(&1, tags, collection)) ->
+        {:excluded, "due to #{excluded} filter"}
 
-    if included? or not excluded? do
-      skip_tag = %{skip: Map.get(tags, :skip, true)}
-      skip_included_explicitly? = Enum.any?(include, &has_tag(&1, skip_tag, collection))
+      true ->
+        maybe_skipped(include, tags, collection)
+    end
+  end
 
-      case Map.fetch(tags, :skip) do
-        {:ok, msg} when is_binary(msg) and not skip_included_explicitly? ->
-          {:skipped, msg}
+  defp maybe_skipped(include, tags, collection) do
+    case tags do
+      %{skip: skip} when is_binary(skip) or skip == true ->
+        skip_tags = %{skip: skip}
+        skip_included_explicitly? = Enum.any?(include, &has_tag(&1, skip_tags, collection))
 
-        {:ok, true} when not skip_included_explicitly? ->
-          {:skipped, "due to skip tag"}
+        cond do
+          skip_included_explicitly? -> :ok
+          is_binary(skip) -> {:skipped, skip}
+          skip -> {:skipped, "due to skip tag"}
+        end
 
-        _ ->
-          :ok
-      end
-    else
-      {:excluded, "due to #{excluded} filter"}
+      _ ->
+        :ok
     end
   end
 

lib/ex_unit/lib/ex_unit/runner.ex

@@ -216,8 +216,7 @@ defmodule ExUnit.Runner do
     EM.module_started(config.manager, test_module)
 
     # Prepare tests, selecting which ones should be run or skipped
-    tests = prepare_tests(config, test_module.tests)
-    {excluded_and_skipped_tests, to_run_tests} = Enum.split_with(tests, & &1.state)
+    {to_run_tests, excluded_and_skipped_tests} = prepare_tests(config, test_module.tests)
 
     for excluded_or_skipped_test <- excluded_and_skipped_tests do
       EM.test_started(config.manager, excluded_or_skipped_test)
@@ -257,14 +256,18 @@ defmodule ExUnit.Runner do
     exclude = config.exclude
     test_ids = config.only_test_ids
 
-    for test <- tests, include_test?(test_ids, test) do
-      tags = Map.merge(test.tags, %{test: test.name, module: test.module})
+    {to_run, to_skip} =
+      for test <- tests, include_test?(test_ids, test), reduce: {[], []} do
+        {to_run, to_skip} ->
+          tags = Map.merge(test.tags, %{test: test.name, module: test.module})
 
-      case ExUnit.Filters.eval(include, exclude, tags, tests) do
-        :ok -> %{test | tags: tags}
-        excluded_or_skipped -> %{test | state: excluded_or_skipped}
+          case ExUnit.Filters.eval(include, exclude, tags, tests) do
+            :ok -> {[%{test | tags: tags} | to_run], to_skip}
+            excluded_or_skipped -> {to_run, [%{test | state: excluded_or_skipped} | to_skip]}
+          end
       end
-    end
+
+    {Enum.reverse(to_run), Enum.reverse(to_skip)}
   end
 
   defp include_test?(test_ids, test) do