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

Task 'checks' could not be found #1

Closed alanvardy closed 5 years ago

alanvardy commented 5 years ago

Hello!

Big fan of your library, and have been enjoying using it!

0.6.0 worked great, but my CI broke with 0.7.0 Screenshot from 2019-07-29 21-00-10

I have no problem running mix check locally.

I'm just going through your commits right now, but was wondering if you already had the answer :).

karolsluszniak commented 5 years ago

@alanvardy Hi and thank you - it’s so nice to hear the tool proves helpful.

The only change between 0.6.0 and 0.7.0 that I can think of that could possibly affect the presence of mix check on your CI was the README update in which it stopped recommending to add :ex_check package to :test env (diff).

If you’ve applied this new only: :dev to your mix.exs and your CI env has something like export MIX_ENV=test then that may be it. If so, you may fix the issue either by running the task with MIX_ENV=dev mix check or by changing dependency in mix.exs to have only: [:dev, :test] (but I wouldn’t recommend that since test env is not intended for tools like ex_check and it was only out of necessary evil that 0.6.0 had it included).

Let me know if it helped. Otherwise we’ll have to dig deeper.

alanvardy commented 5 years ago

Thanks for the quick reply!

I followed the new readme and switched all of the dependencies to only: [:dev]

I am running CircleCI on MIX_ENV=test, if I switch the environment to MIX_ENV=dev then things go rather badly with it needing a development database etc.

I am having luck with just running MIX_ENV=dev mix check however! It does spit out a lot (hundreds) of warnings from postgrex. Getting multiple databases working in CircleCi is a bit of a hassle :)

I am considering using ex_check in dev locally, and then just running the checks individually in CI. The test machine only has 2 cores, so the concurrency isn't as much of an advantage anyways!

Do you think there is a solution for my use case or am I just trying trying to put a square peg in a round hole? Maybe I could make a PR and add a little documentation that could help the next person.

karolsluszniak commented 5 years ago

I am having luck with just running MIX_ENV=dev mix check however! It does spit out a lot (hundreds) of warnings from postgrex. Getting multiple databases working in CircleCi is a bit of a hassle :)

I think I’ve found the culprit for your MIX_ENV=dev mix check on CI starting the app and triggering all these issues.

mix check shouldn’t start your application, but as since 0.7.0 it started wrapping your mix tasks in mix do run -e … (in order to enable ANSI colors) it did trigger the app start by accident. I’ve fixed that in commit 0862b22c by adding —no-start option and covered this case with additional test in 0be2c73e to ensure this problem won’t reoccur ever again. Thanks for catching it!


I’ve released v0.8.0 with a fix so just give it a try and let me know if everything finally works fine.

I am considering using ex_check in dev locally, and then just running the checks individually in CI. The test machine only has 2 cores, so the concurrency isn't as much of an advantage anyways!

It’s up to you but I’d recommend to keep using mix check on the CI as otherwise you’ll lose some of the sweet benefits:

Let me know what you think - it’s great to hear from an early adoptor :)

alanvardy commented 5 years ago

Screenshot from 2019-07-31 20-32-02

We are getting closer! I am running the command MIX_ENV=dev mix check I am not getting the postgrex errors any more, just the errors in the attached picture.

Is there anything I can do to help you diagnose?

karolsluszniak commented 5 years ago

Once again I’m coming back with a fix :)

This time I’ve completely revamped the way the ANSI enabling hack works and I’m pretty sure that now it’s done in the most appropriate & least obtrusive way. It now also handles both mix and elixir commands. And damn, it’s like the 5th version of ex_check that changes the way it works - let’s hope it’s the last.

Both this and previous problem that you’ve reported in this issue are now covered by the ExCheck.ProjectCases.ApplicationModTest test case so ex_check should be rid of them for good.

v0.9.0 is released so please give it a spin. You’ll also find CHANGELOG.md in the repo now. Once again thanks for pushing forward ex_check towards perfection!

alanvardy commented 5 years ago

We are golden! Thank you so much for your help :)

I am thinking of working on a PR to bring in ex_coveralls, would you be interested in that if I decide to go for it?

karolsluszniak commented 5 years ago

The issue is resolved so I’m closing it.

As to excoveralls I don’t think you should run it alongside mix test because then you’re running the test suite twice. As excoveralls also runs all the tests & reports their output, shouldn’t you rather replace ex_unit with excoveralls in .check.exs? Then you’d collect test errors and the coverage in one go. Also you’d then avoid duplicate build of test env since ex_check only precompiles dev env (something to consider fixing in future ex_check versions).

If you’d have trouble with reporting test errors from tasks provided by excoveralls, you could also test the mix test task with —cover option. I think that it’ll run the excoveralls code just fine if it’s configured as coverage tool in mix.exs. Let me know if you need help.

Anyway, I wouldn’t want to add excoveralls to curated tools as it’s not a reasonable default for most of the projects. To be specific:

I’ll soon create a contribution guide to better describe the essence of curated tools for future contributors.

Let me know what you think!