Taint failure manifest if requiring or compiling tests fail, closes #13728

Author: josevalim

lib/ex_unit/lib/ex_unit/failures_manifest.ex

@@ -8,20 +8,6 @@ defmodule ExUnit.FailuresManifest do
   @spec new() :: t
   def new, do: %{}
 
-  @spec files_with_failures(t) :: MapSet.t(Path.t())
-  def files_with_failures(%{} = manifest) do
-    manifest
-    |> Map.values()
-    |> MapSet.new()
-  end
-
-  @spec failed_test_ids(t) :: MapSet.t(ExUnit.test_id())
-  def failed_test_ids(%{} = manifest) do
-    manifest
-    |> Map.keys()
-    |> MapSet.new()
-  end
-
   @spec put_test(t, ExUnit.Test.t()) :: t
   def put_test(%{} = manifest, %ExUnit.Test{state: {ignored_state, _}})
       when ignored_state in [:skipped, :excluded],
@@ -44,21 +30,43 @@ defmodule ExUnit.FailuresManifest do
     File.write!(file, binary)
   end
 
+  @spec fail_all!(Path.t()) :: :ok
+  def fail_all!(file) when is_binary(file) do
+    binary = :erlang.term_to_binary({@manifest_vsn, :all})
+    Path.dirname(file) |> File.mkdir_p!()
+    File.write!(file, binary)
+  end
+
   @spec read(Path.t()) :: t
   def read(file) when is_binary(file) do
     with {:ok, binary} <- File.read(file),
-         {:ok, {@manifest_vsn, manifest}} when is_map(manifest) <- safe_binary_to_term(binary) do
+         {:ok, {@manifest_vsn, %{} = manifest}} <- safe_binary_to_term(binary) do
       manifest
     else
       _ -> new()
     end
   end
 
+  @spec info(Path.t()) :: {MapSet.t(Path.t()), MapSet.t(ExUnit.test_id())} | :all
+  def info(file) when is_binary(file) do
+    with {:ok, binary} <- File.read(file),
+         {:ok, {@manifest_vsn, manifest}} <- safe_binary_to_term(binary) do
+      case manifest do
+        :all ->
+          :all
+
+        %{} ->
+          {manifest |> Map.values() |> MapSet.new(), manifest |> Map.keys() |> MapSet.new()}
+      end
+    else
+      _ -> {MapSet.new(), MapSet.new()}
+    end
+  end
+
   defp safe_binary_to_term(binary) do
     {:ok, :erlang.binary_to_term(binary)}
   rescue
-    ArgumentError ->
-      :error
+    ArgumentError -> :error
   end
 
   defp prune_deleted_tests(manifest) do

lib/ex_unit/lib/ex_unit/filters.ex

@@ -146,18 +146,31 @@ defmodule ExUnit.Filters do
   defp parse_kv(key, value), do: {key, value}
 
   @doc """
-  Returns a tuple containing useful information about test failures from the
-  manifest. The tuple contains:
+  Returns failure information from the manifest file.
+
+  It returns either `:all`, meaning all tests should be considered as stale,
+  or a tuple containing:
 
     * 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(ExUnit.test_id())}
+  @spec failure_info(Path.t()) :: {MapSet.t(Path.t()), MapSet.t(ExUnit.test_id())} | :all
   def failure_info(manifest_file) do
-    manifest = FailuresManifest.read(manifest_file)
-    {FailuresManifest.files_with_failures(manifest), FailuresManifest.failed_test_ids(manifest)}
+    FailuresManifest.info(manifest_file)
+  end
+
+  @doc """
+  Marks the whole suite as failed in the manifest.
+
+  This is useful when the test suite cannot be loaded
+  and there is a desire to make all tests fail.
+  """
+  @spec fail_all!(Path.t()) :: :ok
+  def fail_all!(manifest_file) do
+    FailuresManifest.fail_all!(manifest_file)
   end
 
   @doc """

lib/ex_unit/test/ex_unit/failures_manifest_test.exs

@@ -10,29 +10,40 @@ defmodule ExUnit.FailuresManifestTest do
   @excluded {:excluded, "reason"}
   @failed {:failed, []}
   @invalid {:invalid, %ExUnit.TestModule{}}
+  @manifest_path "example.manifest"
 
