From a4cb71c0cd4fe58f2ff0ca67254f1cc2ac88096f Mon Sep 17 00:00:00 2001 From: Steffen Deusch Date: Sun, 29 Dec 2024 13:27:15 +0100 Subject: [PATCH] Add test_load_filters and test_warn_filters to mix test (#14036) --- lib/mix/lib/mix/tasks/test.ex | 145 ++++++++++++++++-- lib/mix/test/fixtures/test_failed/mix.exs | 2 +- lib/mix/test/fixtures/test_stale/mix.exs | 2 +- .../fixtures/umbrella_test/apps/bar/mix.exs | 2 +- .../fixtures/umbrella_test/apps/foo/mix.exs | 2 +- lib/mix/test/mix/tasks/test_test.exs | 110 +++++++++++++ 6 files changed, 244 insertions(+), 19 deletions(-) diff --git a/lib/mix/lib/mix/tasks/test.ex b/lib/mix/lib/mix/tasks/test.ex index ec3a0b60c5f..167ca3ee111 100644 --- a/lib/mix/lib/mix/tasks/test.ex +++ b/lib/mix/lib/mix/tasks/test.ex @@ -228,10 +228,30 @@ defmodule Mix.Tasks.Test do `["test"]` if the `test` directory exists, otherwise, it defaults to `[]`. It is expected that all test paths contain a `test_helper.exs` file - * `:test_pattern` - a pattern to load test files. Defaults to `*_test.exs` + * `:test_pattern` - a pattern to find potential test files. + Defaults to `*.{ex,exs}`. - * `:warn_test_pattern` - a pattern to match potentially misnamed test files - and display a warning. Defaults to `*_test.ex` + In Elixir versions earlier than 1.19.0, this option defaulted to `*_test.exs`, + but to allow better warnings for misnamed test files, it since matches any + Elixir file and expects those to be filtered by `:test_load_filters` and + `:test_ignore_filters`. + + * `:test_load_filters` - a list of files, regular expressions or one-arity + functions to restrict which files matched by the `:test_pattern` are loaded. + Defaults to `[&String.ends_with?(&1, "_test.exs")]`. Paths are relative to + the project root and separated by `/`, even on Windows. + + * `:test_ignore_filters` - a list of files, regular expressions or one-arity + functions to restrict which files matched by the `:test_pattern`, but not loaded + by `:test_load_filters`, trigger a warning for a potentially misnamed test file. + + Mix ignores files ending in `_helper.exs` by default, as well as any file + included in the project's `:elixirc_paths`. This ensures that any helper + or test support files are not triggering a warning. + + Any extra filters configured in the project are appended to the defaults. + Warnings can be disabled by setting this option to `[fn _ -> true end]`. + Paths are relative to the project root and separated by `/`, even on Windows. ## Coloring @@ -595,20 +615,38 @@ defmodule Mix.Tasks.Test do # Prepare and extract all files to require and run test_paths = project[:test_paths] || default_test_paths() - test_pattern = project[:test_pattern] || "*_test.exs" - warn_test_pattern = project[:warn_test_pattern] || "*_test.ex" + test_pattern = project[:test_pattern] || "*.{ex,exs}" + + # Warn about deprecated warn configuration + if project[:warn_test_pattern] do + Mix.shell().info(""" + warning: the `:warn_test_pattern` configuration is deprecated and will be ignored. \ + Use `:test_load_filters` and `:test_ignore_filters` instead. + """) + end {test_files, test_opts} = if files != [], do: ExUnit.Filters.parse_paths(files), else: {test_paths, []} - unfiltered_test_files = Mix.Utils.extract_files(test_files, test_pattern) + # get a list of all files in the test folders, which we filter by the test_load_filters + {potential_test_files, directly_included_test_files} = extract_files(test_files, test_pattern) + + {load_files, _ignored_files, warn_files} = + classify_test_files(potential_test_files, project) + + # ensure that files given as direct argument to mix test are loaded, + # even if the test_load_filters don't match + load_files = + if files != [], + do: Enum.uniq(load_files ++ directly_included_test_files), + else: load_files matched_test_files = - unfiltered_test_files + load_files |> filter_to_allowed_files(allowed_files) |> filter_by_partition(shell, partitions) - display_warn_test_pattern(test_files, test_pattern, unfiltered_test_files, warn_test_pattern) + warn_files != [] && warn_misnamed_test_files(warn_files) try do Enum.each(test_paths, &require_test_helper(shell, &1)) @@ -660,6 +698,33 @@ defmodule Mix.Tasks.Test do end end + # similar to Mix.Utils.extract_files/2, but returns a list of directly included test files, + # that should be not filtered by the test_load_filters, e.g. + # mix test test/some_file.exs + defp extract_files(paths, pattern) do + {files, directly_included} = + for path <- paths, reduce: {[], []} do + {acc, directly_included} -> + case :elixir_utils.read_file_type(path) do + {:ok, :directory} -> + {[Path.wildcard("#{path}/**/#{pattern}") | acc], directly_included} + + {:ok, :regular} -> + {[path | acc], [path | directly_included]} + + _ -> + {acc, directly_included} + end + end + + files = + files + |> List.flatten() + |> Enum.uniq() + + {files, directly_included} + end + defp raise_with_shell(shell, message) do Mix.shell(shell) Mix.raise(message) @@ -679,14 +744,64 @@ defmodule Mix.Tasks.Test do end end - defp display_warn_test_pattern(test_files, test_pattern, matched_test_files, warn_test_pattern) do - files = Mix.Utils.extract_files(test_files, warn_test_pattern) -- matched_test_files + defp classify_test_files(potential_test_files, project) do + test_load_filters = project[:test_load_filters] || [&String.ends_with?(&1, "_test.exs")] + elixirc_paths = project[:elixirc_paths] || [] + + # ignore any _helper.exs files and files that are compiled (test support files) + test_ignore_filters = + [ + &String.ends_with?(&1, "_helper.exs"), + fn file -> Enum.any?(elixirc_paths, &String.starts_with?(file, &1)) end + ] ++ Keyword.get(project, :test_ignore_filters, []) + + {to_load, to_ignore, to_warn} = + for file <- potential_test_files, reduce: {[], [], []} do + {to_load, to_ignore, to_warn} -> + cond do + any_file_matches?(file, test_load_filters) -> + {[file | to_load], to_ignore, to_warn} + + any_file_matches?(file, test_ignore_filters) -> + {to_load, [file | to_ignore], to_warn} + + true -> + {to_load, to_ignore, [file | to_warn]} + end + end - for file <- files do - Mix.shell().info( - "warning: #{file} does not match #{inspect(test_pattern)} and won't be loaded" - ) - end + # get the files back in the original order + {Enum.reverse(to_load), Enum.reverse(to_ignore), Enum.reverse(to_warn)} + end + + defp any_file_matches?(file, filters) do + Enum.any?(filters, fn filter -> + case filter do + regex when is_struct(regex, Regex) -> + Regex.match?(regex, file) + + binary when is_binary(binary) -> + file == binary + + fun when is_function(fun, 1) -> + fun.(file) + end + end) + end + + defp warn_misnamed_test_files(ignored) do + Mix.shell().info(""" + warning: the following files do not match any of the configured `:test_load_filters` / `:test_ignore_filters`: + + #{Enum.join(ignored, "\n")} + + This might indicate a typo in a test file name (for example, using "foo_tests.exs" instead of "foo_test.exs"). + + You can adjust which files trigger this warning by configuring the `:test_ignore_filters` option in your + Mix project's configuration. To disable the warning entirely, set that option to false. + + For more information, run `mix help test`. + """) end @option_keys [ diff --git a/lib/mix/test/fixtures/test_failed/mix.exs b/lib/mix/test/fixtures/test_failed/mix.exs index 26e152b8d86..ef7cc5749e9 100644 --- a/lib/mix/test/fixtures/test_failed/mix.exs +++ b/lib/mix/test/fixtures/test_failed/mix.exs @@ -5,7 +5,7 @@ defmodule TestFailed.MixProject do [ app: :test_only_failures, version: "0.0.1", - test_pattern: "*_test_failed.exs" + test_load_filters: [~r/.*_test_failed\.exs/] ] end end diff --git a/lib/mix/test/fixtures/test_stale/mix.exs b/lib/mix/test/fixtures/test_stale/mix.exs index 2300c8b58fe..b5be916638f 100644 --- a/lib/mix/test/fixtures/test_stale/mix.exs +++ b/lib/mix/test/fixtures/test_stale/mix.exs @@ -5,7 +5,7 @@ defmodule TestStale.MixProject do [ app: :test_stale, version: "0.0.1", - test_pattern: "*_test_stale.exs" + test_load_filters: [fn file -> String.ends_with?(file, "_test_stale.exs") end] ] end end diff --git a/lib/mix/test/fixtures/umbrella_test/apps/bar/mix.exs b/lib/mix/test/fixtures/umbrella_test/apps/bar/mix.exs index f12eed69de9..64d5d9fe4a4 100644 --- a/lib/mix/test/fixtures/umbrella_test/apps/bar/mix.exs +++ b/lib/mix/test/fixtures/umbrella_test/apps/bar/mix.exs @@ -7,7 +7,7 @@ defmodule Bar.MixProject do version: "0.1.0", # Choose something besides *_test.exs so that these test files don't # get accidentally swept up into the actual Mix test suite. - test_pattern: "*_tests.exs", + test_load_filters: [~r/.*_tests\.exs/], test_coverage: [ignore_modules: [Bar, ~r/Ignore/]], aliases: [mytask: fn _ -> Mix.shell().info("bar_running") end] ] diff --git a/lib/mix/test/fixtures/umbrella_test/apps/foo/mix.exs b/lib/mix/test/fixtures/umbrella_test/apps/foo/mix.exs index 4c4e8358212..4e02cafe1d6 100644 --- a/lib/mix/test/fixtures/umbrella_test/apps/foo/mix.exs +++ b/lib/mix/test/fixtures/umbrella_test/apps/foo/mix.exs @@ -7,7 +7,7 @@ defmodule Foo.MixProject do version: "0.1.0", # Choose something besides *_test.exs so that these test files don't # get accidentally swept up into the actual Mix test suite. - test_pattern: "*_tests.exs", + test_load_filters: [~r/.*_tests\.exs/], aliases: [mytask: fn _ -> Mix.shell().info("foo_running") end] ] end diff --git a/lib/mix/test/mix/tasks/test_test.exs b/lib/mix/test/mix/tasks/test_test.exs index d3d73af2c05..35acb7e6c7f 100644 --- a/lib/mix/test/mix/tasks/test_test.exs +++ b/lib/mix/test/mix/tasks/test_test.exs @@ -624,6 +624,116 @@ defmodule Mix.Tasks.TestTest do end end + describe "test_load_filters and test_ignore_filters" do + test "warns for files that are not loaded and don't match test_ignore_filters" do + in_tmp("test_warn", fn -> + File.write!("mix.exs", """ + defmodule TestWarn.MixProject do + use Mix.Project + + def project do + [ + app: :test_warn, + version: "0.0.1", + test_load_filters: [~r/.*_tests\.exs/], + test_ignore_filters: [ + "test/test_helper.exs", + ~r/ignored_regex/, + fn file -> file == "test/ignored_file.exs" end + ] + ] + end + end + """) + + File.mkdir!("test") + + File.write!("test/a_tests.exs", """ + defmodule ATests do + use ExUnit.Case + + test "dummy" do + assert true + end + end + """) + + File.write!("test/test_helper.exs", "ExUnit.start()") + File.touch("test/a_missing.exs") + File.touch("test/a_tests.ex") + File.touch("test/ignored_file.exs") + File.touch("test/ignored_regex.exs") + File.write!("test/other_file.txt", "this is not a test file") + + output = mix(["test"]) + + # This test relies on the files present in the test_warn fixture. + # + # We test that we don't warn about a_tests.exs, as it already matches the load pattern. + # Similarly, we ignore the empty but present ignored_file.exs and ignored_regex.exs. + # other_file.txt does not match the test_pattern and is ignored from the beginning. + # + # Therefore, we expect to get a warning for a_missing.exs and a_tests.ex. + assert output =~ """ + the following files do not match any of the configured `:test_load_filters` / `:test_ignore_filters`: + + test/a_missing.exs + test/a_tests.ex + + This might indicate a typo\ + """ + + # the dummy test ran successfully + assert output =~ "1 test, 0 failures" + end) + end + + test "does not warn when test_ignore_filters are disabled" do + in_tmp("test_warn", fn -> + File.write!("mix.exs", """ + defmodule TestWarn.MixProject do + use Mix.Project + + def project do + [ + app: :test_warn, + version: "0.0.1", + test_load_filters: [~r/.*_tests\.exs/], + test_ignore_filters: [fn _ -> true end] + ] + end + end + """) + + File.mkdir!("test") + + File.write!("test/a_tests.exs", """ + defmodule ATests do + use ExUnit.Case + + test "dummy" do + assert true + end + end + """) + + File.write!("test/test_helper.exs", "ExUnit.start()") + File.touch("test/a_missing.exs") + File.touch("test/a_tests.ex") + File.touch("test/ignored_file.exs") + File.touch("test/ignored_regex.exs") + File.write!("test/other_file.txt", "this is not a test file") + + output = mix(["test"]) + + refute output =~ "the following files do not match" + + # the dummy test ran successfully + assert output =~ "1 test, 0 failures" + end) + end + end + defp receive_until_match(port, expected, acc) do receive do {^port, {:data, output}} ->