search

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

Feature request: add JS tests #2

Closed bitboxer closed 5 years ago

bitboxer commented 5 years ago

It would be amazing if mix check would also run the tests of the javascript part of an phoenix app. Would you love to have a PR for this?

bitboxer commented 5 years ago

To make it clear: I know that I can add that config myself, but we could check for the existence of a test command in the package json and use that.

karolsluszniak commented 5 years ago

@bitboxer I’m glad to see this feature request because indeed I have it on the agenda, so it’s nice to see it anticipated.

Currently I’m focusing on bringing cross-tool dependencies (already landed on master) and using that for validating test coverage. Seamless & automated execution of npm test (along with possibly for including other JS checking tools) in Phoenix project will come right after.

There are a few blockers though that I need to figure out & remove before this will be possible in quality & bug-free way. To be specific:

  1. Phoenix projects may be generated in a form of umbrella projects. In such projects, mix check may be run in root of the project or in any of the apps, some of which will have assets while others won’t . Now, the expected behaviour with npm test would be to execute it for specific Phoenix app if you’re in its directory, to ignore it for app without assets and to run it recursively (in parallel?) for all viable apps if mix check is run in umbrella root. This is currently not possible, but will be after extending ex_check with an equivalent of Mix.Task’s recursive option. Something that would truly empower checking umbrella projects in many other scenarios as well.

  2. Curated tools aim to be viable defaults for any Elixir project, not just Phoenix project - and should be advertised via messages about skipped tools to hint possible toolset expansion. npm test for assets subdirectory is strictly Phoenix-centric and as such I wouldn’t want this tool to be hinted for non-Phoenix projects but still I’d like it to „awaken” when things are in place. This is currently not possible and I’ll have to figure out a clean way for missing curated tool requirements to sometimes show hints and sometimes not.

  3. If we were to indeed run the check if test script is configured in package.json and skip otherwise, then ex_check would have to start understanding package.json which it currently also doesn’t (or use another means for ensuring that it may be run). I see this as a worthwhile extension for sake of Phoenix users but it’s also not there yet.

If you feel like tackling these challenges, I’m happy to see a PR. Project is still at early stage and lacks contributing guide & code of conduct (I hope to add them soon) so we may have to do some alignments on your code but I don’t see this as a PR blocker if it’s not offputting for you.

Either way I’ll keep this issue up to date as the blockers are removed.

bitboxer commented 5 years ago

Awesome. That sounds like you already have a plan how to refactor the app to make this possible. Can you open tickets for each and explain how you would love these things to be implemented? Then I would pick one and start from there :) .

karolsluszniak commented 5 years ago

I'm happy to report that the addition of including npm test for Phoenix projects has yesterday landed on master (merged from #3) and released as part of v0.11.0.

As promised, it works just as well in regular and umbrellized Phoenix projects. And actually any Elixir projects that have JS assets with package.json in assets directory, which can be easily reconfigured to target different directory via {:npm_test, cd: "other_dir"} in .check.exs. And if one would have more than one web app with assets in an umbrella, it'll hand-pick them and execute npm test for all of them in parallel.

Btw I've decided to proceed with this one on my own considering the vast scope of changes and batches of refactors required for making this one happen without making the library code harder to maintain.

Thanks for pushing this one forward and ofc please let me know in case of any problems.

bitboxer commented 5 years ago

Thank you for your hard work on this!