(mix test) add test_load_filters and test_warn_filters

[SteffenDE]

Edit: This is the old suggestion, but I kept it for posterity. For the updated description, please scroll down :)

This is an alternative to #14034, which only traverses the file system once.

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.

[josevalim]

What do you think about keeping test_pattern? By default it finds both .ex and .exs files. And then we reject the results of test_pattern.

It is a smaller breaking change and if someone has changed the test_pattern, then worst case scenario they don’t get a warning.

[SteffenDE]

But if we keep test_pattern as glob pattern, we need to evaluate the pattern twice, don't we?

I really wish there was a built in glob pattern to regex or something...

[josevalim]

test_pattern is the glob, the others are strings and regexes we run over the matched files

[SteffenDE]

Adjusted to use test_pattern with new default *.{ex,exs}. Existing projects with custom test_pattern might still run into conflicts with the new test_load_pattern.

[SteffenDE]

Updated description (taken from the commit message):

(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.

[mgibowski]

I don't know how far you want to take the default pattern, just sharing my experience - it happened to me calling the test files ...text_exs.

[SteffenDE]

@mgibowski no file extension at all? 😅

With the changes from the PR, you'd get a warning for foo_text_exs if you set the test_paths to **.

[mgibowski]

@SteffenDE
oh, sorry - it was late when I wrote this message.
What I meant was ..._text.exs

[SteffenDE]

@mgibowski that should warn with the new defaults :)

[josevalim]

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?

[SteffenDE]

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

[josevalim]

I added only nits now, we are good to go!

[josevalim]

💚 💙 💜 💛 ❤️