Kill test process only after on_exit runs

We do this to avoid links created during the test from
crashing when the test exits which happened before the on_exit
callbacks run.

This was particularly troubling with GenServer, GenEvent
and Supervisor, in which the first does not crash when
a linked process exits with normal reason, but the other
two do. Due to this behaviour, before this change there was
a possibility of race conditions causing tests to crash on
on_exit.

In case the test process exits with a non-normal reason,
all links will be broken, so the problem doesn't manifest
in such cases.

Author: José Valim

lib/ex_unit/lib/ex_unit/runner.ex

@@ -138,7 +138,7 @@ defmodule ExUnit.Runner do
   end
 
   defp spawn_case(config, test_case, tests) do
-    self_pid = self
+    parent = self
 
     {case_pid, case_ref} =
       spawn_monitor(fn ->
@@ -148,12 +148,14 @@ defmodule ExUnit.Runner do
           {:ok, test_case, context} ->
             Enum.each(tests, &run_test(config, &1, context))
             test_case = exec_case_teardown(test_case, context)
-            send self_pid, {self, :case_finished, test_case, []}
+            send parent, {self, :case_finished, test_case, []}
 
           {:error, test_case} ->
             failed_tests = Enum.map(tests, & %{&1 | state: {:invalid, test_case}})
-            send self_pid, {self, :case_finished, test_case, failed_tests}
+            send parent, {self, :case_finished, test_case, failed_tests}
         end
+
+        receive do: ({^parent, :done} -> :done)
       end)
 
     {test_case, pending} =
@@ -165,7 +167,9 @@ defmodule ExUnit.Runner do
           {test_case, []}
       end
 
-    {exec_on_exit(test_case, case_pid), pending}
+    test_case = exec_on_exit(test_case, case_pid)
+    send case_pid, {parent, :done}
+    {test_case, pending}
   end
 
   defp exec_case_setup(%ExUnit.TestCase{name: case_name} = test_case) do
@@ -197,7 +201,7 @@ defmodule ExUnit.Runner do
   end
 
   defp spawn_test(_config, test, context) do
-    self_pid = self
+    parent = self()
 
     {test_pid, test_ref} =
       spawn_monitor(fn ->
@@ -214,7 +218,8 @@ defmodule ExUnit.Runner do
             end
           end)
 
-        send self_pid, {self, :test_finished, %{test | time: us}}
+        send parent, {self, :test_finished, %{test | time: us}}
+        receive do: ({^parent, :done} -> :done)
       end)
 
     test =
@@ -225,7 +230,9 @@ defmodule ExUnit.Runner do
           %{test | state: {:failed, {{:EXIT, test_pid}, error, []}}}
       end
 
-    exec_on_exit(test, test_pid)
+    test = exec_on_exit(test, test_pid)
+    send test_pid, {parent, :done}
+    test
   end
 
   defp exec_test_setup(%ExUnit.Test{case: case} = test, context) do

lib/ex_unit/test/ex_unit/callbacks_test.exs

@@ -53,7 +53,7 @@ defmodule ExUnit.CallbacksTest do
 
   test "doesn't choke on setup_all errors" do
     defmodule SetupAllTest do
-      use ExUnit.Case, async: false
+      use ExUnit.Case
 
       setup_all _ do
         :ok = error
@@ -72,7 +72,7 @@ defmodule ExUnit.CallbacksTest do
 
   test "doesn't choke on on_exit errors" do
     defmodule OnExitErrorTest do
-      use ExUnit.Case, async: false
+      use ExUnit.Case
 
       test "ok" do
         on_exit fn -> :ok = error end
@@ -105,9 +105,37 @@ defmodule ExUnit.CallbacksTest do
     on_exit fn -> ExUnit.configure(formatters: [ExUnit.CLIFormatter]) end
   end
 
+  test "kills test process only after on_exit runs" do
+    defmodule OnExitAliveTest do
+      use ExUnit.Case
+
+      setup do
+        pid = spawn_link fn ->
+          Process.flag(:trap_exit, true)
+          receive do: ({:EXIT, _, _} -> :ok)
+        end
+
+        on_exit fn ->
+          assert Process.alive?(pid)
+          IO.puts "on_exit run"
+        end
+
+        :ok
+      end
+
+      test "ok" do
+        :ok
+      end
+    end
+
+    output = capture_io(fn -> ExUnit.run end)
+    assert output =~ "on_exit run"
+    assert output =~ "1 tests, 0 failures"
+  end
+
   test "runs multiple on_exit exits and overrides by ref" do
     defmodule OnExitSuccessTest do
-      use ExUnit.Case, async: false
+      use ExUnit.Case
 
       setup do
         on_exit fn ->
@@ -166,7 +194,7 @@ defmodule ExUnit.CallbacksTest do
 
   test "runs multiple on_exit on failure" do
     defmodule OnExitFailureTest do
-      use ExUnit.Case, async: false
+      use ExUnit.Case
 
       setup do
         on_exit fn ->