-  describe "files_with_failures/1" do
-    test "returns the set of files with failures" do
+  describe "info/1" do
+    @tag :tmp_dir
+    test "returns the sets of files and test IDs with failures", context do
       manifest =
         new()
-        |> put_test(new_test(@failed, "file_1"))
-        |> put_test(new_test(@failed, "file_2"))
-        |> put_test(new_test(@passed, "file_3"))
-        |> put_test(new_test(@failed, "file_1"))
+        |> put_test(failed_1 = new_test(@failed, context))
+        |> put_test(failed_2 = new_test(@failed, context))
+        |> put_test(new_test(@passed, context))
+        |> put_test(invalid_1 = new_test(@invalid, context))
+
+      File.cd!(context.tmp_dir, fn ->
+        write!(manifest, @manifest_path)
 
-      assert files_with_failures(manifest) == MapSet.new(["file_1", "file_2"])
+        assert info(@manifest_path) ==
+                 {MapSet.new([context.file]),
+                  MapSet.new([test_id(failed_1), test_id(failed_2), test_id(invalid_1)])}
+      end)
     end
-  end
 
-  describe "failed_test_ids/1" do
-    test "returns the set of failed test IDs" do
-      manifest =
-        new()
-        |> put_test(failed_1 = new_test(@failed))
-        |> put_test(__passed = new_test(@passed))
-        |> put_test(failed_2 = new_test(@invalid))
+    @tag :tmp_dir
+    test "returns all when the whole suite should be considered as failed", context do
+      File.cd!(context.tmp_dir, fn ->
+        fail_all!(@manifest_path)
+        assert info(@manifest_path) == :all
+      end)
+    end
 
-      assert failed_test_ids(manifest) == MapSet.new([test_id(failed_1), test_id(failed_2)])
+    @tag :tmp_dir
+    test "returns no information when loading a file that does not exit", context do
+      path = Path.join(context.tmp_dir, "missing.manifest")
+      refute File.exists?(path)
+      assert info(path) == {MapSet.new(), MapSet.new()}
     end
   end
 
@@ -93,8 +104,6 @@ defmodule ExUnit.FailuresManifestTest do
     end
   end
 
-  @manifest_path "example.manifest"
-
   describe "write!/2" do
     @tag :tmp_dir
     test "stores a manifest that can later be read with read/1", context do
@@ -172,7 +181,6 @@ defmodule ExUnit.FailuresManifestTest do
         assert write!(manifest, @manifest_path) == :ok
         assert {vsn, ^manifest} = @manifest_path |> File.read!() |> :erlang.binary_to_term()
         File.write!(@manifest_path, :erlang.term_to_binary({vsn + 1, manifest}))
-
         assert read(@manifest_path) == new()
       end)
     end

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

@@ -473,6 +473,7 @@ defmodule Mix.Tasks.Test do
   @impl true
   def run(args) do
     {opts, files} = OptionParser.parse!(args, strict: @switches, aliases: [b: :breakpoints])
+    opts = put_manifest_file(opts)
 
     if not Mix.Task.recursing?() do
       do_run(opts, args, files)
@@ -591,30 +592,31 @@ defmodule Mix.Tasks.Test do
     {ex_unit_opts, allowed_files} = process_ex_unit_opts(opts)
     ExUnit.configure(ex_unit_opts)
 
+    # Prepare and extract all files to require and run
     test_paths = project[:test_paths] || default_test_paths()
-    Enum.each(test_paths, &require_test_helper(shell, &1))
-    ExUnit.configure(merge_helper_opts(ex_unit_opts))
-
-    # Finally parse, require and load the files
     test_files = if files != [], do: parse_file_paths(files), else: test_paths
     test_pattern = project[:test_pattern] || "*_test.exs"
     warn_test_pattern = project[:warn_test_pattern] || "*_test.ex"
-
-    files_with_matched_path = Mix.Utils.extract_files(test_files, test_pattern)
+    unfiltered_test_files = Mix.Utils.extract_files(test_files, test_pattern)
 
     matched_test_files =
-      files_with_matched_path
+      unfiltered_test_files
       |> filter_to_allowed_files(allowed_files)
       |> filter_by_partition(shell, partitions)
 
