bhauman / devcards

Devcards aims to provide a visual REPL experience for ClojureScript
1.53k stars 116 forks source link

Tests that call other tests don't timeout #113

Closed paytonrules closed 5 years ago

paytonrules commented 7 years ago

I'm loving using devcards for running tests in the browser ( far superior to karma ) but I wanted to run all the tests for my project at once (instead of one namespace at a time).

I have this working because I wrote a macro that outputs this:

(deftest all-tests
  #namespace-name
  (namespace-name/test1)
  (namespace-name/test2)
  ; for all namespaces and all tests....
)

This almost completely works with one exception - async tests now hang instead of timing out. I've gone through the code a few times and I'm a little perplexed as to why this is. The timeout feature is excellent so I'd like to get it back.

Any fixes or suggestions for how to fix this problem are super-appreciated.

paytonrules commented 7 years ago

Actually it looks like calling the tests this way doesn't work at all on async tests.

bhauman commented 7 years ago

Are the tests you are calling (ie. namespace-name/test1) defined with deftest?

bhauman commented 7 years ago

I think this would work if the tests were defined in a function...

paytonrules commented 7 years ago

Yeah they're using deftest - that way I can run them namespace by namespace or all at once. Plus I get the nice async testing in deftest.

I actually grabbed this idea from the cljs.test documentation: https://github.com/clojure/clojurescript/blob/master/src/main/cljs/cljs/test.cljs#L80

I spent a little time in devcards trying to figure out why this works for deftest but not defcards, and didn't see the problem.

bhauman commented 7 years ago

The problem is that I'm having to bypass the deftest machinery to get custom reporting and async functionality.

Devcards at the moment can't claim 100% cljs.test functionality. It's optimized and intended for an interactive workflow/display of individual tests not for 100% compatibility. Test fixtures are also something that doesn't work.

It's pretty difficult to explain why ... but remember that devcards runs in a reloaded environment and that each "card" is a reporting block that runs independently of other cards, and this is quite different from how cljs.test is designed to work.

I'm happy to merge a fix for this but I think its going to be very difficult to maintain the beautiful workflow and get absolutely perfect cljs.test compatibility. In the end I opted for the beautiful workflow, rationalizing that for me the use case is interactively verifying behavior that I'm actively working on and the tradeoff is worth it.

You can still run all the tests for a namespace with run-tests etc.

If you want a good brain twisting exercise, read devcards test integration and cljs.test, ... a warning it may hurt a bit.

paytonrules commented 7 years ago

I think your approach is right, and I hope I'm not coming off as critical. It's why I'm using Devcards even though I'm not running all the tests at once. It's a much much better experience. It's why I'd like to get all tests running - it's far more pleasant than looking in the console for the final results. Definitely not a showstopper since I can still run all the tests at the command line.

I may have missed your comment earlier but wrapping the calls in a function doesn't work.

(defn all-tests
  #(namespace/test)

It sure looks like it should though. I'm still looking for a way to make this work.

bhauman commented 7 years ago

No what I mean is that when you call a deftest defined test you escape the system.

If you define a function with tests in it it will work

(defn mytest []
  (is true)
  (is false))
bhauman commented 7 years ago

Thanks for the question, gave me a chance to revisit this...

paytonrules commented 7 years ago

Thank you for the suggestion on calling them through functions. I've got a macro now that outputs them as functions, and since it takes a namespace as a parameter it's easy enough to make it run the tests in one namespace or many. I might even be able to use fixtures with it, although I don't really miss those.

paytonrules commented 7 years ago

I currently have a work-around that works reasonably well. I'll be explaining it in detail shortly, but that's for your help. It's really mostly your idea.

dijonkitchen commented 5 years ago

Any updates or should it be closed?