borkdude / speculative

Unofficial community-driven specs for clojure.core
Eclipse Public License 1.0
187 stars 16 forks source link

Problem with merge-with in clojure.test/run-tests #146

Open borkdude opened 5 years ago

borkdude commented 5 years ago

Speculative has a spec for merge-with. In https://github.com/borkdude/advent-of-spec/tree/master/src/aos I'm running tests with instrumentation on, but even when I make deliberate mistakes in the second test ns, no fdef is triggered. I narrowed it down to this expression: https://github.com/clojure/clojure/blob/master/src/clj/clojure/test.clj#L775

(apply merge-with + (map test-ns namespaces))

So tests are currently running as a side effect in a lazy expression.

The spec-checking-fn conforms the args of merge-with within the scope of a with-instrument-disabled (https://github.com/clojure/spec.alpha/blob/master/src/main/clojure/clojure/spec/test/alpha.clj#L139). As a result my tests are executed (during the realization of the lazy seq in run-tests), while instrumentation is turned off.

Solution: make test running strict instead of lazy in clojure.test. So something like: (let [test-results (mapv test-ns namespaces), summary (apply + merge-with test-results)] ...)

I’ll write a JIRA issue + patch if that’s needed after some discussion on Slack #clojure-dev. This doesn't affect ClojureScript testing.

borkdude commented 5 years ago

Repro:

(ns repro
  (:require
   [clojure.spec.alpha :as s]
   [clojure.spec.test.alpha :as stest]))

(defn my-fn [x]
  (println "called my-fn with type" (type x))
  (println (deref #'clojure.spec.test.alpha/*instrument-enabled*))
  {x 1})

(s/fdef my-fn :args (s/cat :x symbol?))

(defn my-merge-with [f & maps]
  (apply merge-with f maps))

(s/fdef my-merge-with :args
        (s/cat :f ifn? :maps (s/* map?)))

(defn my-run-tests [& namespaces]
  (apply my-merge-with + (map my-fn namespaces)))

(stest/instrument)

(defn run []
  (my-run-tests 'foo "bar"))

(run)

Output:

called my-fn with type clojure.lang.Symbol
true
called my-fn with type java.lang.String
nil

Note that my-fn didn't throw when called with a String.

borkdude commented 5 years ago

See https://dev.clojure.org/jira/browse/CLJ-2443