<< Back to previous view

[NREPL-47] Two deftests with same name cause reduced test coverage Created: 23/Dec/13  Updated: 18/Apr/14  Resolved: 18/Apr/14

Status: Closed
Project: tools.nrepl
Component/s: None
Affects Version/s: None
Fix Version/s: 0.2.4

Type: Defect Priority: Trivial
Reporter: Andy Fingerhut Assignee: Chas Emerick
Resolution: Completed Votes: 0
Labels: None

Attachments: File nrepl-47-v1.diff    

 Description   

There are two deftest's with the name switch-ns in the same namespace. When this happens, the first deftest's tests are ignored, and never run. It is best if all tests are given unique names so this does not happen.

Found using a pre-release version of the Eastwood Clojure lint tool.



 Comments   
Comment by Andy Fingerhut [ 23/Dec/13 6:53 PM ]

Patch nrepl-47-v1.diff simply renames one of the deftest's named switch-ns to switch-ns-2.

Comment by Chas Emerick [ 18/Apr/14 10:06 AM ]

Thanks, fix applied.





[NREPL-51] Pretty-printing reference returned by clojure.tools.nrepl.server/start-server causes multimethod exception Created: 12/Apr/14  Updated: 18/Apr/14  Resolved: 18/Apr/14

Status: Closed
Project: tools.nrepl
Component/s: None
Affects Version/s: 0.2.3
Fix Version/s: 0.2.4

Type: Defect Priority: Minor
Reporter: Michael Nygard Assignee: Chas Emerick
Resolution: Completed Votes: 0
Labels: bug


 Description   

I was accidentally printing the reference returned by start-server by calling it as the final function in my main method, which causes this error:

java.lang.IllegalArgumentException: Multiple methods in multimethod 'print-method' match dispatch value: class clojure.tools.nrepl.server.Server -> interface clojure.lang.IDeref and interface clojure.lang.IRecord, and neither is preferred
at clojure.lang.MultiFn.findAndCacheBestMethod(MultiFn.java:136)
at clojure.lang.MultiFn.getMethod(MultiFn.java:111)
at clojure.lang.MultiFn.getFn(MultiFn.java:119)
at clojure.lang.MultiFn.invoke(MultiFn.java:167)
at clojure.core$pr_on.invoke(core.clj:3266)
at clojure.core$pr.invoke(core.clj:3278)
at clojure.lang.AFn.applyToHelper(AFn.java:161)
at clojure.lang.RestFn.applyTo(RestFn.java:132)
at clojure.core$apply.invoke(core.clj:601)
at clojure.core$prn.doInvoke(core.clj:3311)
at clojure.lang.RestFn.invoke(RestFn.java:408)
at clojure.main$eval_opt.invoke(main.clj:299)
at clojure.main$initialize.invoke(main.clj:316)
at clojure.main$null_opt.invoke(main.clj:349)
at clojure.main$main.doInvoke(main.clj:427)
at clojure.lang.RestFn.invoke(RestFn.java:421)
at clojure.lang.Var.invoke(Var.java:419)
at clojure.lang.AFn.applyToHelper(AFn.java:163)
at clojure.lang.Var.applyTo(Var.java:532)
at clojure.main.main(main.java:37)



 Comments   
Comment by Michael Nygard [ 12/Apr/14 12:09 PM ]

I cloned this from NREPL-37, but cannot change any of the other fields. Thanks Jira.

Basically, this is the same dispatch problem as NREPL-37 but on clojure.pprint/simple-dispatch.

Version affected is 0.2.3.

Comment by Chas Emerick [ 12/Apr/14 12:21 PM ]

Thanks, should have thought of this when I fixed the related issue.

Comment by Chas Emerick [ 18/Apr/14 10:04 AM ]

Committed, will be available in 0.2.4-SNAPSHOT momentarily.





