Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Plugging in Eastwood into the test runner #179

Closed
vemv opened this issue Feb 1, 2022 · 13 comments
Closed

Plugging in Eastwood into the test runner #179

vemv opened this issue Feb 1, 2022 · 13 comments
Assignees
Labels
improvement Not a bug but an improvement to overall user experience

Comments

@vemv
Copy link

vemv commented Feb 1, 2022

Context / Problem statement

Eastwood is a linter that provides unique insights, complementing other linter offerings such as clj-kondo's.

It has the nuance that its runtime grows linearly with codebase size - could be some 8-10m for a 100KLOC codebase.

My understanding is that Polylith has some built-in logic that looks at Git diffs and only runs relevant tests.

Proposal

Offer the reuse of that logic for running other test runners or linters, in this case Eastwood.

I don't know much about Poly internals or if this is available already. I'd be willing to implement an Eastwood integration if a nice API/hook is offered.

Alternatives

In the past I've run Eastwood for git-diff-derived workloads via a project called formatting-stack; it works nicely but currently I'm not promoting that project much.

Something that would be integrated with an existing framework would seem a nicer choice for most teams out there.

WYDT?

Cheers - V

@tengstrand
Copy link
Collaborator

Hi!

This sounds great! We have been a bit busy in the team recently, but will soon look into this and come back to you.

Cheers,
Joakim

@tengstrand tengstrand added the improvement Not a bug but an improvement to overall user experience label Feb 10, 2022
@tengstrand
Copy link
Collaborator

Hi again,

Adding support for a linter like Eastwood could be solved with the custom commands that I'm working on right now. The support for auto-complete got me a bit stuck with that issue, but the new plan is to maybe ship it without it to begin with, to be able to release a new version of the poly tool sooner.

/Joakim

@vemv
Copy link
Author

vemv commented Feb 14, 2022

Hey there,

that's awesome news! Thank you for your work.

Looking forward.

@tengstrand tengstrand self-assigned this Feb 15, 2022
@furkan3ayraktar
Copy link
Collaborator

Could this be solved by the pluggable test runner feature in #196?

@vemv
Copy link
Author

vemv commented Apr 7, 2022

I guess so from the PR name, but also don't know given the lack of an extended description atm

@imrekoszo
Copy link
Contributor

Hey there, I'm the author of #196.

While I normally consider linting and testing separate steps, I understand eastwood is a little more involved than other linters like clj-kondo so it might be more fitting to consider it part of the testing step where namespaces are actually loaded.

The changes proposed in #196 should provide a usable integration for this. You could implement the TestRunner protocol, make it invoke eastwood inside the project classloader and then delegate to poly's default test runner for executing unit tests.

It would also be possible in a later feature addition to allow a chain of "test" runners to be configured for a workspace, which could be executed one after another within the same class loader. I haven't considered this option for extension for #196 but it should be an additive change.

Do note, that poly doesn't currently calculate changed namespaces, only changed files, bricks and projects.

@imrekoszo
Copy link
Contributor

Actually, stay tuned for that multiple test runners support.

@imrekoszo
Copy link
Contributor

Using #196 it is now possible to configure multiple test runners for a project. Here's an excerpt from poly help test from that PR:

  {;; To use it as the default test runner for the workspace
   :test {:create-test-runner my.test-runner/create}

   :projects
   {
    ;; To only use it for specific projects
    "foo" {:test {:create-test-runner my.test-runner/create}}

    ;; To revert to poly's built-in default test runner only for specific projects
    "bar" {:test {:create-test-runner :default}}

    ;; To use multiple test runners invoked in series
    "baz" {:test {:create-test-runner [my.linter/create :default my.extra/create]}}
    }
   }

@tengstrand
Copy link
Collaborator

Does the new pluggable test runner solve your problem @vemv ?

@vemv
Copy link
Author

vemv commented May 7, 2022

Hi! Sorry, I don't have much time in my hands nowadays. I don't use poly at my new job either 😔

Could I have an empty project with the boilerplate filled? Config + stubbed protocol implementation. I could definitely check out that and see if it can be easily filled with an Eastwood implementation.

@imrekoszo
Copy link
Contributor

@vemv not a stub per se but an example implementation using kaocha: https://github.com/imrekoszo/polylith-kaocha

@vemv
Copy link
Author

vemv commented May 7, 2022

Thanks!

Quickly eyeing https://github.com/imrekoszo/polylith-kaocha/blob/f8536b683842ac1d70eb1e1ab918be0369c43ce5/components/kaocha-test-runner/src/polylith_kaocha/kaocha_test_runner/core.clj, it looks a little more dense than I would have expected (not a criticism).

I'll give it a go when I find a good chance.

@imrekoszo
Copy link
Contributor

it looks a little more dense than I would have expected (not a criticism).

As most of the action in kaocha's case needs to happen inside the classloader of the project under test, the code you linked is really only responsible for creating the interface between polylith and kaocha. Most of the kaocha-specific action (which the code you linked will invoke) happens in https://github.com/imrekoszo/polylith-kaocha/tree/f8536b683842ac1d70eb1e1ab918be0369c43ce5/components/kaocha-wrapper/src/polylith_kaocha/kaocha_wrapper

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
improvement Not a bug but an improvement to overall user experience
Projects
None yet
Development

No branches or pull requests

4 participants