Deprecate teardown and teardown_all

Author: José Valim

CHANGELOG.md

@@ -15,6 +15,7 @@
   * [Kernel] Ensure Mix `_build` structure works on Windows when copying projects
 
 * Soft deprecations (no warnings emitted)
+  * [ExUnit] `teardown/2` and `teardown_all/2` are deprecated in favor of `on_exit/1` callbacks
 
 * Deprecations
   * [Access] `Access.access/2` is deprecated in favor of `Access.get/2`

lib/elixir/test/elixir/file_test.exs

@@ -12,11 +12,7 @@ defmodule Elixir.FileCase do
 
   setup do
     File.mkdir_p!(tmp_path)
-    :ok
-  end
-
-  teardown do
-    File.rm_rf(tmp_path)
+    on_exit(fn -> File.rm_rf(tmp_path) end)
     :ok
   end
 end

lib/ex_unit/lib/ex_unit/callbacks.ex

@@ -2,64 +2,73 @@ defmodule ExUnit.Callbacks do
   @moduledoc ~S"""
   Defines ExUnit Callbacks.
 
-  This module defines four callbacks: `setup_all`, `teardown_all`,
-  `setup` and `teardown`.
+  This module defines both `setup_all` and `setup` callbacks, as well as
+  the `on_exit` facility.
 
-  These callbacks are defined via macros and each one can optionally receive
-  a map with metadata, usually referred to as `context`. The callback
-  may optionally put extra data into `context` to be used in the tests.
+  The setup callbacks are defined via macros and each one can optionally
+  receive a map with metadata, usually referred to as `context`. The
+  callback may optionally put extra data into `context` to be used in
+  the tests.
 
-  If you return `{:ok, <dict>}` from `setup` or `teardown`, the keyword
-  list will be merged into the context that will be available in all
-  subsequent `setup`, `test` or `teardown` calls.
+  The `setup_all` callbacks are invoked once before the first test's `setup`
+  and all `setup` callbacks are run before each test. No callback runs if the
+  test case has no tests or all tests were filtered out.
 
-  Similarly, returning `{:ok, <dict>}` from `setup_all` or
-  `teardown_all` will merge the keyword list into the context that will be
-  available in all subsequent `setup_all` or `teardown_all` calls.
+  `on_exit` callbacks are registered on demand, usually to undo an action
+  performed by a setup callback. `on_exit` may also take a reference,
+  allowing callback to be overridden in the future. A registered `on_exit`
+  callback always runs, while failures in `setup` and `setup_all` will stop
+  all remaining setup callbacks from executing.
 
-  Returning `:ok` leaves the context unchanged in both cases.
+  Finally, `setup_all` callbacks run in the test case process, while all
+  `setup` callbacks run in the same process as the test itself. `on_exit`
+  callbacks always run in a separate process than the test case or the
+  test itself.
+
+  ## Context
+
+  If you return `{:ok, <dict>}` from `setup_all`, the dictionary
+  will be merged into the current context and be available in all
+  subsequent `setup_all`, `setup` and the test itself.
 
-  Returning anything else from `setup` or `teardown` will force the current
-  test to fail, and subsequent `setup`, `test` and `teardown` callbacks won't
-  be called for it.
+  Similarly, returning `{:ok, <dict>}` from `setup`, the dict returned
+  will be merged into the current context and be available in all
+  subsequent `setup` and the `test` itself.
 
-  Returning anything else from `setup_all` or `teardown_all` will force the
-  whole case to fail, and no other callback will be called.
+  Returning `:ok` leaves the context unchanged in both cases.
 
-  It is possible to define multiple `setup` and `teardown` callbacks and they will
-  be called sequentially. In the case of `setup_all` and `teardown_all` callbacks,
-  each `setup_all` will be called only once before the first test's `setup` and each
-  `teardown_all` will be called once after the last test. No callback runs if the
-  test case has no tests or all tests were filtered out via `include`/`exclude`.
+  Returning anything else from `setup_all` will force all tests to fail,
+  while a bad response from `setup` causes the current test to fail.
 
   ## Examples
 
       defmodule AssertionTest do
         use ExUnit.Case, async: true
 
+        # `setup_all` is called once before every test
+        setup_all do
+          IO.puts "Starting AssertionTest"
+
+          # No metadata
+          :ok
+        end
+
         # `setup` is called before each test is run
         setup do
           IO.puts "This is a setup callback"
 
-          # Return extra metadata, it must be a keyword list / map
+          on_exit fn ->
+            IO.puts "This is invoked once the test is done"
+          end
+
+          # Returns extra metadata, it must be a dict
           {:ok, hello: "world"}
         end
 
-        # Same as `setup`, but receives the context for the current test
+        # Same as `setup`, but receives the context
+        # for the current test
         setup context do
-          # We can access the current test in the context
           IO.puts "Setting up: #{context[:test]}"
-
-          # We can also access the data returned from `setup/0`
-          assert context[:hello] == "world"
-
-          # No metadata
-          :ok
-        end
-
-        # This is called after each test finishes
-        teardown context do
-          assert context[:hello] == "world"
           :ok
         end
 
@@ -96,7 +105,7 @@ defmodule ExUnit.Callbacks do
   end
 
   @doc """
