Skip to content

Commit

Permalink
(mix test) add test_load_pattern and test_warn_pattern
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 configurations
and instead add two new configurations: `test_load_pattern` and
`test_warn_pattern`. Now, we can load all potential test files once and
then match their paths to the patterns.

The `test_load_pattern` is used to filter the files that are loaded by
mix test. This defaults to the regex equivalent of "*_test.exs". The
`test_warn_pattern` is used to filter the files that we warned about if
they are not loaded. By default, we warn about any file that either ends
in `_test.ex` (the old behavior) but also any file that ends in `.exs`,
but does not end in `_helper.exs`, which will prevent the default
test_helper.exs from generating a warning and also provides a way to name
other files that might be required explicitly in tests. As an escape hatch
the `test_warn_ignore_files` list can be used to ignore specific files
where the warning should not be generated.

For projects with an existing `test_pattern` or `warn_test_pattern`
configuration, a deprecation warning is logged. The new warnings can be
disabled by setting `test_warn_pattern` to a falsy value.
  • Loading branch information
SteffenDE committed Dec 7, 2024
1 parent b75cccc commit 5b56d00
Show file tree
Hide file tree
Showing 12 changed files with 113 additions and 17 deletions.
81 changes: 68 additions & 13 deletions lib/mix/lib/mix/tasks/test.ex
Original file line number Diff line number Diff line change
Expand Up @@ -228,10 +228,18 @@ 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_load_pattern` - a regular expression to load test files.
Defaults to `~r/.*_test\\.exs$/`
* `:warn_test_pattern` - a pattern to match potentially misnamed test files
and display a warning. Defaults to `*_test.ex`
* `:test_warn_pattern` - a regular expression to match potentially
misnamed test files and display a warning.
Defaults to `~r/^(?!.*_helper\.exs$)(?:.*_test\.ex|.*\.exs)$/`,
matching any file that ends in `_test.ex` or `.exs`, ignoring files
ending in `_helper.exs`. Warnings can be disabled by setting this to
`nil` or `false`.
* `:test_warn_ignore_files` - a list of files to ignore when displaying
warnings about potentially misnamed test files. Defaults to `[]`.
## Coloring
Expand Down Expand Up @@ -595,20 +603,49 @@ 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_load_pattern = project[:test_load_pattern] || ~r/.*_test\.exs$/

test_warn_pattern =
Keyword.get(project, :test_warn_pattern, ~r/^(?!.*_helper\.exs$)(?:.*_test\.ex|.*\.exs)$/)

test_warn_ignore_files = project[:test_warn_ignore_files] || []

# Warn about deprecated configurations
if project[:test_pattern] do
Mix.shell().info(
"warning: the `:test_pattern` configuration is deprecated and will be ignored. Use `:test_load_pattern` instead."
)
end

if project[:warn_test_pattern] do
Mix.shell().info(
"warning: the `:warn_test_pattern` configuration is deprecated and will be ignored. Use `:test_warn_pattern` 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_pattern
potential_test_files = Mix.Utils.extract_files(test_files, "**")

unfiltered_test_files =
Enum.filter(potential_test_files, &Regex.match?(test_load_pattern, &1))

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)
# do not warn if test_warn_pattern is nil or false
test_warn_pattern &&
maybe_warn_misnamed_test_files(
potential_test_files,
unfiltered_test_files,
test_warn_ignore_files,
test_load_pattern,
test_warn_pattern
)

try do
Enum.each(test_paths, &require_test_helper(shell, &1))
Expand Down Expand Up @@ -679,13 +716,31 @@ 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 maybe_warn_misnamed_test_files(
potential_test_files,
unfiltered_test_files,
test_warn_ignore_files,
test_load_pattern,
test_warn_pattern
)
when is_struct(test_warn_pattern, Regex) do
missing_files = (potential_test_files -- unfiltered_test_files) -- test_warn_ignore_files

for file <- files do
Mix.shell().info(
"warning: #{file} does not match #{inspect(test_pattern)} and won't be loaded"
)
ignored = Enum.filter(missing_files, &Regex.match?(test_warn_pattern, &1))

if ignored != [] do
Mix.shell().info("""
warning: the following files do not match the test load pattern #{inspect(test_load_pattern)} and won't be loaded:
#{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_pattern` 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
end

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_pattern: ~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_pattern: ~r/.*_test_stale\.exs/
]
end
end
12 changes: 12 additions & 0 deletions lib/mix/test/fixtures/test_warn/mix.exs
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
defmodule TestWarn.MixProject do
use Mix.Project

def project do
[
app: :test_warn,
version: "0.0.1",
test_load_pattern: ~r/.*_tests\.exs/,
test_warn_pattern: ~r/^(?!.*_helper\.exs$)(?:.*_tests\.ex|.*\.exs)$/
]
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.
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_pattern: ~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_pattern: ~r/.*_tests\.exs/,
aliases: [mytask: fn _ -> Mix.shell().info("foo_running") end]
]
end
Expand Down
21 changes: 21 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,27 @@ defmodule Mix.Tasks.TestTest do
end
end

describe "test_load_pattern and test_warn_pattern" do
test "warns for files that are not loaded and match test_warn_pattern" do
in_fixture("test_warn", fn ->
output = mix(["test"])

# we don't warn about test_helper.exs or a_tests.exs (<- matches the load pattern)
assert output =~ """
the following files do not match the test load pattern ~r/.*_tests\\.exs/ and won't be loaded:
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 5b56d00

Please sign in to comment.