Example of notifying with terminal-notifier.

[stoyle]

One thing I really miss in doo from lein-autoexpect is growl/terminal notifications. So I had a look at doo, and it seems not too hard to support this also in doo.

The idea is this is optional params to the doo command. E.g. lein doo slimer test notify change-only, where says to use the terminal-notifier (could also be growl), and change-only will only notify when the test status actually changes.

This, of course, is not ready to be merged. But before I do any more work, I would like to know if this feature is welcomed.

If so, I can prepare a complete working PR, and any guidance is of course welcome.

Cheers,
Alf

[bensu]

Hi @stoyle,

Thanks for working on this. I'm not yet sure if it is something we should add to lein-doo. I will test it (both from the PR and in lein-autotest) to see how the trade-offs look like and if there is any way to use with lein-doo without adding it to the codebase.

[stoyle]

I don't think lein-autotest does this (I may be wrong though), but I do know lein-test-refresh does.

I do understand doo is complex as it is, and maybe you don't want to add to the complexity. But still I hope this will make it in, in one form or the other. My colleagues and I are often "annoyed" that we don't get notifications when our tests break, since we don't always watch the terminal.

If you do decide to add this feature, and you want help adding it, I will be happy to help you implement it in any way you'd like. If you're uncomfortable adding it directly, one suggestions would be to a plugin of sorts, but it does need to have a hook into here.

Cheers,
Alf

[stoyle]

Added a few more commits. It now "works on my machine" :) Guess I'll have to wait for the CircleCI build to see that I didn't break anything.

Struggled a bit passing the command line args, but this works, preserving the previous arg parsing.

Crossing my fingers you will accept this, or give me pointers on how to make it acceptable. If not, we probably will build our own, and deploy to our internal artifact server.

Cheers,
Alf

[magnars]

Having tried this branch out locally, I have to say it is quite nice. Have been missing this since we switched over to pure cljs-tests.

[bensu]

Hi @stoyle

I found the time to try this on OSX and it works well. Here are some changes to pull out the notifications to the outermost layer of the plugin. I'm not sure how to push them to this PR.

This PR prompts us to making three more changes:

• Support for Ubuntu
• Support for Karma
• Take this argument in unix-like format --notify --changes-only. My initial concern was that lein-doo is now overflowing with options, I didn't not want to add a new one. If we are falling into this slippery slope, we might as well prepare for it.

I just pushed a new version for 0.1.7-SNAPSHOT. If all goes well, next week I'll release 0.1.7 and on that, we can merge this as is and make those changes.

[stoyle]

Hi @bensu.

I understand your concern. Some friends and me have been discussing to create a separate library for this. One which could be configured through leiningen, e.g. .lein/project.clj. This could be made to work across the various testing libs.

What we would need is a "stable" insertion point into the code to extend the functionality, e.g. a multimethod. It could also be done through alter-var-root, or something like that, but that's kind of dirty.

Anyways, I guess it is up to you. I am off to holidays now, so probably won't have time to look at it for a few weeks. Let me know what you decide.

Cheers,
Alf

[bensu]

@stoyle I thought about that as well and that is in part why I decided to move notifier/handle-notification up to the plugin. If you were to make a library for this, how would users configure lein-doo + your library?

[stoyle]

Hi @bensu, ready for some work again ;)

Updated the PR with your change, and rebased it off the current master. Guess you should add a 1.8.0-SNAPSHOT before merging anything? Did a quick test, and everything seems to be working fine.

Before summer I was talking with Anders, who made flare. It basically uses the technique we were thinking about. You can configure it in your .lein/profiles.clj and it will always be on. If we'd want it to not be so static, we could allow overriding it by usind -D modifiers. E.g. -Dnotify-always, -Dnotify-on-change-only or even -Dnever-notify.

Anders was talking about doing this, inspired by my code, since he had some time on his hand. Not sure if he has though... Will ask him if he's had any progress when summer is over.

By the way he is working on making flare work with cljs.test as well, which will be great. It is a lib which minimizes the diffs for all clojure datastructures.

Anyways, how would you like to go forward? The code in this PR does work as expected. If you want it in, go ahead and merge it. If not, we will probably make an external lib for it. In the latter case it would be nice if you gave a stable "external" extension point we could extend doo with.

[torbjornvatn]

Any chance this could get resurrected? Notifications for test runners running in the background is really handy

[stoyle]

I see there are a few conflicts here. If I fix them, any chance it will be merged?

Cheers,
Alf