[NREPL-50] Configurable eval function Created: 10/Apr/14  Updated: 18/Apr/14  Resolved: 18/Apr/14

Status: Closed
Project: tools.nrepl
Component/s: None
Affects Version/s: None
Fix Version/s: 0.2.4

Type: Enhancement Priority: Minor
Reporter: Brandon Bloom Assignee: Chas Emerick
Resolution: Completed Votes: 1
Labels: patch, patch,

Attachments: Text File 0001-NREPL-50-Overridable-eval-function.patch    
Patch: Code

 Description   

#'clojure.main/repl has an :eval keyword option that is currently not provided by the interruptable-eval middleware.

I'd like to be able to supply that argument on a per session basis.



 Comments   
Comment by Brandon Bloom [ 10/Apr/14 7:36 PM ]

Attaching patch to support :eval option on (= op :eval) messages.

Comment by Chas Emerick [ 11/Apr/14 7:20 AM ]

This patch adds that option on a per-evaluation basis, which I don't think you really want. Having this set on a per-session basis (as you originally described) makes more sense to me, esp. insofar as doing it that way makes it trivially controllable from "userland" (i.e. the human typing in expressions to be evaluated somewhere). Changing the shape of messages going back and forth requires tool support.

Comment by Brandon Bloom [ 11/Apr/14 9:44 AM ]

Ah, sorry, I should have explained why I ultimately did this at the per-message level. I've got a custom evaluation, but I want to be able to freely mix standard and custom evaluations during the same session. In my case, I have .clj files that use clojure.core/eval and .eclj files that use eclj.core/eval via vim-fireplace. As far as I can tell, Fireplace only opens one session per lein project, so session variables would have been a lot more work.

Is there some way that I can easily override session variables on a per message basis? If so, then I can switch to that to get both the user-level customization and tooling-level support. Otherwise, I'm open to suggestions.

Comment by Chas Emerick [ 12/Apr/14 12:40 PM ]

Sessions are essentially just the result of get-thread-bindings, plus some nREPL bookkeeping. So, as long as evaluate binds e.g. *eval*, users can just set! the var to change it. Though, unless you watch for that in your custom evaluation fn, you'll never be able to change it back. :-P This seems like a good enough reason to keep the option per-message, and require tools/users to ask for what they want each time.

(Tangentially, I just remembered that Piggieback does roughly the same thing you're looking for. I thought about pushing something like what you're suggesting down into nREPL, but I'm very cautious about changing it to suit my needs [myopia and so on]. Glad we have a second real use case, which makes me feel better about the whole thing.)

Comment by Brandon Bloom [ 13/Apr/14 12:09 PM ]

Yeah, set! seems like a surefire way to steamroll over later evaluations unless everybody does a defensive set! all the time, and even then, set! may not even be defined for the evaluator that the caller is expecting!

Maybe this means that there needs to be some sort of general-purpose way to override session variables per message? Then, we'd only need to define eval and all messages could be processed inside their own with-bindings call.

Comment by Chas Emerick [ 14/Apr/14 2:13 PM ]

Maybe, but that's a lot to bite off without having more than "wouldn't it be cool if…" as motivation.

Any thoughts as to whether the eval symbol's namespace should be implicitly {{require}}d?

Will be applying the proposed patch (maybe with a tweak or two) this week.

Comment by Brandon Bloom [ 14/Apr/14 2:23 PM ]

Clojure's standard load-file looks for .class or .clj files. EClj has to patch the loader to look for .eclj files too, and eclj.core may be re-loaded after the new evaluator is bootstrapped.

I have to pre-load the namespace manually either way. Therefore, I don't care if it implicitly requires the namespace, as long as it doesn't :reload the namespace.

Comment by Chas Emerick [ 18/Apr/14 10:04 AM ]

Committed, will be available in 0.2.4-SNAPSHOT momentarily.





Generated at Sat Apr 19 20:29:42 CDT 2014 using JIRA 4.4#649-r158309.