dynamic binding for clojure.test/junit-report (1.3-beta1)

[pmbauer]

Background:

Yack shaving, alioth benchmarks, update pom, build failure due to ...

The latest Clojure 1.3 beta defaults all vars to static binding.

The clojure-maven-plugin depends on clojure.test/junit-report being dynamically bound.

Problem:

The clojure-maven-plugin cannot be used with Clojure 1.3

Solution(s):

A) Explicitly make junit-report dynamic. (see attached patch)

B) Alter clojure-maven-plugin, although I do not know an alternative way to accomplish how the plugin uses junit-report without dynamic binding.

Rational

It's reasonable for a non-performance critical reporting function to be dynamic, see clojure.test/report (also dynamic)

Concerns:

Why have dynamically bindable vars they aren't dynamic by default? (yes, yes, that ship has sailed, but applying these :dynamic metadatas is somewhat ... arbitrary)

[pmbauer]

typo:

s/vars they/vars if they/g

P.S.

This needs addressed one way or the other before clojure 1.3 is released.

Direction needed before filling a ticket.

[Meikel Brandmeyer]

Hi,

Am Mittwoch, 27. Juli 2011 06:21:34 UTC+2 schrieb pmbauer:

 

Why have dynamically bindable vars they aren't dynamic by default? (yes, yes, that ship has sailed, but applying these :dynamic metadatas is somewhat ... arbitrary)

This follows the usual strategy: make the fast thing the default. If you need special handling, you have to name it. I think the number operations follow this also, AFAIU. The idea is to think about what you are doing: "Does this Var have to be dynamic?" And then pay the price for it.

The problem is with such legacy code, that this process is not always properly done. With new code it should not be thaaaat arbitrary.

Sincerely
Meikel

[Alex Miller]

I think the counter-argument is that it takes away some of the
wonderful flexibility of Clojure in a way that external users of the
code cannot control or get back. For generic libraries intended to be
reused by others, it's quite possible that many of the vars could
*potentially* need to be dynamic for the purposes of testing, or code
coverage tools, or profilers, or something we can't think of now.

But, I think the decision has been made and I don't have any evidence
that any of the above theoretical argument actually matters in
practice. I'm interested to see whether this is actually an issue or
not.

> --
> You received this message because you are subscribed to the Google Groups
> "Clojure Dev" group.
> To view this discussion on the web visit
> https://groups.google.com/d/msg/clojure-dev/-/CDOBYxwQW6gJ.
> To post to this group, send email to cloju...@googlegroups.com.
> To unsubscribe from this group, send email to
> clojure-dev...@googlegroups.com.
> For more options, visit this group at
> http://groups.google.com/group/clojure-dev?hl=en.
>

[pmbauer]

Well, with primitives parameters and return types, it is the opposite, no?

Muddying the decision further, binding semantics for ClojureScript are different entirely - dynamic with no metadata annotation.

[Stuart Halloway]

I think the counter-argument is that it takes away some of the
wonderful flexibility of Clojure in a way that external users of the
code cannot control or get back.  For generic libraries intended to be
reused by others, it's quite possible that many of the vars could
*potentially* need to be dynamic for the purposes of testing, or code
coverage tools, or profilers, or something we can't think of now.

It is important to separate out redef vs. rebind. Redef is available to any var, and serves the common need in testing.

Also worthy to note that all your examples are dev environment-related. We don't want to compromise production for dev, and one can imagine enabling dev-mode capabilities separately.

But, I think the decision has been made and I don't have any evidence
that any of the above theoretical argument actually matters in
practice.  I'm interested to see whether this is actually an issue or
not.

Well said! There are an infinite number of nice-to-haves, but they aren't all compatible with each other.

Stuart Halloway
Clojure/core
http://clojure.com

[pmbauer]

"redef vs. rebind"

Thanks.

I rechecked the default_test_script.clj in the plugin.

With the manner in which it wants to wrap junit-report, redefining should address this.

I'll submit a patch there instead.

[pmbauer]

I spoke too soon.

Trying to redefine junit-report:

(def results (atom []))

(def junit-report-orig junit-report)

(defn junit-report [x]

  (junit-report-orig x)

  (swap! results conj (:type x)))

Produces ...

Exception in thread "main" java.lang.IllegalStateException: junit-report already refers to: #'clojure.test.junit/junit-report in namespace: com.theoryinpractise.clojure.testrunner (run-test7881998067428815087.clj:24)

So redefinition isn't going to work in this case, or am I missing something?

[pmbauer]

"We don't want to compromise production for dev"

In the case of clojure.test et al, dev is production.

[Stuart Halloway]

Redefinition has to happen in the original namespace.

And you are working too hard. Core already has with-redefs.

All that said, this scenario calls for dynamic binding (or an atom). You aren't hacking a fn to stub it out for testing, you are registering a replacement fn in a way intended by the original lib author.

Stu

[pmbauer]

Thanks for bearing with me, my knowledge of Clojure is still obviously growing.

So should I open a JIRA ticket with the attached patch?

[Stuart Halloway]

Sure.