Raise if trying to override reserved tag, closes #4236

Author: José Valim

lib/ex_unit/lib/ex_unit/callbacks.ex

@@ -146,6 +146,8 @@ defmodule ExUnit.Callbacks do
 
   ## Helpers
 
+  @reserved ~w(case test line file capture_log skip timeout report async)a
+
   @doc false
   def __merge__(_mod, context, :ok) do
     {:ok, context}
@@ -163,12 +165,16 @@ defmodule ExUnit.Callbacks do
     raise_merge_failed!(mod, data)
   end
 
-  defp context_merge(_mod, context, %{} = data) do
-    Map.merge(context, data)
+  defp context_merge(mod, context, %{} = data) do
+    Map.merge(context, data, fn
+      _, v, v -> v
+      k, _, v when k in @reserved -> raise_merge_reserved!(mod, k, v)
+      _, _, v -> v
+    end)
   end
 
-  defp context_merge(_mod, context, data) when is_list(data) do
-    Enum.into(data, context)
+  defp context_merge(mod, context, data) when is_list(data) do
+    context_merge(mod, context, Map.new(data))
   end
 
   defp context_merge(mod, _context, data) do
@@ -177,7 +183,12 @@ defmodule ExUnit.Callbacks do
 
   defp raise_merge_failed!(mod, data) do
     raise "expected ExUnit callback in #{inspect mod} to return :ok " <>
-          " or {:ok, keyword | map}, got #{inspect data} instead"
+          "or {:ok, keyword | map}, got #{inspect data} instead"
+  end
+
+  defp raise_merge_reserved!(mod, key, value) do
+    raise "expected ExUnit callback in #{inspect mod} is trying to set " <>
+          "reserved field #{inspect key} to #{inspect value}"
   end
 
   defp escape(contents) do

lib/ex_unit/lib/ex_unit/case.ex

@@ -114,10 +114,11 @@ defmodule ExUnit.Case do
   The following tags are set automatically by ExUnit and are
   therefore reserved:
 
-    * `:case` - the test case module
-    * `:test` - the test name
-    * `:line` - the line on which the test was defined
-    * `:file` - the file on which the test was defined
+    * `:case`  - the test case module
+    * `:test`  - the test name
+    * `:line`  - the line on which the test was defined
+    * `:file`  - the file on which the test was defined
+    * `:async` - if the test case is in async mode
 
   The following tags customize how tests behaves:
 
@@ -174,7 +175,7 @@ defmodule ExUnit.Case do
 
   This default can be overriden by `@tag capture_log: false` or `@moduletag capture_log: false`.
 
-  Since `setup_all` blocks don't belong to a specific test, log messages generated in them (or 
+  Since `setup_all` blocks don't belong to a specific test, log messages generated in them (or
   between tests) are never captured. If you want to suppress these messages as well, remove the
   console backend globally:
 
@@ -193,20 +194,16 @@ defmodule ExUnit.Case do
     end
 
     quote do
+      async = !!unquote(async)
+
       unless Module.get_attribute(__MODULE__, :ex_unit_tests) do
         Enum.each [:ex_unit_tests, :tag, :moduletag],
           &Module.register_attribute(__MODULE__, &1, accumulate: true)
 
-        if unquote(async) do
-          @moduletag async: true
-          ExUnit.Server.add_async_case(__MODULE__)
-        else
-          @moduletag async: false
-          ExUnit.Server.add_sync_case(__MODULE__)
-        end
-
+        @moduletag async: async
         @before_compile ExUnit.Case
         @ex_unit_test_names %{}
+        @ex_unit_async async
         use ExUnit.Callbacks
       end
 
@@ -254,7 +251,7 @@ defmodule ExUnit.Case do
 
     quote bind_quoted: binding do
       test = :"test #{message}"
-      ExUnit.Case.__on_definition__(__ENV__, test)
+      ExUnit.Case.__on_definition__(__ENV__, test, [])
       def unquote(test)(unquote(var)), do: unquote(contents)
     end
   end