-  Called before the start of each test.
+  Defines a callback to be run before each test in a case.
   """
   defmacro setup(var \\ quote(do: _), block) do
     quote bind_quoted: [var: escape(var), block: escape(block)] do
@@ -107,42 +116,49 @@ defmodule ExUnit.Callbacks do
   end
 
   @doc """
-  Called after the completion of each test. 
-  
-  Note that if the test crashed with an `:exit`
-  message, `teardown` will not be run.
+  Defines a callback to be run before all tests in a case.
   """
-  defmacro teardown(var \\ quote(do: _), block) do
+  defmacro setup_all(var \\ quote(do: _), block) do
     quote bind_quoted: [var: escape(var), block: escape(block)] do
-      name = :"__ex_unit_teardown_#{length(@ex_unit_teardown)}"
+      name = :"__ex_unit_setup_all_#{length(@ex_unit_setup_all)}"
       defp unquote(name)(unquote(var)), unquote(block)
-      @ex_unit_teardown [name|@ex_unit_teardown]
+      @ex_unit_setup_all [name|@ex_unit_setup_all]
     end
   end
 
-  @doc """
-  Called before the start of a case, i.e. called once before the first test in
-  the current module and before any `setup` callbacks.
-  """
-  defmacro setup_all(var \\ quote(do: _), block) do
+  @doc false
+  defmacro teardown(var \\ quote(do: _), block) do
+    # IO.write :stderr, "teardown in ExUnit is deprecated, please use on_exit/1 instead\n" <>
+    #                   Exception.format_stacktrace(Macro.Env.stacktrace(__CALLER__))
     quote bind_quoted: [var: escape(var), block: escape(block)] do
-      name = :"__ex_unit_setup_all_#{length(@ex_unit_setup_all)}"
+      name = :"__ex_unit_teardown_#{length(@ex_unit_teardown)}"
       defp unquote(name)(unquote(var)), unquote(block)
-      @ex_unit_setup_all [name|@ex_unit_setup_all]
+      @ex_unit_teardown [name|@ex_unit_teardown]
     end
   end
 
-  @doc """
-  Called once after the last test finishes without emitting an `:exit` message.
-  """
+  @doc false
   defmacro teardown_all(var \\ quote(do: _), block) do
+    # IO.write :stderr, "teardown_all in ExUnit is deprecated, please use on_exit/1 instead\n" <>
+    #                   Exception.format_stacktrace(Macro.Env.stacktrace(__CALLER__))
     quote bind_quoted: [var: escape(var), block: escape(block)] do
       name = :"__ex_unit_teardown_all_#{length(@ex_unit_teardown_all)}"
       defp unquote(name)(unquote(var)), unquote(block)
       @ex_unit_teardown_all [name|@ex_unit_teardown_all]
     end
   end
 
+  @doc """
+  Defines a callback that runs on the test (or test case) exit.
+
+  An `on_exit` callback is a function that receives no arguments and
+  runs in a separate process than the caller.
+
+  `on_exit/2` is usually called from `setup` and `setup_all` callbacks,
+  often to undo the action performed during `setup`. However, `on_exit`
+  may also be called dynamically, where a reference can be used to
+  guarantee the callback will be invoked only once.
+  """
   @spec on_exit(term, (() -> term)) :: :ok
   def on_exit(ref \\ make_ref, callback) do
     case ExUnit.OnExitHandler.add(self, ref, callback) do

lib/ex_unit/test/ex_unit_test.exs

@@ -5,11 +5,9 @@ defmodule ExUnitTest do
 
   setup do
     ExUnit.configure(formatters: [])
-    :ok
-  end
-
-  teardown do
-    ExUnit.configure(formatters: [ExUnit.CLIFormatter])
+    on_exit(fn ->
+      ExUnit.configure(formatters: [ExUnit.CLIFormatter])
+    end)
     :ok
   end
 

lib/mix/test/mix/shell_test.exs

@@ -11,6 +11,13 @@ defmodule Mix.ShellTest do
     ExUnit.CaptureIO.capture_io(from, somefunc) |> String.replace("\r\n","\n")
   end
 
+  setup do
+    on_exit fn ->
+      Mix.shell(Mix.Shell.Process)
+    end
+    :ok
+  end
+
   test "shell process" do
     Mix.shell.info "abc"
     Mix.shell.error "def"
@@ -57,9 +64,4 @@ defmodule Mix.ShellTest do
       assert Mix.shell.cmd("echo first && echo second") == 0
     end) |> String.replace(" \n", "\n")) == "first\nsecond\n"
   end
-
-  teardown do
-    Mix.shell(Mix.Shell.Process)
-    :ok
-  end
 end

lib/mix/test/mix/tasks/app.start_test.exs

@@ -22,22 +22,16 @@ defmodule Mix.Tasks.App.StartTest do
   end
 
   setup config do
-    if config[:app] do
+    if app = config[:app] do
       :error_logger.tty(false)
-    end
-    :ok
-  end
 
-  teardown config do
-    if app = config[:app] do
-      :application.stop(app)
-      :application.unload(app)
+      on_exit fn ->
+        :application.stop(app)
+        :application.unload(app)
+        :error_logger.tty(true)
+      end
     end
-    :ok
-  end
 
-  teardown do
-    :error_logger.tty(true)
     :ok
   end
 

lib/mix/test/mix/tasks/loadconfig_test.exs

@@ -5,10 +5,12 @@ defmodule Mix.Tasks.LoadconfigTest do
 
   @apps [:my_app, :other_app]
 
-  teardown do
-    Enum.each @apps, fn app ->
-      Enum.each Application.get_all_env(app), fn {key, _} ->
-        Application.delete_env(app, key, persistent: true)
+  setup do
+    on_exit fn ->
+      Enum.each @apps, fn app ->
+        Enum.each Application.get_all_env(app), fn {key, _} ->
+          Application.delete_env(app, key, persistent: true)
+        end
       end
     end
     :ok

lib/mix/test/test_helper.exs

@@ -19,14 +19,17 @@ defmodule MixTest.Case do
     end
   end
 
-  teardown do
-    Mix.env(:dev)
-    Mix.Task.clear
-    Mix.Shell.Process.flush
-    Mix.ProjectStack.clear_cache
-    Mix.ProjectStack.clear_stack
-    System.put_env("MIX_HOME", tmp_path(".mix"))
-    delete_tmp_paths
+  setup do
+    on_exit fn ->
+      Mix.env(:dev)
+      Mix.Task.clear
+      Mix.Shell.Process.flush
+      Mix.ProjectStack.clear_cache
+      Mix.ProjectStack.clear_stack
+      System.put_env("MIX_HOME", tmp_path(".mix"))
+      delete_tmp_paths
+    end
+
     :ok
   end
 
@@ -97,7 +100,7 @@ defmodule MixTest.Case do
       end
     end
   end
-    
+
   def os_newline do
     case :os.type do
       {:win32, _} -> "\r\n"