Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Adding :with-repl option to start a clojure.main/repl in-between test execution #50

Merged
merged 4 commits into from
Feb 26, 2016

Conversation

Pancia
Copy link
Contributor

@Pancia Pancia commented Feb 16, 2016

This isn't necessarily finished, comments/docstrings/README, but I wanted to see if you were receptive to the idea. Let me know if/what you want me to write to finish this up.

The current use case is for changing timbre's log level & ns filters, and it seems like a repl is not all that intrusive.

Either way, awesome tool you got here, thanks!

Anthony D'Ambrosio added 2 commits February 16, 2016 13:47
without it when enter is pressed on an empty (or just whitespace) line
@@ -12,6 +12,6 @@
:username :gpg :password :gpg}]
["releases" {:url "https://clojars.org/repo"
:username :gpg :password :gpg}]]
:profiles {:dev {:dependencies [[org.clojure/clojure "1.6.0"]]}}
:profiles {:dev {:dependencies [[org.clojure/clojure "1.7.0"]]}}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I kept getting warnings about some of my plugins not running with 1.6.0.

@jakemcc
Copy link
Owner

jakemcc commented Feb 22, 2016

Interesting idea. Definitely something I hadn't thought of before. I haven't had a chance to play around with it or think about it too much yet (just got back from a vacation) but definitely will over the next couple of days.

To summarize, the basic idea is to expose an option that starts a repl and runs tests automatically. Having a repl exposed is useful for changing state that isn't blown away by refreshes (such as modifying timbre's logging levels?).

Is that summary correct?

@Pancia
Copy link
Contributor Author

Pancia commented Feb 22, 2016

Sounds right, often I'll want to change the log level to debug (from warn) after seeing a failure and re run the tests.

@jakemcc
Copy link
Owner

jakemcc commented Feb 23, 2016

Great.

Having thought about it a bit and conferred with a couple other people on it and I'm cool with proceeding with this feature. Its interesting and something I never even considered.

Could you update README to add some text to the feature list near the top and a more detailed section later with a description of the feature and motivation and put something in CHANGES.md (add a NEXT section at top with a sentence).

If there is anything else you feel needs to be added feel free to do it.

Thanks for the PR and putting in the work.

Anthony D'Ambrosio added 2 commits February 23, 2016 11:01
@Pancia
Copy link
Contributor Author

Pancia commented Feb 23, 2016

Ok I think it's all good to go, let me know if you want anything else added/changed.

I'm thinking as a later endeavor I may try to integrate tab-completion and history (arrow navigation), maybe using trptcolin/reply.
For now using rlwrap lein test-refresh :with-repl should suffice for most uses cases of this anyway.

@Pancia Pancia closed this Feb 23, 2016
@Pancia Pancia reopened this Feb 23, 2016
@Pancia
Copy link
Contributor Author

Pancia commented Feb 23, 2016

Woops didn't mean to close it....

@jakemcc jakemcc merged commit 45aa3fc into jakemcc:master Feb 26, 2016
@jakemcc
Copy link
Owner

jakemcc commented Feb 26, 2016

Thanks for the submission. Merged. Will deploy shortly.

@jakemcc
Copy link
Owner

jakemcc commented Feb 26, 2016

Version [com.jakemccrary/lein-test-refresh 0.14.0] released with this feature.

Enjoy.

@donbonifacio
Copy link
Contributor

Really like this!

Is it possible to expose an nrepl port? This way my editor could easily jack in.

@jakemcc
Copy link
Owner

jakemcc commented May 4, 2016

@donbonifacio Probably requires pulling in another dependency to get nrepl working. If you think it would be useful feel free to give it a shot and then try it out for a while.

I'm not sure I'd want a repl that automatically refreshes namespaces as you'll end up losing state (not that having lots of state around in a repl is great anyway) whenever a namespace refreshes.

If you do it and find it useful I'd be willing to accept a PR. I'm also not sure how this changes with socket repl changes in 1.8 (if tooling eventually moves to use that instead of nrepl, I'm less up-to-date with various toolings thoughts on that).

@donbonifacio
Copy link
Contributor

In our scenario we don't have state on the tests, and we rely on the reply when we want to run just a specific test/code. So this would work fine for me. I'll try to investigate what's needed to jack in. :)

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants