From 2069f8d23fb3a5dae9ff7e4f4a38fe03382eeb96 Mon Sep 17 00:00:00 2001 From: Steffen Deusch Date: Thu, 5 Dec 2024 18:33:37 +0100 Subject: [PATCH 1/9] (mix test) add test_load_filters and test_warn_filters When using the default project configuration, mix test would only warn about files ending in `_test.ex`. The only mistake this warns about is forgetting the `s` of `.exs`. In a work project we noticed (after more than a year) that we had multiple test files that did not match the test pattern, preventing them from running in mix test locally and CI. Because we have many other tests, nobody noticed this. If CI passes, all is good, right? Because there is no easy way to evaluate glob patterns in Elixir without touching the filesystem, I decided to deprecate the old `warn_test_pattern` configuration and instead add two new configurations: `test_load_filters` and `test_warn_filters` Now, by changing the default of `test_pattern` to `*.{ex,exs}`, we can load all potential test files once and then match their paths to the patterns. The `test_load_filters` is used to filter the files that are loaded by mix test. This defaults to the regex equivalent of "*_test.exs". The `test_warn_filters` is used to filter the files that we warn about if they are not loaded. By default, we ignore any file ending in `_helper.exs`, which will prevent the default test_helper.exs from generating a warning and also provides a simple way to name other files that might be required explicitly in tests. We also default to ignore any files that start with a configured elixirc_path, which are compiled often test support files. For projects with an existing `warn_test_pattern` configuration, a deprecation warning is logged. The warnings can be disabled by setting `test_warn_filters` to a falsy value. Projects with an existing custom `test_pattern` should check if their pattern conflicts with the new `test_load_filters` and adjust their configuration accordingly. It is also possible to keep the old `test_pattern` and configure the `test_load_filters` to accept any file, for example by configuring it to `[fn _ -> true end]`. In that case, the `test_warn_filters` don't have an effect, as any potential test file is also loaded. --- lib/mix/lib/mix/tasks/test.ex | 115 +++++++++++++++--- lib/mix/test/fixtures/test_failed/mix.exs | 2 +- lib/mix/test/fixtures/test_stale/mix.exs | 2 +- lib/mix/test/fixtures/test_warn/mix.exs | 16 +++ .../fixtures/test_warn/test/a_missing.exs | 0 .../test/fixtures/test_warn/test/a_tests.ex | 0 .../test/fixtures/test_warn/test/a_tests.exs | 7 ++ .../fixtures/test_warn/test/ignored_file.exs | 0 .../fixtures/test_warn/test/ignored_regex.exs | 0 .../fixtures/test_warn/test/other_file.txt | 0 .../fixtures/test_warn/test/test_helper.exs | 1 + .../fixtures/umbrella_test/apps/bar/mix.exs | 2 +- .../fixtures/umbrella_test/apps/foo/mix.exs | 2 +- lib/mix/test/mix/tasks/test_test.exs | 27 ++++ 14 files changed, 156 insertions(+), 18 deletions(-) create mode 100644 lib/mix/test/fixtures/test_warn/mix.exs create mode 100644 lib/mix/test/fixtures/test_warn/test/a_missing.exs create mode 100644 lib/mix/test/fixtures/test_warn/test/a_tests.ex create mode 100644 lib/mix/test/fixtures/test_warn/test/a_tests.exs create mode 100644 lib/mix/test/fixtures/test_warn/test/ignored_file.exs create mode 100644 lib/mix/test/fixtures/test_warn/test/ignored_regex.exs create mode 100644 lib/mix/test/fixtures/test_warn/test/other_file.txt create mode 100644 lib/mix/test/fixtures/test_warn/test/test_helper.exs diff --git a/lib/mix/lib/mix/tasks/test.ex b/lib/mix/lib/mix/tasks/test.ex index ec3a0b60c5f..e38d615cce1 100644 --- a/lib/mix/lib/mix/tasks/test.ex +++ b/lib/mix/lib/mix/tasks/test.ex @@ -228,10 +228,33 @@ 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_warn_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 `[~r/.*_test\\.exs$/]` + + * `:test_warn_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. + + Defaults to: + + ```elixir + [ + ~r/.*_helper\.exs$/, + fn file -> Enum.any?(elixirc_paths(Mix.env()), &String.starts_with?(file, &1)) end + ] + ``` + + This ensures that any helper or test support files are not triggering a warning. + Warnings can be disabled by setting this option to `false` or `nil`. ## Coloring @@ -595,20 +618,31 @@ 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_warn_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 = Mix.Utils.extract_files(test_files, test_pattern) + + {unfiltered_test_files, _ignored_files, warn_files} = + classify_test_files(potential_test_files, project) matched_test_files = unfiltered_test_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)) @@ -679,14 +713,67 @@ 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] || [~r/.*_test\.exs$/] + elixirc_paths = project[:elixirc_paths] || [] - for file <- files do - Mix.shell().info( - "warning: #{file} does not match #{inspect(test_pattern)} and won't be loaded" - ) - end + # ignore any _helper.exs files and files that are compiled (test support files) + test_warn_filters = + Keyword.get_lazy(project, :test_warn_filters, fn -> + [ + ~r/.*_helper\.exs$/, + fn file -> Enum.any?(elixirc_paths, &String.starts_with?(file, &1)) end + ] + end) + + {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_warn_filters) -> + {to_load, [file | to_ignore], to_warn} + + # don't warn if test_warn_filters is explicitly set to nil / false + !!test_warn_filters -> + {to_load, to_ignore, [file | to_warn]} + end + 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_warn_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_warn_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/test_warn/mix.exs b/lib/mix/test/fixtures/test_warn/mix.exs new file mode 100644 index 00000000000..3932943b776 --- /dev/null +++ b/lib/mix/test/fixtures/test_warn/mix.exs @@ -0,0 +1,16 @@ +defmodule TestWarn.MixProject do + use Mix.Project + + def project do + [ + app: :test_warn, + version: "0.0.1", + test_load_filters: [~r/.*_tests\.exs/], + test_warn_filters: [ + "test/test_helper.exs", + ~r/ignored_regex/, + fn file -> file == "test/ignored_file.exs" end + ] + ] + end +end diff --git a/lib/mix/test/fixtures/test_warn/test/a_missing.exs b/lib/mix/test/fixtures/test_warn/test/a_missing.exs new file mode 100644 index 00000000000..e69de29bb2d diff --git a/lib/mix/test/fixtures/test_warn/test/a_tests.ex b/lib/mix/test/fixtures/test_warn/test/a_tests.ex new file mode 100644 index 00000000000..e69de29bb2d diff --git a/lib/mix/test/fixtures/test_warn/test/a_tests.exs b/lib/mix/test/fixtures/test_warn/test/a_tests.exs new file mode 100644 index 00000000000..06ebaca48b6 --- /dev/null +++ b/lib/mix/test/fixtures/test_warn/test/a_tests.exs @@ -0,0 +1,7 @@ +defmodule ATests do + use ExUnit.Case + + test "dummy" do + assert true + end +end diff --git a/lib/mix/test/fixtures/test_warn/test/ignored_file.exs b/lib/mix/test/fixtures/test_warn/test/ignored_file.exs new file mode 100644 index 00000000000..e69de29bb2d diff --git a/lib/mix/test/fixtures/test_warn/test/ignored_regex.exs b/lib/mix/test/fixtures/test_warn/test/ignored_regex.exs new file mode 100644 index 00000000000..e69de29bb2d diff --git a/lib/mix/test/fixtures/test_warn/test/other_file.txt b/lib/mix/test/fixtures/test_warn/test/other_file.txt new file mode 100644 index 00000000000..e69de29bb2d diff --git a/lib/mix/test/fixtures/test_warn/test/test_helper.exs b/lib/mix/test/fixtures/test_warn/test/test_helper.exs new file mode 100644 index 00000000000..869559e709e --- /dev/null +++ b/lib/mix/test/fixtures/test_warn/test/test_helper.exs @@ -0,0 +1 @@ +ExUnit.start() 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..038927c4389 100644 --- a/lib/mix/test/mix/tasks/test_test.exs +++ b/lib/mix/test/mix/tasks/test_test.exs @@ -624,6 +624,33 @@ defmodule Mix.Tasks.TestTest do end end + describe "test_load_filters and test_warn_filters" do + test "warns for files that are not loaded and don't match test_warn_filters" do + in_fixture("test_warn", fn -> + 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_warn_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 + end + defp receive_until_match(port, expected, acc) do receive do {^port, {:data, output}} -> From c8825849471c891d8f322718a360c77a396c4872 Mon Sep 17 00:00:00 2001 From: Steffen Deusch Date: Fri, 13 Dec 2024 17:40:15 +0100 Subject: [PATCH 2/9] always include files directly passed to mix test --- lib/mix/lib/mix/tasks/test.ex | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/lib/mix/lib/mix/tasks/test.ex b/lib/mix/lib/mix/tasks/test.ex index e38d615cce1..b39e6f676f8 100644 --- a/lib/mix/lib/mix/tasks/test.ex +++ b/lib/mix/lib/mix/tasks/test.ex @@ -634,11 +634,18 @@ defmodule Mix.Tasks.Test do # get a list of all files in the test folders, which we filter by the test_load_filters potential_test_files = Mix.Utils.extract_files(test_files, test_pattern) - {unfiltered_test_files, _ignored_files, warn_files} = + {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(files)), + else: load_files + matched_test_files = - unfiltered_test_files + load_files |> filter_to_allowed_files(allowed_files) |> filter_by_partition(shell, partitions) @@ -713,6 +720,10 @@ defmodule Mix.Tasks.Test do end end + defp directly_included_test_files(files) do + Enum.filter(files, fn path -> :elixir_utils.read_file_type(path) == {:ok, :regular} end) + end + defp classify_test_files(potential_test_files, project) do test_load_filters = project[:test_load_filters] || [~r/.*_test\.exs$/] elixirc_paths = project[:elixirc_paths] || [] From e6b21b59c42695de515ae04606b4e32ae4aa1dfa Mon Sep 17 00:00:00 2001 From: Steffen Deusch Date: Fri, 27 Dec 2024 15:29:17 +0100 Subject: [PATCH 3/9] implement suggestions from code review --- lib/mix/lib/mix/tasks/test.ex | 65 +++++++++++++------ lib/mix/test/fixtures/test_warn/mix.exs | 16 ----- .../fixtures/test_warn/test/a_missing.exs | 0 .../test/fixtures/test_warn/test/a_tests.ex | 0 .../test/fixtures/test_warn/test/a_tests.exs | 7 -- .../fixtures/test_warn/test/ignored_file.exs | 0 .../fixtures/test_warn/test/ignored_regex.exs | 0 .../fixtures/test_warn/test/other_file.txt | 0 .../fixtures/test_warn/test/test_helper.exs | 1 - lib/mix/test/mix/tasks/test_test.exs | 46 +++++++++++-- 10 files changed, 87 insertions(+), 48 deletions(-) delete mode 100644 lib/mix/test/fixtures/test_warn/mix.exs delete mode 100644 lib/mix/test/fixtures/test_warn/test/a_missing.exs delete mode 100644 lib/mix/test/fixtures/test_warn/test/a_tests.ex delete mode 100644 lib/mix/test/fixtures/test_warn/test/a_tests.exs delete mode 100644 lib/mix/test/fixtures/test_warn/test/ignored_file.exs delete mode 100644 lib/mix/test/fixtures/test_warn/test/ignored_regex.exs delete mode 100644 lib/mix/test/fixtures/test_warn/test/other_file.txt delete mode 100644 lib/mix/test/fixtures/test_warn/test/test_helper.exs diff --git a/lib/mix/lib/mix/tasks/test.ex b/lib/mix/lib/mix/tasks/test.ex index b39e6f676f8..2215409fa22 100644 --- a/lib/mix/lib/mix/tasks/test.ex +++ b/lib/mix/lib/mix/tasks/test.ex @@ -234,13 +234,14 @@ defmodule Mix.Tasks.Test do 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_warn_filters`. + `: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 `[~r/.*_test\\.exs$/]` + Defaults to `[&String.ends_with?(&1, "_test.exs")]`. Paths are relative to + the project root and separated by `/`, even on Windows. - * `:test_warn_filters` - a list of files, regular expressions or one-arity + * `: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. @@ -248,13 +249,14 @@ defmodule Mix.Tasks.Test do ```elixir [ - ~r/.*_helper\.exs$/, + &String.ends_with?(&1, "_helper.exs"), fn file -> Enum.any?(elixirc_paths(Mix.env()), &String.starts_with?(file, &1)) end ] ``` This ensures that any helper or test support files are not triggering a warning. Warnings can be disabled by setting this option to `false` or `nil`. + Paths are relative to the project root and separated by `/`, even on Windows. ## Coloring @@ -624,7 +626,7 @@ defmodule Mix.Tasks.Test do 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_warn_filters` instead. + Use `:test_load_filters` and `:test_ignore_filters` instead. """) end @@ -632,7 +634,7 @@ defmodule Mix.Tasks.Test do if files != [], do: ExUnit.Filters.parse_paths(files), else: {test_paths, []} # get a list of all files in the test folders, which we filter by the test_load_filters - potential_test_files = Mix.Utils.extract_files(test_files, test_pattern) + {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) @@ -641,7 +643,7 @@ defmodule Mix.Tasks.Test do # even if the test_load_filters don't match load_files = if files != [], - do: Enum.uniq(load_files ++ directly_included_test_files(files)), + do: Enum.uniq(load_files ++ directly_included_test_files), else: load_files matched_test_files = @@ -701,6 +703,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} -> + {[acc, Path.wildcard("#{path}/**/#{pattern}")], directly_included} + + {:ok, :regular} -> + {[acc, path], [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) @@ -720,19 +749,15 @@ defmodule Mix.Tasks.Test do end end - defp directly_included_test_files(files) do - Enum.filter(files, fn path -> :elixir_utils.read_file_type(path) == {:ok, :regular} end) - end - defp classify_test_files(potential_test_files, project) do - test_load_filters = project[:test_load_filters] || [~r/.*_test\.exs$/] + test_load_filters = project[:test_load_filters] || [&String.starts_with?(&1, "_test.exs")] elixirc_paths = project[:elixirc_paths] || [] # ignore any _helper.exs files and files that are compiled (test support files) - test_warn_filters = - Keyword.get_lazy(project, :test_warn_filters, fn -> + test_ignore_filters = + Keyword.get_lazy(project, :test_ignore_filters, fn -> [ - ~r/.*_helper\.exs$/, + &String.ends_with?(&1, "_helper.exs"), fn file -> Enum.any?(elixirc_paths, &String.starts_with?(file, &1)) end ] end) @@ -744,11 +769,11 @@ defmodule Mix.Tasks.Test do any_file_matches?(file, test_load_filters) -> {[file | to_load], to_ignore, to_warn} - any_file_matches?(file, test_warn_filters) -> + any_file_matches?(file, test_ignore_filters) -> {to_load, [file | to_ignore], to_warn} - # don't warn if test_warn_filters is explicitly set to nil / false - !!test_warn_filters -> + # don't warn if test_ignore_filters is explicitly set to nil / false + !!test_ignore_filters -> {to_load, to_ignore, [file | to_warn]} end end @@ -774,13 +799,13 @@ defmodule Mix.Tasks.Test do 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_warn_filters`: + 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_warn_filters` option in your + 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`. diff --git a/lib/mix/test/fixtures/test_warn/mix.exs b/lib/mix/test/fixtures/test_warn/mix.exs deleted file mode 100644 index 3932943b776..00000000000 --- a/lib/mix/test/fixtures/test_warn/mix.exs +++ /dev/null @@ -1,16 +0,0 @@ -defmodule TestWarn.MixProject do - use Mix.Project - - def project do - [ - app: :test_warn, - version: "0.0.1", - test_load_filters: [~r/.*_tests\.exs/], - test_warn_filters: [ - "test/test_helper.exs", - ~r/ignored_regex/, - fn file -> file == "test/ignored_file.exs" end - ] - ] - end -end diff --git a/lib/mix/test/fixtures/test_warn/test/a_missing.exs b/lib/mix/test/fixtures/test_warn/test/a_missing.exs deleted file mode 100644 index e69de29bb2d..00000000000 diff --git a/lib/mix/test/fixtures/test_warn/test/a_tests.ex b/lib/mix/test/fixtures/test_warn/test/a_tests.ex deleted file mode 100644 index e69de29bb2d..00000000000 diff --git a/lib/mix/test/fixtures/test_warn/test/a_tests.exs b/lib/mix/test/fixtures/test_warn/test/a_tests.exs deleted file mode 100644 index 06ebaca48b6..00000000000 --- a/lib/mix/test/fixtures/test_warn/test/a_tests.exs +++ /dev/null @@ -1,7 +0,0 @@ -defmodule ATests do - use ExUnit.Case - - test "dummy" do - assert true - end -end diff --git a/lib/mix/test/fixtures/test_warn/test/ignored_file.exs b/lib/mix/test/fixtures/test_warn/test/ignored_file.exs deleted file mode 100644 index e69de29bb2d..00000000000 diff --git a/lib/mix/test/fixtures/test_warn/test/ignored_regex.exs b/lib/mix/test/fixtures/test_warn/test/ignored_regex.exs deleted file mode 100644 index e69de29bb2d..00000000000 diff --git a/lib/mix/test/fixtures/test_warn/test/other_file.txt b/lib/mix/test/fixtures/test_warn/test/other_file.txt deleted file mode 100644 index e69de29bb2d..00000000000 diff --git a/lib/mix/test/fixtures/test_warn/test/test_helper.exs b/lib/mix/test/fixtures/test_warn/test/test_helper.exs deleted file mode 100644 index 869559e709e..00000000000 --- a/lib/mix/test/fixtures/test_warn/test/test_helper.exs +++ /dev/null @@ -1 +0,0 @@ -ExUnit.start() diff --git a/lib/mix/test/mix/tasks/test_test.exs b/lib/mix/test/mix/tasks/test_test.exs index 038927c4389..a76eb818dd3 100644 --- a/lib/mix/test/mix/tasks/test_test.exs +++ b/lib/mix/test/mix/tasks/test_test.exs @@ -624,9 +624,47 @@ defmodule Mix.Tasks.TestTest do end end - describe "test_load_filters and test_warn_filters" do - test "warns for files that are not loaded and don't match test_warn_filters" do - in_fixture("test_warn", fn -> + 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_tests.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. @@ -637,7 +675,7 @@ defmodule Mix.Tasks.TestTest do # # 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_warn_filters`: + the following files do not match any of the configured `:test_load_filters` / `:test_ignore_filters`: test/a_missing.exs test/a_tests.ex From 3efedc9cbc437ec590320b2fdad1b24b1be5673f Mon Sep 17 00:00:00 2001 From: Steffen Deusch Date: Fri, 27 Dec 2024 15:41:03 +0100 Subject: [PATCH 4/9] fix classify_test_files not properly handling disabled test_ignore_filters --- lib/mix/lib/mix/tasks/test.ex | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/lib/mix/lib/mix/tasks/test.ex b/lib/mix/lib/mix/tasks/test.ex index 2215409fa22..b1447f9f57e 100644 --- a/lib/mix/lib/mix/tasks/test.ex +++ b/lib/mix/lib/mix/tasks/test.ex @@ -750,7 +750,7 @@ defmodule Mix.Tasks.Test do end defp classify_test_files(potential_test_files, project) do - test_load_filters = project[:test_load_filters] || [&String.starts_with?(&1, "_test.exs")] + 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) @@ -769,11 +769,10 @@ defmodule Mix.Tasks.Test do any_file_matches?(file, test_load_filters) -> {[file | to_load], to_ignore, to_warn} - any_file_matches?(file, test_ignore_filters) -> + !test_ignore_filters || any_file_matches?(file, test_ignore_filters) -> {to_load, [file | to_ignore], to_warn} - # don't warn if test_ignore_filters is explicitly set to nil / false - !!test_ignore_filters -> + true -> {to_load, to_ignore, [file | to_warn]} end end From 8081e6a588725af81cc063aebacc36618c182d37 Mon Sep 17 00:00:00 2001 From: Steffen Deusch Date: Fri, 27 Dec 2024 15:44:08 +0100 Subject: [PATCH 5/9] add test for disabled test_ignore_filters --- lib/mix/test/mix/tasks/test_test.exs | 45 ++++++++++++++++++++++++++++ 1 file changed, 45 insertions(+) diff --git a/lib/mix/test/mix/tasks/test_test.exs b/lib/mix/test/mix/tasks/test_test.exs index a76eb818dd3..717791cefea 100644 --- a/lib/mix/test/mix/tasks/test_test.exs +++ b/lib/mix/test/mix/tasks/test_test.exs @@ -687,6 +687,51 @@ defmodule Mix.Tasks.TestTest do 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: false + ] + 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_tests.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 From cd7edd155b24ec9a592afeb51b93f0e23931c400 Mon Sep 17 00:00:00 2001 From: Steffen Deusch Date: Fri, 27 Dec 2024 15:46:20 +0100 Subject: [PATCH 6/9] fix test_helper filename in tests --- lib/mix/test/mix/tasks/test_test.exs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/mix/test/mix/tasks/test_test.exs b/lib/mix/test/mix/tasks/test_test.exs index 717791cefea..d5614a9091a 100644 --- a/lib/mix/test/mix/tasks/test_test.exs +++ b/lib/mix/test/mix/tasks/test_test.exs @@ -658,7 +658,7 @@ defmodule Mix.Tasks.TestTest do end """) - File.write!("test/test_helper_tests.exs", "ExUnit.start()") + 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") @@ -717,7 +717,7 @@ defmodule Mix.Tasks.TestTest do end """) - File.write!("test/test_helper_tests.exs", "ExUnit.start()") + 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") From 299bb53783a7f786a9c499b74d0c2dc9edeb8e8b Mon Sep 17 00:00:00 2001 From: Steffen Deusch Date: Sun, 29 Dec 2024 12:30:30 +0100 Subject: [PATCH 7/9] Update lib/mix/lib/mix/tasks/test.ex MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: José Valim --- lib/mix/lib/mix/tasks/test.ex | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/mix/lib/mix/tasks/test.ex b/lib/mix/lib/mix/tasks/test.ex index b1447f9f57e..de20cdd686f 100644 --- a/lib/mix/lib/mix/tasks/test.ex +++ b/lib/mix/lib/mix/tasks/test.ex @@ -712,10 +712,10 @@ defmodule Mix.Tasks.Test do {acc, directly_included} -> case :elixir_utils.read_file_type(path) do {:ok, :directory} -> - {[acc, Path.wildcard("#{path}/**/#{pattern}")], directly_included} + {[Path.wildcard("#{path}/**/#{pattern}") | acc], directly_included} {:ok, :regular} -> - {[acc, path], [path | directly_included]} + {[path | acc], [path | directly_included]} _ -> {acc, directly_included} From 5c018289e5c4c2e505c5d3e109d809f8fafc36a0 Mon Sep 17 00:00:00 2001 From: Steffen Deusch Date: Sun, 29 Dec 2024 12:44:00 +0100 Subject: [PATCH 8/9] test_ignore_filters: append to defaults --- lib/mix/lib/mix/tasks/test.ex | 27 ++++++++++----------------- lib/mix/test/mix/tasks/test_test.exs | 2 +- 2 files changed, 11 insertions(+), 18 deletions(-) diff --git a/lib/mix/lib/mix/tasks/test.ex b/lib/mix/lib/mix/tasks/test.ex index de20cdd686f..feefb250684 100644 --- a/lib/mix/lib/mix/tasks/test.ex +++ b/lib/mix/lib/mix/tasks/test.ex @@ -245,17 +245,12 @@ defmodule Mix.Tasks.Test do 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. - Defaults to: + 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. - ```elixir - [ - &String.ends_with?(&1, "_helper.exs"), - fn file -> Enum.any?(elixirc_paths(Mix.env()), &String.starts_with?(file, &1)) end - ] - ``` - - This ensures that any helper or test support files are not triggering a warning. - Warnings can be disabled by setting this option to `false` or `nil`. + 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 @@ -755,12 +750,10 @@ defmodule Mix.Tasks.Test do # ignore any _helper.exs files and files that are compiled (test support files) test_ignore_filters = - Keyword.get_lazy(project, :test_ignore_filters, fn -> - [ - &String.ends_with?(&1, "_helper.exs"), - fn file -> Enum.any?(elixirc_paths, &String.starts_with?(file, &1)) end - ] - end) + [ + &String.ends_with?(&1, "_helper.exs"), + fn file -> Enum.any?(elixirc_paths, &String.starts_with?(file, &1)) end + ] ++ Keyword.get_lazy(project, :test_ignore_filters, []) {to_load, to_ignore, to_warn} = for file <- potential_test_files, reduce: {[], [], []} do @@ -769,7 +762,7 @@ defmodule Mix.Tasks.Test do any_file_matches?(file, test_load_filters) -> {[file | to_load], to_ignore, to_warn} - !test_ignore_filters || any_file_matches?(file, test_ignore_filters) -> + any_file_matches?(file, test_ignore_filters) -> {to_load, [file | to_ignore], to_warn} true -> diff --git a/lib/mix/test/mix/tasks/test_test.exs b/lib/mix/test/mix/tasks/test_test.exs index d5614a9091a..35acb7e6c7f 100644 --- a/lib/mix/test/mix/tasks/test_test.exs +++ b/lib/mix/test/mix/tasks/test_test.exs @@ -699,7 +699,7 @@ defmodule Mix.Tasks.TestTest do app: :test_warn, version: "0.0.1", test_load_filters: [~r/.*_tests\.exs/], - test_ignore_filters: false + test_ignore_filters: [fn _ -> true end] ] end end From 972bb822fea4e9ab9fe51a7a3c012971698ce6cb Mon Sep 17 00:00:00 2001 From: Steffen Deusch Date: Sun, 29 Dec 2024 12:54:43 +0100 Subject: [PATCH 9/9] get_lazy -> get --- lib/mix/lib/mix/tasks/test.ex | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/mix/lib/mix/tasks/test.ex b/lib/mix/lib/mix/tasks/test.ex index feefb250684..167ca3ee111 100644 --- a/lib/mix/lib/mix/tasks/test.ex +++ b/lib/mix/lib/mix/tasks/test.ex @@ -753,7 +753,7 @@ defmodule Mix.Tasks.Test do [ &String.ends_with?(&1, "_helper.exs"), fn file -> Enum.any?(elixirc_paths, &String.starts_with?(file, &1)) end - ] ++ Keyword.get_lazy(project, :test_ignore_filters, []) + ] ++ Keyword.get(project, :test_ignore_filters, []) {to_load, to_ignore, to_warn} = for file <- potential_test_files, reduce: {[], [], []} do