Return --exit-status + 1 when warnings as errors and test failures (#14405)

Author: naserca

lib/mix/lib/mix/compilers/test.ex

@@ -77,18 +77,17 @@ defmodule Mix.Compilers.Test do
 
       true ->
         task = ExUnit.async_run()
-        warnings_as_errors? = Keyword.get(opts, :warnings_as_errors, false)
         seed = Application.fetch_env!(:ex_unit, :seed)
         rand_algorithm = Application.fetch_env!(:ex_unit, :rand_algorithm)
         test_files = shuffle(seed, rand_algorithm, test_files)
 
         try do
           parallel_require_options = shared_require_options ++ parallel_require_callbacks
 
-          failed? =
+          warnings? =
             case Kernel.ParallelCompiler.require(test_files, parallel_require_options) do
               {:ok, _, %{compile_warnings: c, runtime_warnings: r}}
-              when (c != [] or r != []) and warnings_as_errors? ->
+              when c != [] or r != [] ->
                 true
 
               {:ok, _, _} ->
@@ -101,18 +100,10 @@ defmodule Mix.Compilers.Test do
           %{failures: failures} = results = ExUnit.await_run(task)
 
           if failures == 0 do
-            if failed? do
-              message =
-                "\nERROR! Test suite aborted after successful execution due to warnings while using the --warnings-as-errors option"
-
-              IO.puts(:stderr, IO.ANSI.format([:red, message]))
-              exit({:shutdown, 1})
-            end
-
             agent_write_manifest(stale_manifest_pid)
           end
 
-          {:ok, results}
+          {:ok, Map.put(results, :warnings?, warnings?)}
         catch
           kind, reason ->
             # In case there is an error, shut down the runner task

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

@@ -209,8 +209,12 @@ defmodule Mix.Tasks.Test do
       Note that in trace mode test timeouts will be ignored as timeout is set to `:infinity`
 
     * `--warnings-as-errors` *(since v1.12.0)* - treats compilation warnings (from loading the
-      test suite) as errors and returns a non-zero exit status if the test suite would otherwise
-      pass. Note that failures reported by `--warnings-as-errors` cannot be retried with the
+      test suite) as errors and returns an exit status of 1 if the test suite would otherwise
+      pass. If the test suite fails and also include warnings as errors, the exit
+      status returned will be the value of the `--exit-status` option, which
+      defaults to 2, plus one. Therefore in the default case, this will be exit status 3.
+
+      Note that failures reported by `--warnings-as-errors` cannot be retried with the
       `--failed` flag.
 
       This option only applies to test files. To treat warnings as errors during compilation and
@@ -617,6 +621,9 @@ defmodule Mix.Tasks.Test do
     {ex_unit_opts, allowed_files} = process_ex_unit_opts(opts)
     ExUnit.configure(ex_unit_opts)
 
+    warnings_as_errors? = Keyword.get(opts, :warnings_as_errors, false)
+    exit_status = Keyword.fetch!(ex_unit_opts, :exit_status)
+
     # Prepare and extract all files to require and run
     test_paths = project[:test_paths] || default_test_paths()
     test_pattern = project[:test_pattern] || "*.{ex,exs}"
@@ -664,17 +671,32 @@ defmodule Mix.Tasks.Test do
         ExUnit.Filters.fail_all!(file)
         :erlang.raise(kind, reason, __STACKTRACE__)
     else
-      {:ok, %{excluded: excluded, failures: failures, total: total}} ->
+      {:ok, %{excluded: excluded, failures: failures, warnings?: warnings?, total: total}} ->
         Mix.shell(shell)
         cover && cover.()
 
         cond do
+          warnings_as_errors? and warnings? and failures == 0 ->
+            message =
+              "\nERROR! Test suite aborted after successful execution due to warnings while using the --warnings-as-errors option"
+
+            IO.puts(:stderr, IO.ANSI.format([:red, message]))
+
+            System.at_exit(fn _ ->
+              exit({:shutdown, 1})
+            end)
+
           failures > 0 and opts[:raise] ->
             raise_with_shell(shell, "\"mix test\" failed")
 
+          warnings_as_errors? and warnings? and failures > 0 ->
+            System.at_exit(fn _ ->
+              exit({:shutdown, exit_status + 1})
+            end)
+
           failures > 0 ->
             System.at_exit(fn _ ->
-              exit({:shutdown, Keyword.fetch!(ex_unit_opts, :exit_status)})
+              exit({:shutdown, exit_status})
             end)
 
           excluded == total and Keyword.has_key?(opts, :only) ->

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

@@ -558,7 +558,7 @@ defmodule Mix.Tasks.TestTest do
   end
 
   describe "--warnings-as-errors" do
-    test "fail on warning in tests" do
+    test "fail with exit status 1 if warning in tests but tests pass" do
       in_fixture("test_stale", fn ->
         msg =
           "Test suite aborted after successful execution due to warnings while using the --warnings-as-errors option"
@@ -579,10 +579,42 @@ defmodule Mix.Tasks.TestTest do
         end
         """)
 
-        output = mix(["test", "--warnings-as-errors", "test/warning_test_stale.exs"])
+        {output, exit_status} =
+          mix_code(["test", "--warnings-as-errors", "test/warning_test_stale.exs"])
+
         assert output =~ "variable \"unused_compile_var\" is unused"
         assert output =~ "variable \"unused_test_var\" is unused"
         assert output =~ msg
+        assert exit_status == 1
+      end)
+    end
+
+    test "fail with --exit-status + 1 if warning in tests and tests fail" do
+      in_fixture("test_stale", fn ->
+        File.write!("test/warning_test_warnings_as_errors_and_failures.exs", """
+        defmodule WarningsAsErrorsAndFailuresTest do
+          use ExUnit.Case
+
+          test "warning and failure" do
+            unused_test_var = 1
+            assert false
+          end
+        end
+        """)
+
+        {output, exit_status} =
+          mix_code([
+            "test",
+            "--warnings-as-errors",
+            "--exit-status",
+            "42",
+            "test/warning_test_warnings_as_errors_and_failures.exs"
+          ])
+
+        assert output =~ "variable \"unused_test_var\" is unused"
+        assert output =~ "1 failure"
+
+        assert exit_status == 43
       end)
     end