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

Add unit testing support to the Figwheel template. #13

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

rorygibson
Copy link

I thought it'd be nice to include unit testing support by default now that CLJS has a core library for it.
So I did it :-) ....

Includes the necessary configuration in project.clj, plus supporting files, to allow the user to execute lein cljsbuild once test and have unit tests run in PhantomJS if it's installed.

I had to make one small compromise; running the unit tests naturally loads the core.cljs, which prior to my modifications caused Reagent / Om (if used) to trigger, because they were being called directly from the core NS.

I've changed core.cljs and index.html to explicitly call a main function when we're running normally - and not to call it when we're testing (unless the user expliclty does so) which prevents Om / Reagent erroring because they can't find the app root when we're running tests.

(The alternative would have been to add a

to the unit-test.html, but at that point we're taking control of where to root the app away from the developer)

The template now includes the necessary configuration in project.clj,
plus supporting files, to allow the user to execute ```lein cljsbuild
once test``` and have unit tests run in PhantomJS< if it's installed.

Running the unit tests naturally loads the core.cljs, which prior to my
modifications caused Reagent / Om (if used) to trigger, because they were
being called directly from the core NS.

I've changed core.cljs and index.html to
explicitly call a main function when we're running normally - and not to
call it when we're testing (unless the user expliclty does so) which
prevents Om / Reagent erroring because they can't find the app root
when we're running tests.

(The alternative would have been to add a <div id="app"> to the
unit-test.html, but at that point we're taking control of where to root
the app away from the developer)
@bhauman
Copy link
Owner

bhauman commented Mar 13, 2015

Thanks for your work on this.

I have lots of thoughts on this.
One: it should be optional. --testing
Two: the setup and requirements are much simpler if you use the browser to run the tests and use
figwheel's multiple build functionality to run the tests. I don't want to require that people install phantom js for testing when the browser is always installed. This template is meant to be a simple starting point that just works, not an industrial strength template, there is no way to meet all needs.

i.e lein figwheel dev test -- will build both targets as files are changed and push the changes to the appropriate client tabs. Or even multiple clients Safari, Chrome, iOS, Angdroid etc.

The tests will be running in a test.html and perhaps have a ^:figwheel-always on the test runner cljs file.

It is even simpler to have a test-runner that gets loaded into the same env (tab) as your running app on every save by puttting a :figwheel-always on the test runner ns and adding the test path to the :source-paths of the dev build. This eliminates the need for a separate build and a test html.

I know people want to just run a lein test and get a result, but that is a separate concern, to getting feedback from testing.

I'm going to reflect on this more I would appreciate your thoughts on these options as well.

@devth
Copy link

devth commented May 12, 2016

While running tests in the browser is great for dev time, it's almost always necessary to be able to run tests from CLI as well. I'm currently trying to figure out how to do this.

@bensu
Copy link
Contributor

bensu commented May 12, 2016

Hi @rorygibson

Have you tried that setup with async tests?

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

Successfully merging this pull request may close these issues.

None yet

4 participants