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

Weird case of doo not respecting :source-paths in profile #81

Closed arichiardi closed 8 years ago

arichiardi commented 8 years ago

Hello Bensu!

I apologise if it is going to be my mistake, but this is really odd:

https://github.com/ScalaConsultants/replumb/issues/72

The two doo different commands execute the same tests??

I happens only on Travis and not locally which makes it even more difficult to debug.

bensu commented 8 years ago

Hi @arichiardi

From what I can tell you are using the same launcher (launcher.runner) for both test-node* and test-browser*. launcher.runner then requires a set of namespaces and ClojureScript will compile only those namespaces. When you call doo-run-all, that's all you are going to see.

Try using more launchers with different required namespaces.

arichiardi commented 8 years ago

Yes it is exactly like this, but namespaces are provided in the source path or not, for instance require_test in src/browser is now empty whether in src/node is full. And everything works locally, there probably is something wrong in Travis' build.

arichiardi commented 8 years ago

Weird...it looks like :source-paths are not really respected per profile while build. For instance my :browser-test has ["src/cljs" "src/browser" "test/cljs" "test/browser"] and :node-test has ["src/cljs" "src/node" "test/cljs" "test/node"] but then I get that require is not found, meaning that is actually putting together all the namespaces..puzzling...maybe I did not understand how doo compiles the tests...

https://github.com/ScalaConsultants/replumb/blob/c5fd920f631e959619f76b0d1bff6c06ddb3dcef/project.clj

arichiardi commented 8 years ago

It might even just be cljsbuild actually so I should probably exclude that as cause.

arichiardi commented 8 years ago

FYI (scroll to bottom):

Build without my browser-test builds section: https://travis-ci.org/ScalaConsultants/replumb/builds/96525991

Build with it re-added: https://travis-ci.org/ScalaConsultants/replumb/builds/96526301

Odd

arichiardi commented 8 years ago

Forget about cljsbuild, I saw that lein-doo actually lives without. I will investigate more.

arichiardi commented 8 years ago

So this is were things get weird. We are merging everything from every profile...in my opinion we should just compile and execute the profile indicated with {build-id}. Am I right? Will I break everything (yes!) ? :+1:

arichiardi commented 8 years ago

I renamed the files and this is a workaround for that. I decided to wait for an answer before diving into some fix as there might be other implications that I ignore...

bensu commented 8 years ago

Hi @arichiardi

I looked at this and saw no obvious problem. I will look into it soon. I think it's related to a misunderstanding of what the make-subproject does (add all :source-paths to the classpath) and what ClojureScript does when you require namespaces and call doo-run-tests.

If you want to run different namespaces for different build-ids then you should have a different runner, that requires the different namespaces. Otherwise (if you have only one runner) ClojureScript will require an compile the same namespaces for different build-ids.

arichiardi commented 8 years ago

Hey @bensu, thanks for taking the time for this and sorry I maybe did not explain my problem, I will put it in a better (hopefully) form below.

In my mind I thought I could have two folders for two different platforms:

test/browser/replumb/require_test.cljs
test/node/replumb/require_test.cljs

I was not worried about the clash because I am discerning them using :source-paths and the build id:

    {:id "browser-test"
       :source-paths ["src/cljs" "test/cljs" "test/browser"]
       :compiler {...
                        :output-to "dev-resources/private/test/browser/compiled/browser-test.js" }} 
    {:id "node-test"
        :source-paths ["src/cljs" "src/node" "test/cljs" "test/node"]
        :compiler {
                        :output-to "dev-resources/private/test/node/compiled/nodejs-test.js"}}

At this point (always in my mind) it does not matter how many runners I have, I know that lein doo phantom browser-test once will never look into src/node and test/node.

Unfortunately this does not happen in doo at the moment, because of make-subprobject (if I read it right): everything is in the classpath and of course I have a clash.

I used your workaround in replumb (renaming the files and having two runners), it is just that I was wondering whether we can improve doo on this :smile:

bensu commented 8 years ago

Sounds reasonable. We should then add the source-paths to the class-path selectively depending on which build-id is being called. I'll try it out and see if anything major breaks.

arichiardi commented 8 years ago

Yes I was trying exactly that, only make-subpoject (and of course it's caller) needs to be changed ;)

arichiardi commented 8 years ago

I also opened another (I hope reasonable) PR that I found necessary in replumb

bensu commented 8 years ago

Hi @arichiardi

Do you mind trying the fix? I deployed it to clojars under lein-doo-0.1.7-SNAPSHOT which resolved to lein-doo-0.1.7-20151215.193410-2.

arichiardi commented 8 years ago

Yep, I kept a branch on replumb exactly for that!

arichiardi commented 8 years ago

It works correctly (the test count is different). There is a dummy test failing in the node version of require_test.cljs, same namespace for node and browser tests. I will switch to the old "style" (new for doo) testing for release 0.1.7.

bensu commented 8 years ago

Glad it works!