Skip to content

Commit

Permalink
(mix test) add test_load_filters and test_warn_filters
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
SteffenDE committed Dec 10, 2024
1 parent b75cccc commit 09ebf2b
Show file tree
Hide file tree
Showing 14 changed files with 156 additions and 18 deletions.
115 changes: 101 additions & 14 deletions lib/mix/lib/mix/tasks/test.ex
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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))
Expand Down Expand Up @@ -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 [
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
16 changes: 16 additions & 0 deletions lib/mix/test/fixtures/test_warn/mix.exs
Original file line number Diff line number Diff line change
@@ -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
Empty file.
Empty file.
7 changes: 7 additions & 0 deletions lib/mix/test/fixtures/test_warn/test/a_tests.exs
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
defmodule ATests do
use ExUnit.Case

test "dummy" do
assert true
end
end
Empty file.
Empty file.
Empty file.
1 change: 1 addition & 0 deletions lib/mix/test/fixtures/test_warn/test/test_helper.exs
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
ExUnit.start()
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
27 changes: 27 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,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}} ->
Expand Down

0 comments on commit 09ebf2b

Please sign in to comment.