Mix.Tasks.Test: use refactored functions from ExUnit.Filters

lib/ex_unit/lib/ex_unit/filters.ex

@@ -14,27 +14,43 @@ defmodule ExUnit.Filters do
   on the command line) includes a line number filter, and if so returns the
   appropriate ExUnit configuration options.
   """
-  @spec parse_path(String.t()) :: {String.t(), Keyword.t()}
-  def parse_path(file) do
-    case extract_locations(file) do
-      {path, []} -> {path, []}
-      {path, locations} -> {path, exclude: [:test], include: locations}
+  @spec parse_path(String.t()) :: {String.t(), ex_unit_opts :: Keyword.t()}
+  def parse_path(file_path) do
+    {[parsed_path], ex_unit_opts} = parse_paths([file_path])
+    {parsed_path, ex_unit_opts}
+  end
+
+  @doc "Like `parse_path/1` but for multiple paths. ExUnit filter options are combined."
+  @spec parse_paths([String.t()]) :: {[String.t()], ex_unit_opts :: Keyword.t()}
+  def parse_paths(file_paths) do
+    Enum.reduce(file_paths, {[], []}, fn file_path, {parsed_paths, includes} ->
+      {parsed_path, lines} = extract_line_numbers(file_path)
+      includes = Enum.reduce(lines, includes, &[{:location, {parsed_path, &1}} | &2])
+      {[parsed_path | parsed_paths], includes}
+    end)
+    |> case do
+      {parsed_paths, []} ->
+        {Enum.reverse(parsed_paths), []}
+
+      {parsed_paths, includes} ->
+        {Enum.reverse(parsed_paths), exclude: [:test], include: Enum.reverse(includes)}
     end
   end
 
-  defp extract_locations(file) do
-    case String.split(file, ":") do
-      [part] ->
-        {part, []}
+  @spec extract_path(String.t()) :: String.t()
+  def extract_path(file_path), do: elem(extract_line_numbers(file_path), 0)
+
+  defp extract_line_numbers(file_path) do
+    case String.split(file_path, ":") do
+      [path] ->
+        {path, []}
 
       [path | parts] ->
         {path_parts, line_numbers} = Enum.split_while(parts, &(not valid_line_number?(&1)))
-
+        # Add back the parts that are not valid integer line numbers.
         path = Enum.join([path | path_parts], ":")
-
-        locations = for n <- line_numbers, valid_line_number(n), do: {:location, {path, n}}
-
-        {path, locations}
+        lines = Enum.filter(line_numbers, &valid_line_number/1)
+        {path, lines}
     end
   end
 
@@ -186,15 +202,14 @@ defmodule ExUnit.Filters do
     end
   end
 
-  defp has_tag(
-         {:location, {_file, line}},
-         %{line: _, describe_line: describe_line} = tags,
-         collection
-       ) do
+  defp has_tag({:location, {file, line}}, %{line: _, describe_line: _} = tags, collection) do
     line = to_integer(line)
 
     cond do
-      describe_line == line ->
+      file && not String.ends_with?(tags.file, file) ->
+        false
+
+      tags.describe_line == line ->
         true
 
       describe_block?(line, collection) ->
@@ -205,6 +220,10 @@ defmodule ExUnit.Filters do
     end
   end
 
+  defp has_tag({:line, line}, %{line: _, describe_line: _} = tags, collection) do
+    has_tag({:location, {nil, line}}, tags, collection)
+  end
+
   defp has_tag(pair, tags, _collection) do
     has_tag(pair, tags)
   end

lib/ex_unit/test/ex_unit/filters_test.exs

@@ -270,5 +270,25 @@ defmodule ExUnit.FiltersTest do
 
     assert output =~ "invalid line number given as ExUnit filter: 0"
     assert output =~ "invalid line number given as ExUnit filter: -789"
+
+    # Test multiple paths:
+
+    other_unix_path = "test/some/other_path.exs"
+
+    assert ExUnit.Filters.parse_paths(["#{unix_path}:123", "#{other_unix_path}:456"]) ==
+             {[unix_path, other_unix_path],
+              [
+                exclude: [:test],
+                include: [location: {unix_path, "123"}, location: {other_unix_path, "456"}]
+              ]}
+
+    other_windows_path = "C:\\some\\other_path.exs"
+
+    assert ExUnit.Filters.parse_paths(["#{windows_path}:123", "#{other_windows_path}:456"]) ==
+             {[windows_path, other_windows_path],
+              [
+                exclude: [:test],
+                include: [location: {windows_path, "123"}, location: {other_windows_path, "456"}]
+              ]}
   end
 end

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

@@ -475,10 +475,8 @@ defmodule Mix.Tasks.Test do
     end
   end
 
-  defp relative_app_file_exists?(file) do
-    {file, _} = ExUnit.Filters.parse_path(file)
-    File.exists?(Path.join("../..", file))
-  end
+  defp relative_app_file_exists?(file),
+    do: File.exists?(Path.join("../..", ExUnit.Filters.extract_path(file)))
 
   defp do_run(opts, args, files) do
     _ = Mix.Project.get!()
@@ -555,7 +553,7 @@ defmodule Mix.Tasks.Test do
 
     # Finally parse, require and load the files
     test_elixirc_options = project[:test_elixirc_options] || []
-    test_files = parse_files(files, shell, test_paths)
+    test_files = parse_file_paths(files, test_paths)
     test_pattern = project[:test_pattern] || "*_test.exs"
     warn_test_pattern = project[:warn_test_pattern] || "*_test.ex"
 
@@ -692,30 +690,19 @@ defmodule Mix.Tasks.Test do
     [autorun: false] ++ opts
   end
 
-  defp parse_files([], _shell, test_paths) do
+  defp parse_file_paths([], test_paths) do
     test_paths
   end
 
-  defp parse_files([single_file], _shell, _test_paths) do
-    # Check if the single file path matches test/path/to_test.exs:123. If it does,
-    # apply "--only line:123" and trim the trailing :123 part.
-    {single_file, opts} = ExUnit.Filters.parse_path(single_file)
-    ExUnit.configure(opts)
-    [single_file]
-  end
-
-  defp parse_files(files, shell, _test_paths) do
-    if Enum.any?(files, &match?({_, [_ | _]}, ExUnit.Filters.parse_path(&1))) do
-      raise_with_shell(shell, "Line numbers can only be used when running a single test file")
-    else
-      files
-    end
+  defp parse_file_paths(file_paths, _test_paths) do
+    {parsed_file_paths, ex_unit_opts} = ExUnit.Filters.parse_paths(file_paths)
+    ExUnit.configure(ex_unit_opts)
+    parsed_file_paths
   end
 
   defp parse_filters(opts, key) do
-    if Keyword.has_key?(opts, key) do
-      ExUnit.Filters.parse(Keyword.get_values(opts, key))
-    end
+    values = Keyword.get_values(opts, key)
+    if values != [], do: ExUnit.Filters.parse(values)
   end
 
   defp filter_opts(opts, :only) do

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

@@ -438,14 +438,14 @@ defmodule Mix.Tasks.TestTest do
       end)
     end
 
-    test "raises an exception if line numbers are given with multiple files" do
+    test "runs multiple test files if line numbers are given" do
       in_fixture("test_stale", fn ->
         assert_run_output(
-          [
-            "test/a_test_stale.exs",
-            "test/b_test_stale.exs:4"
-          ],
-          "Line numbers can only be used when running a single test file"
+          ["test/a_test_stale.exs:2", "test/b_test_stale.exs:4"],
+          """
+          Excluding tags: [:test]
+          Including tags: [location: {"test/a_test_stale.exs", "2"}, location: {"test/b_test_stale.exs", "4"}]
+          """
         )
       end)
     end
@@ -508,6 +508,18 @@ defmodule Mix.Tasks.TestTest do
 
         refute output =~ "==> foo"
         refute output =~ "Paths given to \"mix test\" did not match any directory/file"
+
+        output = mix(["test", "apps/foo/test/foo_tests.exs:9", "apps/bar/test/bar_tests.exs:5"])
+
+        assert output =~ """
+               Excluding tags: [:test]
+               Including tags: [location: {"test/foo_tests.exs", "9"}]
+               """
+
+        assert output =~ """
+               Excluding tags: [:test]
+               Including tags: [location: {"test/bar_tests.exs", "5"}]
+               """
       end)
     end
   end