Have mix test fail if warnings and --warnings-as-errors (#14756)

Author: sabiwara

lib/elixir/pages/references/compatibility-and-deprecations.md

@@ -34,7 +34,7 @@ Although we expect the vast majority of programs to remain compatible over time,
 
   * Bugs: if an API has undesired behavior, a program that depends on the buggy behavior may break if the bug is fixed. We reserve the right to fix such bugs.
 
-  * Compiler front-end: improvements may be done to the compiler, introducing new warnings for ambiguous modes and providing more detailed error messages. Those can lead to compilation errors (when running with `--warning-as-errors`) or tooling failures when asserting on specific error messages (although one should avoid such). We reserve the right to do such improvements.
+  * Compiler front-end: improvements may be done to the compiler, introducing new warnings for ambiguous modes and providing more detailed error messages. Those can lead to compilation errors (when running with `--warnings-as-errors`) or tooling failures when asserting on specific error messages (although one should avoid such). We reserve the right to do such improvements.
 
   * Imports: new functions may be added to the `Kernel` module, which is auto-imported. They may collide with local functions defined in your modules. Collisions can be resolved in a backwards compatible fashion using `import Kernel, except: [...]` with a list of all functions you don't want to be imported from `Kernel`. We reserve the right to do such additions.
 

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

@@ -666,23 +666,26 @@ defmodule Mix.Tasks.Test do
     warn_files != [] && warn_misnamed_test_files(warn_files)
 
     try do
-      Enum.each(test_paths, &require_test_helper(shell, &1))
+      helper_warned? = Enum.any?(test_paths, &require_test_helper(shell, &1))
       # test_opts always wins because those are given via args
       ExUnit.configure(ex_unit_opts |> merge_helper_opts() |> Keyword.merge(test_opts))
-      CT.require_and_run(matched_test_files, test_paths, test_elixirc_options, opts)
+
+      {CT.require_and_run(matched_test_files, test_paths, test_elixirc_options, opts),
+       helper_warned?}
     catch
       kind, reason ->
         # Also mark the whole suite as failed
         file = get_manifest_path(opts)
         ExUnit.Filters.fail_all!(file)
         :erlang.raise(kind, reason, __STACKTRACE__)
     else
-      {:ok, %{excluded: excluded, failures: failures, warnings?: warnings?, total: total}} ->
+      {{:ok, %{excluded: excluded, failures: failures, warnings?: warnings?, total: total}},
+       helper_warned?} ->
         Mix.shell(shell)
         cover && cover.()
 
         cond do
-          warnings_as_errors? and warnings? and failures == 0 ->
+          warnings_as_errors? and (warnings? or helper_warned?) and failures == 0 ->
             message =
               "\nERROR! Test suite aborted after successful execution due to warnings while using the --warnings-as-errors option"
 
@@ -715,7 +718,7 @@ defmodule Mix.Tasks.Test do
             :ok
         end
 
-      :noop ->
+      {:noop, _} ->
         cond do
           opts[:stale] ->
             Mix.shell().info("No stale tests")
@@ -1024,7 +1027,8 @@ defmodule Mix.Tasks.Test do
     file = Path.join(dir, "test_helper.exs")
 
     if File.exists?(file) do
-      Code.require_file(file)
+      {_result, warnings} = Code.with_diagnostics([log: true], fn -> Code.require_file(file) end)
+      warnings != []
     else
       raise_with_shell(
         shell,

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

@@ -593,6 +593,25 @@ defmodule Mix.Tasks.TestTest do
         assert output =~ "2 failures"
       end)
     end
+
+    test "fail with exit status 1 if warning in test_helper.exs" do
+      in_fixture("test_stale", fn ->
+        File.write!("test/test_helper.exs", """
+        unused_var = 123
+
+        ExUnit.start()
+        """)
+
+        msg =
+          "Test suite aborted after successful execution due to warnings while using the --warnings-as-errors option"
+
+        {output, exit_status} = mix_code(["test", "--warnings-as-errors"])
+
+        assert output =~ "variable \"unused_var\" is unused"
+        assert output =~ msg
+        assert exit_status == 1
+      end)
+    end
   end
 
   describe "--exit-status" do