Navigation Menu

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

Tests aren't rebuilt upon code changes #140

Closed
bmaddy opened this issue Sep 21, 2012 · 10 comments
Closed

Tests aren't rebuilt upon code changes #140

bmaddy opened this issue Sep 21, 2012 · 10 comments
Labels

Comments

@bmaddy
Copy link
Contributor

bmaddy commented Sep 21, 2012

I've noticed that running lein cljsbuild test doens't recompile the test file after a change in the main code, so the tests still pass even if I introduce a bug.

Example:

$ git clone git://github.com/emezeske/lein-cljsbuild.git
$ cd lein-cljsbuild/example-projects/advanced/
$ lein cljsbuild test
...
Test succeeded.
$ # edit src-cljs/example/hello.cljs, change apply + numbers to apply - numbers
$ lein cljsbuild test
...
Test succeeded.

I would expect that test to fail.

@emezeske
Copy link
Owner

emezeske commented Oct 2, 2012

I agree with you in spirit.

Unfortunately, right now the plugin doesn't have any way to know which of the :builds entries a given :test-commands entry depends on to run. So, anytime lein cljsbuild test was run, all of the :builds would have to be compiled first to ensure that no out-of-date code would be tested.

For small projects, with one build and one test target, this would work perfectly. However, for bigger projects, with several builds , this could often result in a large amount of code being needlessly compiled before the tests are run. For instance, my current pet project has two builds, one which takes about 1 second to compile, and another that takes about 20 seconds to compile. If I want to run tests just for the 1 second build, it would be painful to wait while the 20 second build compiled for no reason.

I'm not really sure what the best solution to this is. The first thing that comes to mind would be to explicitly list which :builds a test target depends on, but that's kind of clunky. Do you have any thoughts on possible solutions?

In the meantime, I recommend running lein cljsbuild once, cljsbuild test if you're using Lein version 1.x, or lein do cljsbuild once, cljsbuild test if you're running version 2.x. These will run much faster than the equivalent lein cljsbuild once && lein cljsbuild test because only one instance of the JVM will be started, instead of two.

@cbilson
Copy link

cbilson commented Nov 7, 2012

I am noticing I also have to clean, to get my "production" code included into the test js file. In other words, if I don't clean and lein do cljsbuild once, cljsbuild test, my tests all fail when they try to goog.require my non-test code.

Does this sound right? If not, I can try and produce a simplified repro.

@sfnelson
Copy link
Contributor

How about an explicit :deps to register other build ids, or :source-paths to explicitly register that "test" depends on main code as well as test code. This is a major pain at present, as it makes 'cljsbuild auto' unusable for TDD.

@konrad-garus
Copy link

+1

@zoldar
Copy link

zoldar commented Jan 2, 2013

Hacky solution for that is putting path to the js deployed by "production" into :libs vector in :compiler map of test profile. Not ideal, but does the job.

@emezeske
Copy link
Owner

Issue #173 (which will go out in the 0.3.0 release) added support for :source-paths (note that it's now plural). I think that should ease the pain here a little bit. Let me know if it's helpful!

@zoldar
Copy link

zoldar commented Jan 22, 2013

Looks good to me. At last, no dodgy hacks needed :)

@cemerick
Copy link
Collaborator

Is this still a problem? Trying to assess / triage potentially stale issues here…

@zoldar
Copy link

zoldar commented Aug 29, 2013

It can be closed in my opinion.

@bmaddy
Copy link
Contributor Author

bmaddy commented Aug 29, 2013

Yeah, I think this can be closed too. That pull request would just make things a little more clear for beginners.

@bmaddy bmaddy closed this as completed Aug 29, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

7 participants