ClojureScript

Add on-testing-complete hook

Details

  • Type: Enhancement Enhancement
  • Status: Closed Closed
  • Priority: Minor Minor
  • Resolution: Completed
  • Affects Version/s: 0.0-3269
  • Fix Version/s: 0.0-3308
  • Component/s: None
  • Patch:
    Code

Description

When a test runner runs async tests created with cljs.test/async there is no reliable way to return the control from the async code in the test suite to the test runner. This is problematic since the test script might need the tests results to proceed or terminate.

A function to be called after all tests are done is proposed: cljs.test/*on-testing-complete-fn* and it would take the test summary as its only argument

It can be set by the user by calling cljs.test/set-on-testing-complete! which should be callable from JS (^:export)

Notes:
In the patch, the function cljs.test/successful? also has the ^:export metadata to be called from JS test runners.
The code was tested manually with V8, Spidermonkey, Nashorn, SlimerJs, and PhantomJS but not with JavaScript Core.

  1. cljs_1226.patch
    27/Apr/15 1:07 PM
    2 kB
    Sebastian Bensusan
  2. cls_1226_v2.patch
    12/May/15 5:39 AM
    1 kB
    Sebastian Bensusan

Activity

Hide
Jenan Wise added a comment -

Rather than relying on a global, what about if run-tests took a callback? It would differentiate the API from clojure.test but they are already different due to the env arg and nil return value.

Show
Jenan Wise added a comment - Rather than relying on a global, what about if run-tests took a callback? It would differentiate the API from clojure.test but they are already different due to the env arg and nil return value.
Hide
Sebastian Bensusan added a comment - - edited

It is currently in a global to allow easy access from outside cljs (js runners). By having it to pass it to run-tests it would be harder to make reusable test runners from outside cljs/ Said runners would have to know both the ns to run (TBD in each project) and the callback (what we want to reuse, i.e. phantom.exit(0))

Show
Sebastian Bensusan added a comment - - edited It is currently in a global to allow easy access from outside cljs (js runners). By having it to pass it to run-tests it would be harder to make reusable test runners from outside cljs/ Said runners would have to know both the ns to run (TBD in each project) and the callback (what we want to reuse, i.e. phantom.exit(0))
Hide
David Nolen added a comment -

Is there any particular reason to not invoke do-report with a suitable dispatch value instead of adding a new function here?

Show
David Nolen added a comment - Is there any particular reason to not invoke do-report with a suitable dispatch value instead of adding a new function here?
Hide
Sebastian Bensusan added a comment - - edited

I'm not sure that I understand. I thought do-report and report were meant for printing out the summary as a report. This on-testing-complete hook should allow the caller to do whatever he/she wants after all tests are done. Do you want to allow the caller to specify how report works for some new dispatch value by adding:

(defmethod report [::default :on-testing-complete] [m]
..custom-code)

Show
Sebastian Bensusan added a comment - - edited I'm not sure that I understand. I thought do-report and report were meant for printing out the summary as a report. This on-testing-complete hook should allow the caller to do whatever he/she wants after all tests are done. Do you want to allow the caller to specify how report works for some new dispatch value by adding: (defmethod report [::default :on-testing-complete] [m] ..custom-code)
Hide
David Nolen added a comment -

The purpose of report is just side effects, printing is only one possibility. Note there are several testing events that aren't even really used by the standard reporter.

Show
David Nolen added a comment - The purpose of report is just side effects, printing is only one possibility. Note there are several testing events that aren't even really used by the standard reporter.
Hide
Leon Grapenthin added a comment -

IMHO if you want to run anything synchronously before after or between tests, you should use fixtures or if necessary compose the desired blocks yourself.

At least making the latter possible is why I made run-block and the block builder fns public.

You can easily run

(run-block (concat (run-tests-block 'a-ns-to-be-tested 'another-ns-to-be-test )
[(fn []
;; your continuation code here
)]))

This is a reliable way to pick up control after testing.

This patch only affects testing done via run-tests. E. g. on-testing-complete-fn won't be invoked if a user invoked test-ns and testing is complete. You could hack it into the block composer in test-ns, but still users composing blocks in the way mentioned above couldn't rely on it being invoked.

Show
Leon Grapenthin added a comment - IMHO if you want to run anything synchronously before after or between tests, you should use fixtures or if necessary compose the desired blocks yourself. At least making the latter possible is why I made run-block and the block builder fns public. You can easily run (run-block (concat (run-tests-block 'a-ns-to-be-tested 'another-ns-to-be-test ) [(fn [] ;; your continuation code here )])) This is a reliable way to pick up control after testing. This patch only affects testing done via run-tests. E. g. on-testing-complete-fn won't be invoked if a user invoked test-ns and testing is complete. You could hack it into the block composer in test-ns, but still users composing blocks in the way mentioned above couldn't rely on it being invoked.
Hide
Sebastian Bensusan added a comment -

I followed Leon's suggestions and used run-block to make my own test-block. The only problem is that my continuation function doesn't get the summary/env as an argument. From what I understood I could pass env as an argument to run-tests but it would get cleared right before my continuation function. I don't know how to work around this yet.

If you think my solution is a correct use of the cljs.test API then this patch isn't needed. If you believe that run-tests should report on-testing-done I'd be happy to submit a new patch adding a new event to the report event system. Then I could get the summary/env from a custom report method as David suggested.

Show
Sebastian Bensusan added a comment - I followed Leon's suggestions and used run-block to make my own test-block. The only problem is that my continuation function doesn't get the summary/env as an argument. From what I understood I could pass env as an argument to run-tests but it would get cleared right before my continuation function. I don't know how to work around this yet. If you think my solution is a correct use of the cljs.test API then this patch isn't needed. If you believe that run-tests should report on-testing-done I'd be happy to submit a new patch adding a new event to the report event system. Then I could get the summary/env from a custom report method as David suggested.
Hide
David Nolen added a comment - - edited

I would not consider anything at the level of run-block part of the public API nor anything that anyone should be looking into. This stuff may change.

Please add a real reporting hook thanks.

Show
David Nolen added a comment - - edited I would not consider anything at the level of run-block part of the public API nor anything that anyone should be looking into. This stuff may change. Please add a real reporting hook thanks.
Hide
Sebastian Bensusan added a comment -

Added :end-run-test event to cljs.test and a dummy event handler for it.

Show
Sebastian Bensusan added a comment - Added :end-run-test event to cljs.test and a dummy event handler for it.
Hide
Leon Grapenthin added a comment -

For API consistency this should also be in the blocks of `test-all-vars` in `test-ns`, `test-var` and `test-vars`.

Show
Leon Grapenthin added a comment - For API consistency this should also be in the blocks of `test-all-vars` in `test-ns`, `test-var` and `test-vars`.
Hide
David Nolen added a comment -

Happy to see a new ticket + patch that addresses the consistency issue.

Show
David Nolen added a comment - Happy to see a new ticket + patch that addresses the consistency issue.
Hide
Sebastian Bensusan added a comment -

Leon, I filled CLJS-1267 to address your point. Feel free to add any other API inconsistencies that :end-run-tests introduced.

Show
Sebastian Bensusan added a comment - Leon, I filled CLJS-1267 to address your point. Feel free to add any other API inconsistencies that :end-run-tests introduced.

People

Vote (0)
Watch (3)

Dates

  • Created:
    Updated:
    Resolved: