Add support for mix test --failed

This uses the ExUnit manifest to filter to only tests that failed
the last time they ran. As an optimization, we filter out files
that have no failures so we load the minimum set of files necessary.

Signed-off-by: José Valim <jose.valim@plataformatec.com.br>

lib/ex_unit/lib/ex_unit/filters.ex

@@ -1,4 +1,6 @@
 defmodule ExUnit.Filters do
+  alias ExUnit.Manifest
+
   @moduledoc """
   Conveniences for parsing and evaluating filters.
   """
@@ -88,6 +90,20 @@ defmodule ExUnit.Filters do
     end)
   end
 
+  @doc """
+  Returns a tuple containing useful information about test failures from the
+  manifest. The tuple contains:
+
+    - A set of files that contain tests that failed the last time they ran.
+      The paths are absolute paths.
+    - A set of test ids that failed the last time they ran
+  """
+  @spec failure_info(Path.t()) :: {MapSet.t(Path.t()), MapSet.t(Manifest.test_id())}
+  def failure_info(manifest_file) do
+    manifest = Manifest.read(manifest_file)
+    {Manifest.files_with_failures(manifest), Manifest.failed_test_ids(manifest)}
+  end
+
   @doc """
   Evaluates the `include` and `exclude` filters against the given `tags` to
   determine if tests should be skipped or excluded.

lib/ex_unit/lib/ex_unit/manifest.ex

@@ -16,6 +16,18 @@ defmodule ExUnit.Manifest do
     []
   end
 
+  @spec files_with_failures(t) :: MapSet.t(Path.t())
+  def files_with_failures(manifest) do
+    for({_, entry(last_run_status: :failed, file: file)} <- manifest, do: file, uniq: true)
+    |> MapSet.new()
+  end
+
+  @spec failed_test_ids(t) :: MapSet.t(test_id)
+  def failed_test_ids(manifest) do
+    for({test_id, entry(last_run_status: :failed)} <- manifest, do: test_id, uniq: true)
+    |> MapSet.new()
+  end
+
   @spec add_test(t, ExUnit.Test.t()) :: t
   def add_test(manifest, %ExUnit.Test{tags: %{file: file}})
       when not is_binary(file),

lib/ex_unit/lib/ex_unit/runner.ex

@@ -34,6 +34,7 @@ defmodule ExUnit.Runner do
       include: opts[:include],
       manager: manager,
       max_cases: opts[:max_cases],
+      only_test_ids: opts[:only_test_ids],
       seed: opts[:seed],
       modules: :async,
       timeout: opts[:timeout],
@@ -132,8 +133,9 @@ defmodule ExUnit.Runner do
     tests = shuffle(config, tests)
     include = config.include
     exclude = config.exclude
+    test_ids = config.only_test_ids
 
-    for test <- tests do
+    for test <- tests, include_test?(test_ids, test) do
       tags = Map.merge(test.tags, %{test: test.name, module: test.module})
 
       case ExUnit.Filters.eval(include, exclude, tags, tests) do
@@ -143,6 +145,12 @@ defmodule ExUnit.Runner do
     end
   end
 
+  defp include_test?(nil, _test), do: true
+
+  defp include_test?(test_ids, test) do
+    MapSet.member?(test_ids, {test.module, test.name})
+  end
+
   defp spawn_module(config, test_module, tests) do
     parent = self()
 

lib/ex_unit/test/ex_unit/manifest_test.exs

@@ -6,6 +6,37 @@ defmodule ExUnit.ManifestTest do
   import ExUnit.Manifest
   import ExUnit.TestHelpers, only: [tmp_path: 0, in_tmp: 2]
 
+  describe "files_with_failures/1" do
+    test "returns a set of the files that have failures" do
+      manifest = [
+        {{TestMod1, :test_1}, entry(last_run_status: :failed, file: "file_1")},
+        {{TestMod1, :test_2}, entry(last_run_status: :failed, file: "file_1")},
+        {{TestMod2, :test_1}, entry(last_run_status: :passed, file: "file_2")},
+        {{TestMod3, :test_1}, entry(last_run_status: :failed, file: "file_3")}
+      ]
+
+      assert files_with_failures(manifest) == MapSet.new(["file_1", "file_3"])
+    end
+  end
+
+  describe "failed_test_ids/1" do
+    test "returns the set of test ids" do
+      manifest = [
+        {{TestMod1, :test_1}, entry(last_run_status: :failed, file: "file_1")},
+        {{TestMod1, :test_2}, entry(last_run_status: :failed, file: "file_1")},
+        {{TestMod2, :test_1}, entry(last_run_status: :passed, file: "file_2")},
+        {{TestMod3, :test_1}, entry(last_run_status: :failed, file: "file_3")}
+      ]
+
+      assert failed_test_ids(manifest) ==
+               MapSet.new([
+                 {TestMod1, :test_1},
+                 {TestMod1, :test_2},
+                 {TestMod3, :test_1}
+               ])
+    end
+  end
+
   describe "add_test/2" do
     test "ignores tests that have an invalid :file value (which can happen when returning a `:file` option from `setup`))" do
       test = %ExUnit.Test{state: nil, tags: %{file: :not_a_file}}

lib/ex_unit/test/ex_unit_test.exs

@@ -136,6 +136,26 @@ defmodule ExUnitTest do
     assert config[:max_cases] == 1
   end
 
+  test "filters to the given test ids when an `only_test_ids` option is provided" do
+    defmodule TestIdTestModule do
+      use ExUnit.Case
+
+      test "passing", do: :ok
+      test "failing", do: assert(1 == 2)
+    end
+
+    test_ids =
+      MapSet.new([
+        {TestIdTestModule, :"test failing"},
+        {TestIdTestModule, :"test missing"},
+        {MissingModule, :"test passing"}
+      ])
+
+    {result, output} = run_with_filter([only_test_ids: test_ids], [])
+    assert result == %{failures: 1, skipped: 0, excluded: 0, total: 1}
+    assert output =~ "1 test, 1 failure"
+  end
+
   test "filtering cases with tags" do
     defmodule ParityTest do
       use ExUnit.Case

lib/mix/lib/mix/tasks/test.ex

@@ -75,6 +75,7 @@ defmodule Mix.Tasks.Test do
       Automatically sets `--trace` and `--preload-modules`
     * `--stale` - runs only tests which reference modules that changed since the
       last `test --stale`. You can read more about this option in the "Stale" section below.
+    * `--failed` - runs only tests that failed the last time they ran
     * `--timeout` - sets the timeout for the tests
     * `--trace` - runs tests with detailed reporting; automatically sets `--max-cases` to 1
 
@@ -188,6 +189,7 @@ defmodule Mix.Tasks.Test do
     deps_check: :boolean,
     archives_check: :boolean,
     elixir_version_check: :boolean,
+    failed: :boolean,
     stale: :boolean,
     listen_on_stdin: :boolean,
     formatter: :keep,
@@ -249,7 +251,7 @@ defmodule Mix.Tasks.Test do
     # Configure ExUnit with command line options before requiring
     # test helpers so that the configuration is available in helpers.
     # Then configure ExUnit again so command line options override
-    ex_unit_opts = ex_unit_opts(opts)
+    {ex_unit_opts, allowed_files} = process_ex_unit_opts(opts)
     ExUnit.configure(ex_unit_opts)
 
     test_paths = project[:test_paths] || default_test_paths()
@@ -261,7 +263,11 @@ defmodule Mix.Tasks.Test do
     test_pattern = project[:test_pattern] || "*_test.exs"
     warn_test_pattern = project[:warn_test_pattern] || "*_test.ex"
 
-    matched_test_files = Mix.Utils.extract_files(test_files, test_pattern)
+    matched_test_files =
+      test_files
+      |> Mix.Utils.extract_files(test_pattern)
+      |> filter_to_allowed_files(allowed_files)
+
     display_warn_test_pattern(test_files, test_pattern, matched_test_files, warn_test_pattern)
 
     case CT.require_and_run(matched_test_files, test_paths, opts) do
@@ -331,20 +337,28 @@ defmodule Mix.Tasks.Test do
     :formatters,
     :colors,
     :slowest,
-    :manifest_file
+    :manifest_file,
+    :only_test_ids
   ]
 
   @doc false
-  def ex_unit_opts(opts) do
-    opts
-    |> filter_opts(:include)
-    |> filter_opts(:exclude)
-    |> filter_opts(:only)
-    |> formatter_opts()
-    |> manifest_opts()
-    |> color_opts()
-    |> Keyword.take(@option_keys)
-    |> default_opts()
+  def process_ex_unit_opts(opts) do
+    {opts, allowed_files} =
+      opts
+      |> manifest_opts()
+      |> failed_opts()
+
+    opts =
+      opts
+      |> filter_opts(:include)
+      |> filter_opts(:exclude)
+      |> filter_opts(:only)
+      |> formatter_opts()
+      |> color_opts()
+      |> Keyword.take(@option_keys)
+      |> default_opts()
+
+    {opts, allowed_files}
   end
 
   defp merge_helper_opts(opts) do
@@ -417,6 +431,25 @@ defmodule Mix.Tasks.Test do
     Keyword.put(opts, :manifest_file, manifest_file)
   end
 
+  defp failed_opts(opts) do
+    if opts[:failed] do
+      if opts[:stale] do
+        Mix.raise("Combining `--failed` and `--stale` is not supported.")
+      end
+
+      {allowed_files, failed_ids} = ExUnit.Filters.failure_info(opts[:manifest_file])
+      {Keyword.put(opts, :only_test_ids, failed_ids), allowed_files}
+    else
+      {opts, nil}
+    end
+  end
+
+  defp filter_to_allowed_files(matched_test_files, nil), do: matched_test_files
+
+  defp filter_to_allowed_files(matched_test_files, %MapSet{} = allowed_files) do
+    Enum.filter(matched_test_files, &MapSet.member?(allowed_files, Path.expand(&1)))
+  end
+
   defp color_opts(opts) do
     case Keyword.fetch(opts, :color) do
       {:ok, enabled?} ->

lib/mix/test/fixtures/test_failed/mix.exs

@@ -0,0 +1,11 @@
+defmodule TestFailed.MixProject do
+  use Mix.Project
+
+  def project do
+    [
+      app: :test_only_failures,
+      version: "0.0.1",
+      test_pattern: "*_test_failed.exs"
+    ]
+  end
+end

lib/mix/test/fixtures/test_failed/test/only_failing_test_failed.exs

@@ -0,0 +1,5 @@
+defmodule OnlyFailingTest do
+  use ExUnit.Case
+
+  test "fails", do: assert(System.get_env("PASS_FAILING_TESTS"))
+end

lib/mix/test/fixtures/test_failed/test/only_passing_test_failed.exs

@@ -0,0 +1,7 @@
+IO.puts("loading OnlyPassingTest")
+
+defmodule OnlyPassingTest do
+  use ExUnit.Case
+
+  test "passes", do: assert(1 == 1)
+end

lib/mix/test/fixtures/test_failed/test/passing_and_failing_test_failed.exs

@@ -0,0 +1,9 @@
+defmodule PassingAndFailingTest do
+  use ExUnit.Case
+
+  @tag :foo
+  test "fails", do: assert(System.get_env("PASS_FAILING_TESTS"))
+
+  @tag :foo
+  test "passes", do: assert(1 == 1)
+end

lib/mix/test/fixtures/test_failed/test/test_helper.exs

@@ -0,0 +1 @@
+ExUnit.start()

lib/mix/test/mix/tasks/test_test.exs

@@ -3,8 +3,6 @@ Code.require_file("../../test_helper.exs", __DIR__)
 defmodule Mix.Tasks.TestTest do
   use MixTest.Case
 
-  import Mix.Tasks.Test, only: [ex_unit_opts: 1]
-
   test "ex_unit_opts/1 returns ex unit options" do
     assert ex_unit_opts_from_given(unknown: "ok", seed: 13) == [seed: 13]
   end
@@ -40,9 +38,14 @@ defmodule Mix.Tasks.TestTest do
            ]
   end
 
+  defp ex_unit_opts(opts) do
+    {ex_unit_opts, _allowed_files} = Mix.Tasks.Test.process_ex_unit_opts(opts)
+    ex_unit_opts
+  end
+
   defp ex_unit_opts_from_given(passed) do
     passed
-    |> Mix.Tasks.Test.ex_unit_opts()
+    |> ex_unit_opts()
     |> Keyword.drop([:manifest_file, :autorun])
   end
 
@@ -120,6 +123,46 @@ defmodule Mix.Tasks.TestTest do
     end
   end
 
+  test "--failed: loads only files with failures and runs just the failures" do
+    in_fixture "test_failed", fn ->
+      loading_only_passing_test_msg = "loading OnlyPassingTest"
+
+      # Run `mix test` once to record failures...
+      output = mix(["test"])
+      assert output =~ loading_only_passing_test_msg
+      assert output =~ "4 tests, 2 failures"
+
+      # `mix test --failed` runs only failed tests and avoids loading files with no failures
+      output = mix(["test", "--failed"])
+      refute output =~ loading_only_passing_test_msg
+      assert output =~ "2 tests, 2 failures"
+
+      # `mix test --failed` can be applied to a directory or file
+      output = mix(["test", "test/passing_and_failing_test_failed.exs", "--failed"])
+      assert output =~ "1 test, 1 failure"
+
+      # `--failed` composes with an `--only` filter by running the intersection.
+      # Of the failing tests, 1 is tagged with `@tag :foo`.
+      # Of the passing tests, 1 is tagged with `@tag :foo`.
+      # But only the failing test with that tag should run.
+      output = mix(["test", "--failed", "--only", "foo"])
+      assert output =~ "2 tests, 1 failure, 1 excluded"
+
+      # Run again to give it a chance to record as passed
+      System.put_env("PASS_FAILING_TESTS", "true")
+      assert mix(["test", "--failed"]) =~ "2 tests, 0 failures"
+
+      # Nothing should get run if we try it again since everything is passing.
+      assert mix(["test", "--failed"]) =~ "There are no tests to run"
+
+      # `--failed` and `--stale` cannot be combined
+      output = mix(["test", "--failed", "--stale"])
+      assert output =~ "Combining `--failed` and `--stale` is not supported"
+    end
+  after
+    System.delete_env("PASS_FAILING_TEST")
+  end
+
   test "logs test absence for a project with no test paths" do
     in_fixture "test_stale", fn ->
       File.rm_rf!("test")