TestsInTestFolder is over-agressive

[halostatue]

This check asserts the whether "lib" in Path.split(source_file.filename) is true. However, this is finding cases like: test/lib/context/employees_test.exs, which is technically correct (and indicates that the test probably belongs in a different folder), but is not correct because the test is under the test/ folder.

It seems to me that the condition should be that a test file is, first and foremost, found outside of the test/ directory, or if that’s not preferred, that there’s a second check to make sure that the file’s path doesn’t begin with test/.

[uesteibar]

Hi @halostatue! Not sure I'm following, test/lib/context/employees_test.exs would (according to the rule) indeed be wrong and should be test/context/employees_test.exs right? I'm sure I'm missing something here tho, so if you could clarify that it'd be very helpful.

[halostatue]

It took me a bit to recall what I meant from months ago. However, it should not be incorrect to have test/lib/context/employees_test.exs, even if it’s a little unusual. More problematic might be something like test/module/lib/function_test.exs that would be testing lib/module/lib/function.ex (Module.Lib.Function). The test here should probably be verifying that the first path part for a test file is test/ and not lib/.

[uesteibar]

Ah I see what you mean! Yes that is correct, we should indeed be checking for the first part of the path only 👍

[halostatue]

The main question would be whether source_file.filename is a partial path test/foo/bar.exs or a full path /home/user/projects/foo_bar/test/foo/bar.exs. If it’s the former, the test is really easy: match?("test/" <> _, source_file.filename). If it’s the latter…

[uesteibar]

yep, true that, I'll poke around 😸