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

Support for custom Karma launchers #108

Merged
merged 2 commits into from
Mar 10, 2018
Merged

Support for custom Karma launchers #108

merged 2 commits into from
Mar 10, 2018

Conversation

cdorrat
Copy link

@cdorrat cdorrat commented Jun 11, 2016

I'm using lein-doo to test a clojurescript chrome plugin and I needed to be able to launch chrome with the "--load-extension=target/chrome" command line argument.

This pull request allows you to add new karma launch configurations in the project.clj and run them from lein doo. There's an example in readme.md and the sample project

@bensu
Copy link
Owner

bensu commented Jun 19, 2016

Hi @cdorrat,

Thanks for your work! It is a big PR with many changes, so I'll take some time to review it (I first need to wrap up and release 0.1.7).

@kamituel
Copy link

@cdorrat Have you published your doo fork anywhere for the time being (before this gets merged)? I'd like to start running our unit tests in CI and this would be very helpful. If not, I'll publish it temporarily.

@kamituel
Copy link

Also, on this PR's branch, the following config causes an exception to be thrown when running doo:

  :doo {:paths {:karma "node_modules/.bin/karma"}
        :karma {:config {"reporters" ["progress", "junit"]
                         "plugins" ["karma-junit-reporter"]
                         "junitReporter" {"outputDir" "test-output"}}}}

It can be solved by adding :alias {} to the map above. I guess it could be handled gracefully though.

@cdorrat
Copy link
Author

cdorrat commented Jun 23, 2016

Hi @kamituel, I had noticed the alias issue, I think it was in the 1.7-SNAPSHOT branch before my changes, we should probably raise an issue or fix it in master before the 1.7 release.

I'm happy for you to temporarily push a build of my fork to clojars until the pr is accepted, I'd meant to do it myself but hadn't had time yet.

@kamituel
Copy link

I published a Doo version with this PR as [kamituel/lein-doo "0.1.8-PR-108-1-SNAPSHOT"]. Hope to get rid of it soon, though, once this PR gets merged.

@bensu
Copy link
Owner

bensu commented Jul 3, 2016

@cdorrat can you give me an example of a config that caused problems in master?

@kamituel thank you for pushing to Clojars!

@cdorrat
Copy link
Author

cdorrat commented Jul 4, 2016

Hi @bensu, commenting out the :doo :alias section in the example project will give the following error when running lein doo phantomjs on a checkout of the current master (0.1.7-SNAPSHOT)

Exception in thread "main" java.lang.AssertionError: Assert failed: (map? alias-table)
        at doo.core$resolve_alias.invoke(core.clj:23)
        at leiningen.doo$cli__GT_js_envs.invoke(doo.clj:130)
        at leiningen.doo$doo.doInvoke(doo.clj:201)
        at clojure.lang.RestFn.invoke(RestFn.java:423)
        at clojure.lang.Var.invoke(Var.java:383)
        at clojure.lang.AFn.applyToHelper(AFn.java:156)
        at clojure.lang.Var.applyTo(Var.java:700)
        at clojure.core$apply.invoke(core.clj:632)
        at leiningen.core.main$partial_task$fn__5953.doInvoke(main.clj:261)
        at clojure.lang.RestFn.applyTo(RestFn.java:139)
        at clojure.lang.AFunction$1.doInvoke(AFunction.java:29)
        at clojure.lang.RestFn.applyTo(RestFn.java:137)
        at clojure.core$apply.invoke(core.clj:632)
        at leiningen.core.main$apply_task.invoke(main.clj:311)
        at leiningen.core.main$resolve_and_apply.invoke(main.clj:317)
        at leiningen.core.main$_main$fn__6019.invoke(main.clj:390)
        at leiningen.core.main$_main.doInvoke(main.clj:383)
        at clojure.lang.RestFn.invoke(RestFn.java:421)
        at clojure.lang.Var.invoke(Var.java:383)
        at clojure.lang.AFn.applyToHelper(AFn.java:156)
        at clojure.lang.Var.applyTo(Var.java:700)
        at clojure.core$apply.invoke(core.clj:630)
        at clojure.main$main_opt.invoke(main.clj:316)
        at clojure.main$main.doInvoke(main.clj:421)
        at clojure.lang.RestFn.invoke(RestFn.java:457)
        at clojure.lang.Var.invoke(Var.java:394)
        at clojure.lang.AFn.applyToHelper(AFn.java:165)
        at clojure.lang.Var.applyTo(Var.java:700)
        at clojure.main.main(main.java:37)

@bensu
Copy link
Owner

bensu commented Jul 4, 2016

@cdorrat thanks for the quick response.

@arichiardi also found it and pointed me to the solution in #113 and it is now fixed.

@smogg
Copy link

smogg commented Oct 3, 2016

We encountered the same problem, is this PR still a work-in-progress?

@mulchy
Copy link

mulchy commented Jul 26, 2017

Is this still being worked on?

@miikka miikka merged commit 418fa9b into bensu:master Mar 10, 2018
@miikka miikka mentioned this pull request Mar 10, 2018
6 tasks
@miikka
Copy link
Collaborator

miikka commented Mar 10, 2018

🎉 Merged! 🎉

Thanks for the patch @cdorrat. I have a further improvement proposal in #171 and I hope to create a release soon that will include this change.

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.

6 participants