karolsluszniak/ex_check

Respect mix.exs test_path configuration

bradleygolden opened this issue · 1 comments

First, I want to say I really like this project a lot and thank you for your hard work!

I have a test directory structure in an app within an umbrella project that looks like this:

test/
  unit/
    test_helper.exs
    some_test.exs
  acceptance/
    test_helper.exs
    another_test.exs

I've done this by customizing test_paths as specified in the mix documentation.

When I run mix check, I get the following output:

=> finished in 0:10

 ✓ compiler success in 0:03
 ✓ credo success in 0:05
 ✓ dialyzer success in 0:06
 ✓ ex_doc success in 0:07
 ✓ ex_unit in my_app success in 0:04
 ✓ formatter success in 0:01
 ✓ sobelow in my_app_web success in 0:02
   ex_unit in my_app_web skipped due to missing file apps/my_app_web/test/test_helper.exs

When I run mix test alone, all tests run successfully within my umbrella project. As a workaround, I've gotten mix check to run tests if I add a blank test_helper.exs to the test directory in my app. While this is a decent solution for the interim, I'd prefer to not have that file there if it's not needed. Looking at the source code in the project, it looks like test/test_helper.exs is hardcoded here:

{:ex_unit, "mix test", detect: [{:file, "test/test_helper.exs"}]},

Would it be possible to respect the test_paths configuration in mix.exs or not require test_helper.exs at that specific path?

Thanks for the kind words on the project and I'm glad it proves useful!

My goal with check detection was to keep it as generic as possible hence I settled on hardcoded detection instead of pulling the Mix config for ExUnit. The only exception from that rule is the behavior of the compiler check.

Your case is 2 test suites in test directory so I've implemented a much simpler (& indeed generic) solution for this case in #12. I just check for existence of test directory itself instead of test helper file, much like Mix itself does as explained in docs linked by you above. This also keeps the config less ExUnit-specific.

So this particular case is covered, but what if someone comes up with something even more weird e.g. test suite in different directory? Well, for these cases I'd go for manual config in .check.exs. For instance you could disable the detection for ex_unit tool by passing:

[
  tools: [
    {:ex_unit, detect: []}
  ]
]

Or, for umbrella, you could manually specify which umbrella apps to run it on:

[
  tools: [
    {:ex_unit, umbrella: [apps: [:app_a, :app_c]], detect: []}
  ]
]

And as for future of ex_check goes we could consider dropping the detection & assume that all apps have tests but I bet there will be some edge cases there as well, or we could indeed look for :test_paths via some public function built into ex_check and refernce it via detect: &ExCheck.Detection.ex_unit?/0 in default config. But something tells me we're trying too hard for a really really edge case that can be covered manually via configs presented above (we could ensure that docs are up for the task of explaining how to configure these cases properly).