-    display_warn_test_pattern(
-      test_files,
-      test_pattern,
-      files_with_matched_path,
-      warn_test_pattern
-    )
-
-    case CT.require_and_run(matched_test_files, test_paths, test_elixirc_options, opts) do
+    display_warn_test_pattern(test_files, test_pattern, unfiltered_test_files, warn_test_pattern)
+
+    try do
+      Enum.each(test_paths, &require_test_helper(shell, &1))
+      ExUnit.configure(merge_helper_opts(ex_unit_opts))
+      CT.require_and_run(matched_test_files, test_paths, test_elixirc_options, opts)
+    catch
+      kind, reason ->
+        # Also mark the whole suite as failed
+        file = Keyword.fetch!(opts, :failures_manifest_path)
+        ExUnit.Filters.fail_all!(file)
+        :erlang.raise(kind, reason, __STACKTRACE__)
+    else
       {:ok, %{excluded: excluded, failures: failures, total: total}} ->
         Mix.shell(shell)
         cover && cover.()
@@ -776,23 +778,29 @@ defmodule Mix.Tasks.Test do
 
   @manifest_file_name ".mix_test_failures"
 
-  defp manifest_opts(opts) do
-    opts =
-      Keyword.put_new(
-        opts,
-        :failures_manifest_path,
-        Path.join(Mix.Project.manifest_path(), @manifest_file_name)
-      )
+  defp put_manifest_file(opts) do
+    Keyword.put_new_lazy(
+      opts,
+      :failures_manifest_path,
+      fn -> Path.join(Mix.Project.manifest_path(), @manifest_file_name) end
+    )
+  end
 
-    manifest_file = Keyword.get(opts, :failures_manifest_path)
+  defp manifest_opts(opts) do
+    manifest_file = Keyword.fetch!(opts, :failures_manifest_path)
 
     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(manifest_file)
-      {Keyword.put(opts, :only_test_ids, failed_ids), allowed_files}
+      case ExUnit.Filters.failure_info(manifest_file) do
+        {allowed_files, failed_ids} ->
+          {Keyword.put(opts, :only_test_ids, failed_ids), allowed_files}
+
+        :all ->
+          {opts, nil}
+      end
     else
       {opts, nil}
     end

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

@@ -33,25 +33,14 @@ defmodule Mix.Tasks.TestTest do
     end
 
     test "accepts custom :exit_status" do
-      assert {:exit_status, 5} in ex_unit_opts(exit_status: 5)
+      assert {:exit_status, 5} in ex_unit_opts(exit_status: 5, failures_manifest_path: "foo.bar")
     end
 
     test "includes some default options" do
-      assert ex_unit_opts([]) == [
+      assert ex_unit_opts(failures_manifest_path: "foo.bar") == [
                autorun: false,
                exit_status: 2,
-               failures_manifest_path:
-                 Path.join(Mix.Project.manifest_path(), ".mix_test_failures")
-             ]
-    end
-
-    test "respect failures_manifest_path option" do
-      custom_manifest_file = Path.join(Mix.Project.manifest_path(), ".mix_test_failures_custom")
-
-      assert ex_unit_opts(failures_manifest_path: custom_manifest_file) == [
-               autorun: false,
-               exit_status: 2,
-               failures_manifest_path: custom_manifest_file
+               failures_manifest_path: "foo.bar"
              ]
     end
 
@@ -62,6 +51,7 @@ defmodule Mix.Tasks.TestTest do
 
     defp ex_unit_opts_from_given(passed) do
       passed
+      |> Keyword.put(:failures_manifest_path, "foo.bar")
       |> ex_unit_opts()
       |> Keyword.drop([:failures_manifest_path, :autorun, :exit_status])
     end
@@ -268,6 +258,18 @@ defmodule Mix.Tasks.TestTest do
     after
       System.delete_env("PASS_FAILING_TESTS")
     end
+
+    test "marks the whole suite as failed on compilation error" do
+      in_fixture("test_failed", fn ->
+        File.write!("test/passing_and_failing_test_failed.exs", "raise ~s(oops)")
+
+        output = mix(["test"])
+        assert output =~ "** (RuntimeError) oops"
+
+        output = mix(["test", "--failed"])
+        assert output =~ "** (RuntimeError) oops"
+      end)
+    end
   end
 
   describe "--listen-on-stdin" do