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

Test middleware for ClojureScript #555

Open
arichiardi opened this issue Oct 18, 2018 · 12 comments
Open

Test middleware for ClojureScript #555

arichiardi opened this issue Oct 18, 2018 · 12 comments

Comments

@arichiardi
Copy link
Contributor

@arichiardi arichiardi commented Oct 18, 2018

Hi all, I have been longing for some minimal test support for ClojureScript for long time and I actually had a look at the code.

It seems to me it is doing a lot and I am quite wondering why. I did get the custom reporting piece so that we can stream back to cider but it is also basically replicating the test execution functions.

I told myself "there must be a reason!". So I opened this ticket for asking more info 😄

In my head the most basic test functionality is something that calls test/test-var with the var of the symbol at point.

@plexus plexus mentioned this issue Dec 27, 2018
0 of 5 tasks complete
@bbatsov

This comment has been minimized.

Copy link
Member

@bbatsov bbatsov commented Dec 29, 2018

It seems to me it is doing a lot and I am quite wondering why. I did get the custom reporting piece so that we can stream back to cider but it is also basically replicating the test execution functions.

I guess @jeffvalk can speak best to his intentions, but I think he implemented the middleware in terms of eval mostly to be able to interrupt the test execution - otherwise we'd potentially need some special mechanism to do so.

Btw, when it comes to ClojureScript support that's actually not a bad thing, as the eval op is only only way to evaluate ClojureScript code currently. :-)

@arichiardi

This comment has been minimized.

Copy link
Contributor Author

@arichiardi arichiardi commented Dec 30, 2018

Yep I am aware of the eval limitation and still hammock-ing on that as well 😄

@jeffvalk

This comment has been minimized.

Copy link
Contributor

@jeffvalk jeffvalk commented Jan 3, 2019

@arichiardi I did attempt to explain the reason in the comment here...

;; These functions are based on the ones in `clojure.test`, updated to accept
;; a list of vars to test, use the report implementation above, and distinguish
;; between test errors and faults outside of assertions.

Apparently I wasn't as clear as I'd hoped! 🙂

Originally, the code used clojure.test's implementation of test-var, etc, just as you probably expected. A few changes required overriding these functions, essentially one at a time. Here's that history in brief:

  • test-ns was originally reimplemented to change its argument list. (Admittedly, this could have been done differently at the onset, but it made the code more clear.) That said, this function was updated in 70db5f1 to better handle exceptions in test fixtures, a feature clojure.test doesn't have.
  • test-var was reimplemented in abc6c2b "to differentiate between errors occurring in assertions and test faults that do not"
  • test-nss was created in e826247 to "enable running tests in multiple namespaces"
@arichiardi

This comment has been minimized.

Copy link
Contributor Author

@arichiardi arichiardi commented Jan 3, 2019

Thank you @jeffvalk this is exactly what I was looking for!

And yes I would say that - naively maybe - I was kind of thinking you could still use test-var and the like for testing and hook up custom reporters to clojure.test for modifying the test output to our needs.

Probably not though because the user might have already overridden those.

What one would need for tooling is something like a tee maybe? I haven't given this much thought and as the moment I am not of dedicating all my open source time to another orchard PR but this is a useful conversation nonetheless.

@jeffvalk

This comment has been minimized.

Copy link
Contributor

@jeffvalk jeffvalk commented Jan 3, 2019

The custom report function is still the basis of what the test middleware does...it just has additional execution logic to do things like select which namespaces to test, handle exceptions thrown in test fixtures, store exceptions for later inspection, store failed tests to be rerun, etc.

What data were you thinking of piping multiple ways? If that's really what's needed, it sounds simple enough. Not sure what you're envisioning though.

@bbatsov

This comment has been minimized.

Copy link
Member

@bbatsov bbatsov commented Jan 3, 2019

@jeffvalk What I don't understand is just the use of the eval middleware to drive all of this. What was the problem with simply running the tests directly in the test middleware? Above I speculated you wanted to make them interruptible, but this could have been achieved with an extra op for the test middleware as well.

I was also thinking that if we simply counted the tests the before we ran them and sent an update after each ran implementing the dynamic run progress would be much easier.

@jeffvalk

This comment has been minimized.

Copy link
Contributor

@jeffvalk jeffvalk commented Jan 3, 2019

@bbatsov Using the eval middleware wasn't my doing, so I can't speak to the trade-offs. That addition happened in 8b9e341. Looks like this was in response to clojure-emacs/cider#1095.

@bbatsov

This comment has been minimized.

Copy link
Member

@bbatsov bbatsov commented Jan 3, 2019

Oh, I totally forgot it was @Malabarba who did this. Well, at least it seems I guessed right. :-)

@PEZ

This comment has been minimized.

Copy link

@PEZ PEZ commented Sep 15, 2019

Is this the issue where people should voice their support for making the test ops work with ClojureScript? Here's my vote anyway, let me know if I should open a separate issue. It creates a bit of a broken user experience when the editor's test features don't work with cljs code.

@bbatsov

This comment has been minimized.

Copy link
Member

@bbatsov bbatsov commented Sep 15, 2019

@PEZ I get this, but unfortunately everything that requires evaluation of ClojureScript code is a bit hard to do, as you have to actually pipe this code to the eval middleware. Remember that cider-nrepl is a Clojure app that can only evaluate Clojure code. I guess something simple can be done via eval - e.g. just calling the default running and parsing its output, but someone else will have to tackle this.

@benedekfazekas

This comment has been minimized.

Copy link
Member

@benedekfazekas benedekfazekas commented Sep 17, 2019

worth noting here I suppose that there is a workaround to run cljs tests in emacs with kaocha via https://github.com/magnars/kaocha-runner.el

kaocha takes care of actually running the tests (both clj and cljs) the above emacs package is the frontend so you can run test at point, tests in ns etc.

@bbatsov

This comment has been minimized.

Copy link
Member

@bbatsov bbatsov commented Sep 17, 2019

Yeah, that's true. But it's just an eval-based solution which we can immediately have in CIDER even without an extra package. I believe the real point of this ticket is to provide a generic solution that other editors can use directly as well.

@SevereOverfl0w brought up the point that we can actually evaluate code via the cljs repl environment and if that's the case probably that's the simplest real solution that we can come up with.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
5 participants
You can’t perform that action at this time.