Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

(mix test) add test_load_filters and test_warn_filters #14036

Merged
merged 9 commits into from
Dec 29, 2024
152 changes: 137 additions & 15 deletions lib/mix/lib/mix/tasks/test.ex
Original file line number Diff line number Diff line change
Expand Up @@ -228,10 +228,35 @@ 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.

Defaults to:

```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`.
SteffenDE marked this conversation as resolved.
Show resolved Hide resolved
Paths are relative to the project root and separated by `/`, even on Windows.

## Coloring

Expand Down Expand Up @@ -595,20 +620,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))
Expand Down Expand Up @@ -660,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]}
SteffenDE marked this conversation as resolved.
Show resolved Hide resolved

_ ->
{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)
Expand All @@ -679,14 +749,66 @@ 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] || []

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_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
SteffenDE marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should include this filter by default, because if the user wants to add a new one, we should not ask them to reimplement this. WDYT? I am not sure if we should exclude the test_helper.exs by default too, but we probably should. So the user simply passes additional filters?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changed! If anyone feels the need to change those defaults, they'll hopefully open up an issue :)

]
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}

!test_ignore_filters || any_file_matches?(file, test_ignore_filters) ->
{to_load, [file | to_ignore], to_warn}

true ->
{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_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 [
Expand Down
2 changes: 1 addition & 1 deletion lib/mix/test/fixtures/test_failed/mix.exs
Original file line number Diff line number Diff line change
Expand Up @@ -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
2 changes: 1 addition & 1 deletion lib/mix/test/fixtures/test_stale/mix.exs
Original file line number Diff line number Diff line change
Expand Up @@ -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
2 changes: 1 addition & 1 deletion lib/mix/test/fixtures/umbrella_test/apps/bar/mix.exs
Original file line number Diff line number Diff line change
Expand Up @@ -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]
]
Expand Down
2 changes: 1 addition & 1 deletion lib/mix/test/fixtures/umbrella_test/apps/foo/mix.exs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
110 changes: 110 additions & 0 deletions lib/mix/test/mix/tasks/test_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -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: 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.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}} ->
Expand Down