@@ -284,15 +281,20 @@ defmodule ExUnit.Case do
   @doc false
   defmacro __before_compile__(_) do
     quote do
+      if @ex_unit_async do
+        ExUnit.Server.add_async_case(__MODULE__)
+      else
+        ExUnit.Server.add_sync_case(__MODULE__)
+      end
+
       def __ex_unit__(:case) do
         %ExUnit.TestCase{name: __MODULE__, tests: @ex_unit_tests}
       end
     end
   end
 
   @doc false
-  def __on_definition__(env, name, tags \\ []) do
-    mod = env.module
+  def __on_definition__(%{module: mod, file: file, line: line}, name, tags) do
     moduletag = Module.get_attribute(mod, :moduletag)
 
     unless moduletag do
@@ -304,7 +306,7 @@ defmodule ExUnit.Case do
       (tags ++ Module.get_attribute(mod, :tag) ++ moduletag)
       |> normalize_tags
       |> validate_tags
-      |> Map.merge(%{line: env.line, file: env.file})
+      |> Map.merge(%{line: line, file: file})
 
     test = %ExUnit.Test{name: name, case: mod, tags: tags}
     test_names = Module.get_attribute(mod, :ex_unit_test_names)

lib/ex_unit/test/ex_unit_test.exs

@@ -152,7 +152,7 @@ defmodule ExUnitTest do
     refute output =~ "[debug] four"
   end
 
-  test "it supports multi errors" do
+  test "supports multi errors" do
     capture_io :stderr, fn ->
       defmodule MultiTest do
         use ExUnit.Case
@@ -187,7 +187,7 @@ defmodule ExUnitTest do
     assert output =~ "Failure #2"
   end
 
-  test "it registers only the first test with any given name" do
+  test "registers only the first test with any given name" do
     capture_io :stderr, fn ->
       defmodule TestWithSameNames do
         use ExUnit.Case
@@ -207,7 +207,7 @@ defmodule ExUnitTest do
     end) =~ "1 test, 0 failure"
   end
 
-  test "it produces error on not implemented tests" do
+  test "produces error on not implemented tests" do
     defmodule TestNotImplemented do
       use ExUnit.Case
 
@@ -227,7 +227,7 @@ defmodule ExUnitTest do
     assert output =~ "1 test, 1 failure"
   end
 
-  test "it skips tagged test with skip" do
+  test "skips tagged test with skip" do
     defmodule TestSkipped do
       use ExUnit.Case
 
@@ -250,6 +250,55 @@ defmodule ExUnitTest do
     assert output =~ "2 tests, 0 failures, 2 skipped"
   end
 
+  test "raises on reserved tag in module" do
+    assert_raise RuntimeError, "cannot set tag :file because it is reserved by ExUnit", fn ->
+      defmodule ReservedTag do
+        use ExUnit.Case
+
+        setup do
+          {:ok, file: :foo}
+        end
+
+        @tag file: "oops"
+        test "sample", do: :ok
+      end
+    end
+  end
+
+  test "raises on reserved tag in setup" do
+    defmodule ReservedSetupTag do
+      use ExUnit.Case
+
+      setup do
+        {:ok, file: :foo}
+      end
+
+      test "sample", do: :ok
+    end
+
+    output = capture_io(fn ->
+      assert ExUnit.run == %{failures: 1, skipped: 0, total: 1}
+    end)
+
+    assert output =~ "trying to set reserved field :file"
+  end
+
+  test "does not raise on reserved tag in setup_all (lower priority)" do
+    defmodule ReservedSetupAllTag do
+      use ExUnit.Case
+
+      setup_all do
+        {:ok, file: :foo}
+      end
+
+      test "sample", do: :ok
+    end
+
+    capture_io(fn ->
+      assert ExUnit.run == %{failures: 0, skipped: 0, total: 1}
+    end)
+  end
+
   defp run_with_filter(filters, {async, sync, load_us}) do
     opts = Keyword.merge(ExUnit.configuration, filters)
     output = capture_io fn ->