karolsluszniak / ex_check

One task to efficiently run all code analysis & testing tools in an Elixir project. Born out of 💜 to Elixir and pragmatism.
https://hex.pm/packages/ex_check
MIT License
308 stars 11 forks source link

Order of test runs in umbrella app #7

Closed bforchhammer closed 4 years ago

bforchhammer commented 4 years ago

Hi,

Thank you for publishing this library! It's a super convenient library to have in our stack and we were quite happy when we are able to remove extra logic from our Makefiles/ build pipelines for things like dyalixir and credo :heart:

We've been having some issues when trying to upgrade to the latest version (0.11) and the new parallel testing of umbrella apps. Unfortunately, some of our service tests seem to depend on each other (we commonly have a :core app for shared database-related functionality and other api and ui apps depending on core); ideally, our tests would not have those implicit dependencies, but for now this is what we have to live with :wink:

I already tried to disable the parallel testing by setting parallel: false in the umbrella config, but the order of apps still seems to be different to what it was in 0.10.

Is there any way for us to configure the order of apps in tests, or define :core as one that always needs to run first?

karolsluszniak commented 4 years ago

Yes, due to changes that enabled extra perks for umbrella cases the app order is now determined by Mix.Project.apps_paths() and not the cross-app dependencies.

You have two choices with the current feature set of ex_check:

A) Disable recursive execution of a tool in umbrella - then tests will run just like you'd run mix test from umbrella root and will be ordered with respect to the cross-app dependencies.

# .check.exs
[
  tools: [
    {:ex_unit, umbrella: [recursive: false], detect: []},
  ]
]

B) Hand-pick specific umbrella apps - it's a little verbose but this way you'll be able to customize the run order and keep the perks of parallel execution of checks for at least some apps + separate error presentation:

[
  tools: [
    {:ex_unit, false},
    {:ex_unit_core, "mix test", umbrella: [apps: [:app_core]]},
    {:ex_unit_rest, "mix test", umbrella: [apps: [:app_a, :app_b]], deps: [{:ex_unit_core, :app_core}]}
  ]
]

I'll keep this issue in mind for possible improvements of the dependency system in the future.

Sorry for the late response. 🙇

bforchhammer commented 4 years ago

Thanks for following up! :heart:

I wasn't able to get Option B to work; I see that run_after was just removed in the latest version, and deps: [:ex_unit_core] didn't seem to work for me... I might try again at a later point.

Option A works perfectly though, so that's what I'm going with for now. Thanks again for following up and for this library!

karolsluszniak commented 4 years ago

OK, option B should work with deps but only if you refer to umbrella-aware dependency with the {tool, umbrella_app} tuple (or tuples if wanting to depend on multiple ones) - like in code from my previous comment - I've updated it with deps name change.

This is a limitation of current dependency system that I'm well aware of - it doesn't reduce per-umbrella-app invocations of specific tool into a single status so that you could refer to it simply with tool name when defining deps on said tool. TBH I don't know how to solve this yet - i.e. how to logically reduce a possible plethora of exit statuses from multiple sub-apps into a single status that the dependent could refer to. Solving it is also not so high on my current priority list, but it does stand as a cool challenge to tackle someday.

Anyway, option A is simpler and unless you're chasing parallelism you should be good with it. :)