weavejester / compojure

A concise routing library for Ring/Clojure
Eclipse Public License 1.0
4.09k stars 256 forks source link

Code coverage with cloverage suddenly failing with 1.3.2 #135

Closed jvah closed 9 years ago

jvah commented 9 years ago

I haven't investigated this further yet, and I'm not sure if this is a problem with compojure, clout or cloverage, but I suspect this might be related to commit 639fffd3f8b77153644d0c9b87bca5af56be0cb0.

ERROR in (users-test) (core_deftype.clj:544)
Uncaught exception, not in assertion.
expected: nil
  actual: java.lang.IllegalArgumentException: No implementation of method: :route-matches of protocol: #'clout.core/Route found for class: clojure.lang.PersistentArrayMap
 at clojure.core$_cache_protocol_fn.invoke (core_deftype.clj:544)
    clout.core$eval5420$fn__5421$G__5411__5428.invoke (core.clj:39)
    compojure.core$if_route$fn__5712.invoke (core.clj:40)
    compojure.core$if_method$fn__5704.invoke (core.clj:27)
    compojure.core$routing$fn__5743.invoke (core.clj:127)
    clojure.core$some.invoke (core.clj:2515)
    compojure.core$routing.doInvoke (core.clj:127)
    clojure.lang.RestFn.applyTo (RestFn.java:139)
    clojure.core$apply.invoke (core.clj:626)
    compojure.core$routes$fn__5747.invoke (core.clj:132)
    ring.middleware.json$wrap_json_response$fn__5872.invoke (json.clj:65)
    ring.middleware.json$wrap_json_body$fn__5855.invoke (json.clj:35)
    creal_backend.handler_test$fn__6388$fn__6389.invoke (handler_test.clj:23)
    creal_backend.handler_test/fn (handler_test.clj:21)
    clojure.test$test_var$fn__7187.invoke (test.clj:704)
    clojure.test$test_var.invoke (test.clj:704)
    clojure.test$test_vars$fn__7209$fn__7214.invoke (test.clj:722)
    creal_backend.db_utils$db_fixture.invoke (db_utils.clj:41)
    clojure.test$compose_fixtures$fn__7181$fn__7182.invoke (test.clj:681)
    clojure.test$default_fixture.invoke (test.clj:674)
    clojure.test$compose_fixtures$fn__7181.invoke (test.clj:681)
    clojure.test$test_vars$fn__7209.invoke (test.clj:722)
    clojure.test$default_fixture.invoke (test.clj:674)
    clojure.test$test_vars.invoke (test.clj:718)
    clojure.test$test_all_vars.invoke (test.clj:728)
    clojure.test$test_ns.invoke (test.clj:747)
    clojure.core$map$fn__4245.invoke (core.clj:2557)
    clojure.lang.LazySeq.sval (LazySeq.java:40)
    clojure.lang.LazySeq.seq (LazySeq.java:49)
    clojure.lang.Cons.next (Cons.java:39)
    clojure.lang.RT.boundedLength (RT.java:1654)
    clojure.lang.RestFn.applyTo (RestFn.java:130)
    clojure.core$apply.invoke (core.clj:626)
    clojure.test$run_tests.doInvoke (test.clj:762)
    clojure.lang.RestFn.applyTo (RestFn.java:137)
    clojure.core$apply.invoke (core.clj:624)
    cloverage.coverage$_main.doInvoke (coverage.clj:153)
    clojure.lang.RestFn.invoke (RestFn.java:1523)
    clojure.lang.Var.invoke (Var.java:519)
    user$eval5.invoke (form-init2558567407060657676.clj:1)
    clojure.lang.Compiler.eval (Compiler.java:6703)
    clojure.lang.Compiler.eval (Compiler.java:6693)
    clojure.lang.Compiler.load (Compiler.java:7130)
    clojure.lang.Compiler.loadFile (Compiler.java:7086)
    clojure.main$load_script.invoke (main.clj:274)
    clojure.main$init_opt.invoke (main.clj:279)
    clojure.main$initialize.invoke (main.clj:307)
    clojure.main$null_opt.invoke (main.clj:342)
    clojure.main$main.doInvoke (main.clj:420)
    clojure.lang.RestFn.invoke (RestFn.java:421)
    clojure.lang.Var.invoke (Var.java:383)
    clojure.lang.AFn.applyToHelper (AFn.java:156)
    clojure.lang.Var.applyTo (Var.java:700)
    clojure.main.main (main.java:37)
weavejester commented 9 years ago

A stacktrace without source code isn't that useful, I'm afraid. I can't tell much without knowing what line of code produced the exception.

jvah commented 9 years ago

See the test case in https://github.com/jvah/compojure-cloverage-crash.

If I run lein test everything passes but lein cloverage crashes, so I'm suspecting this might actually be a bug in how cloverage instruments the code. The following patch would seem to fix the issue, but it's up to you to decide if this is a compojure problem or a problem somewhere else:

From 2a5404c85a08cd026f697bd7ee71f558dad2aeca Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Juho=20V=C3=A4h=C3=A4-Herttua?=
 <juho.vaha-herttua@futurice.com>
Date: Sun, 1 Mar 2015 09:07:23 +0200
Subject: [PATCH] Reintroduce syntax quote in clout route compilation.

---
 src/compojure/core.clj | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/src/compojure/core.clj b/src/compojure/core.clj
index df35656..58b3d23 100644
--- a/src/compojure/core.clj
+++ b/src/compojure/core.clj
@@ -48,11 +48,11 @@
 (defn- prepare-route [route]
   (cond
     (string? route)
-      (clout/route-compile route)
+      `(clout/route-compile ~route)
     (and (vector? route) (literal? route))
-      (clout/route-compile
-       (first route)
-       (apply hash-map (rest route)))
+      `(clout/route-compile
+        ~(first route)
+        ~(apply hash-map (rest route)))
     (vector? route)
       `(clout/route-compile
         ~(first route)
@@ -201,11 +201,11 @@
   (let [re-context {:__path-info #"|/.*"}]
     (cond
       (string? route)
-        (clout/route-compile (str route ":__path-info") re-context)
+       `(clout/route-compile ~(str route ":__path-info") ~re-context)
       (and (vector? route) (literal? route))
-        (clout/route-compile
-         (str (first route) ":__path-info")
-         (merge (apply hash-map (rest route)) re-context))
+       `(clout/route-compile
+         (str ~(first route) ":__path-info")
+         ~(merge (apply hash-map (rest route)) re-context))
       (vector? route)
        `(clout/route-compile
          (str ~(first route) ":__path-info")
--
1.9.3 (Apple Git-50)
weavejester commented 9 years ago

Looks like this is an issue with Cloverage. I can't apply the patch you've provided, because it would revert the performance gains in Compojure 1.3.2.

jvah commented 9 years ago

Just wanted to get a confirmation that there should be nothing wrong with compojure here. Indeed after looking at the code more thoroughly it seems like a cloverage bug, there should be nothing wrong with running the route-compile at compile-time instead of runtime. The discussion is moved to cloverage repository issue, thank you for your help.

MichaelBlume commented 9 years ago

@weavejester I have a workaround that doesn't revert the performance gains in 1.3.2, if you're interested

weavejester commented 9 years ago

Isn't this a cloverage bug?

MichaelBlume commented 9 years ago

It absolutely is, I'm just kind of worried that Cloverage might have been abandoned and I'm not quite sure what to do about that.

MichaelBlume commented 9 years ago

https://github.com/MichaelBlume/cloverage-compojure-fix