bensu / doo

doo is a library and lein plugin to run cljs.test on different js environments.
Eclipse Public License 1.0
324 stars 63 forks source link

Support custom command path / unoptimized test builds #68

Closed crisptrutski closed 8 years ago

crisptrutski commented 8 years ago

Having some trouble using 0.1.6 with unoptimised CLJS builds - and the issue seems to be http://dev.clojure.org/jira/browse/CLJS-1444.

Until that is sorted out, such builds would need to be run from the :output-to directory.

Although this has its own problems, e.g. for commands only installed locally for the project, I can't think of another solution. On the flip-side this has advantages too (ie. support for non-global commands outside the project)

danielcompton commented 8 years ago

@crisptrutski we're doing ok running :optimizations :none on our projects. What problems are you having specifically?

crisptrutski commented 8 years ago

When using Boot compiled files are created in a temp directory, not at the assumed relative path to the project. That means that require statements for Closure generated by the CLJS shim for :none use incorrect paths, and the bootstrapping fails:

Running cljs tests...ClojureScript could not load :main, did you forget to specify
   :asset-path?
ERROR: doo was not loaded from the compiled script.

This is precisely the scenario mentioned in the linked CLJS ticket.

You could run the tests here against doo with and without this branch to see the difference.

This is a blocker for upgrading Boot tooling to use the latest doo (0.1.4 did not have this problem), but open to any solution - admit this one is not pretty.

danielcompton commented 8 years ago

Gotcha, that makes more sense. I'll have a look at how boot does it and see what I can think of too.

bensu commented 8 years ago

@crisptrutski CLJS-1444 deals with a node-only problem. Is the problem you are reporting also isolated to node?

crisptrutski commented 8 years ago

@bensu no, it's actually been affecting phantom in my testing - looks like I didn't read the ticket closely enough.

Here's the actual shim that's been generated:

var CLOSURE_UNCOMPILED_DEFINES = null;
if(typeof goog == "undefined") document.write('<script src="output.out/goog/base.js"></script>');
document.write('<script src="output.out/cljs_deps.js"></script>');
document.write('<script>if (typeof goog != "undefined") { goog.require("boot.cljs.main750"); } else { console.warn("ClojureScript could not load :main, did you forget to specify :asset-path?"); };</script>');
bensu commented 8 years ago

Thanks @crisptrutski

To solve this I'll need a better understanding of boot-cljs. Luckily I've already been using it in one project but it will take a while until we figure out the best way to re conciliate these two execution models.

crisptrutski commented 8 years ago

Totally understand you wanting to avoid a knee-jerk solution!

Would you be open to a scaled back PR in the meantime, which just provides a "command-dir" option, and skipping any such magic around the compiler options? This would be enough for me to cut a boot-cljs-test release, without any implicit magic.

If not, I can wait - and maybe find the time to help :smile:

crisptrutski commented 8 years ago

Since @danielcompton made the same suggestion, I've gone ahead and created the PR :v: :palm_tree: