gfredericks / vcr-clj

Generic IO playback for clojure
117 stars 20 forks source link

don't print on the output to avoid tooling issues #3

Closed wilkerlucio closed 10 years ago

wilkerlucio commented 10 years ago

While using CIDER I was having issues because of those prints, the cider-test-mode was having issues running the tests because of them. So I imported my own copy and tried removing then, after that it worked fine :)

wilkerlucio commented 10 years ago

also, after working a bit more I found another issue, I'm not sure why, but the data_readers.clj were not being picked up, I fixed that by adding a link for it on project root, I saw other projects doing that so I would guess it's something version related, anyhow, after those changes everything is working great here :)

gfredericks commented 10 years ago

Thanks!

This sounds more like a bug with cider than with vcr-clj. I don't think tooling should ever be sensitive to tests printing. Though I might be sympathetic to an argument that it's distracting and unhelpful for vcr-clj to be printing at all.

For working around tooling issues, I would recommend something like:

(let [v #'vcr-clj.core/with-cassette-fn*]
  (when-not (-> v meta ::patched?)
    (alter-var-root v (fn [f] (fn [& args] (binding [*out* (clojure.java.io/writer "/dev/null")] (apply f args)))))
    (alter-meta! v assoc ::patched? true)))

As for the data_readers.clj issue, I'm interested in the details of what you were doing such that that helped -- as far as I know the only requirement for data_readers.clj is that it be on the root of the classpath, and the root of the project is not on the classpath in any tooling setup I know of.

wilkerlucio commented 10 years ago

Hi @gfredericks

Yeah, I agree that tooling should not shape the printing, but I also agree that would be better to not have any printing at all (unless maybe with some verbose option, but not as default).

In order to work and test the code I just manually copied the VCR source into my own sources and added the required dependencies on my project.

The thing is, after removing the printing I still got the error from CIDER (it happens that when I thought it was fixed, I was actually missing the usave...), so I started thinking that the printing wasn't the initial problem at all...

So most of times I got an helpless error message from CIDER saying something about "error: End of buffer", but once I got an error saying it wasn't able to find the data readers.

So I looked up a bit and somehow I found this repo here: https://github.com/LonoCloud/lein-voom and saw him making this link on the root. I tried the same here on my project and it made it work.

I'm not sure if that's CIDER related, Lein related or Clojure version related... But I think that was the initial issue after all. Maybe on new version of something, the classpath is defined as the project root and not the src folder, which would require moving the data_readers.clj down to project root, that's exactly how I managed to make it work on my project, but I'm not 100% if the same applies on dependencies (since here I was using everything direct on my project).

gfredericks commented 10 years ago

Yeah the symlink should definitely have no bearing on normal use of vcr-clj, since it wouldn't get packed into the jarfile that you end up using. So I can't think that this was your original problem.

Perhaps the original problem was printing-related, but once you copied the source into your project it became about the data_readers.clj?

I'll make a modified version of your first commit that defaults to no printing, and I'll push up a version of that to clojars so you can try it out.

wilkerlucio commented 10 years ago

@gfredericks cool, thank you :+1:

gfredericks commented 10 years ago

Okay, try version 0.3.4-alpha-503e4e4.

wilkerlucio commented 10 years ago

@gfredericks didn't worked...

And now I'm pretty sure it's the data readers, that's because if I do keep the data_readers.clj on my root it works, if I remove it, it fails.

That's a requirement? If so I think we should add it on the README

gfredericks commented 10 years ago

Do you still have the vcr-clj sources in your project? Otherwise what does your symlink even point to?

wilkerlucio commented 10 years ago

@gfredericks no, I deleted all the sources, removed the dependencies and added only the vcr-clj dependency.

gfredericks commented 10 years ago

So you're saying your symlink is broken, but the tests only run if it's there?

wilkerlucio commented 10 years ago

no, it's like that, imagine a clean new project, then you just add this to your project.clj:

:profiles {:dev {:dependencies [[com.gfredericks/vcr-clj "0.3.4-alpha-503e4e4"]]}}

If I only do that, it doesn't work (it fails when tries to read).

But I can fix that by creating the following file on my project root:

data_readers.clj

{vcr-clj/bytes vcr-clj.cassettes/str->bytes
 vcr-clj/input-stream vcr-clj.clj-http/read-input-stream}

So, it's like it's not getting the setup from the dependency.

gfredericks commented 10 years ago

Can you check if the tests also fail to run with lein test? I.e., removing cider from the picture.

wilkerlucio commented 10 years ago
[node-webkit-build] ls                                                                                                                                                                                                                                                                                                            11:18:44 [ruby-2.0.0]  ☁  master ☂ ✖ ⚡
#project.clj#    LICENSE          README.md        cassettes        data_readers.clj project.clj      src              target           test
[node-webkit-build] lein test                                                                                                                                                                                                                                                                                                     11:18:51 [ruby-2.0.0]  ☁  master ☂ ✖ ⚡

lein test node-webkit-build.core-test

Ran 2 tests containing 2 assertions.
0 failures, 0 errors.
[node-webkit-build] rm data_readers.clj                                                                                                                                                                                                                                                                                           11:19:01 [ruby-2.0.0]  ☁  master ☂ ✖ ⚡
[node-webkit-build] lein test                                                                                                                                                                                                                                                                                                     11:19:04 [ruby-2.0.0]  ☁  master ☂ ✖ ⚡

lein test node-webkit-build.core-test

lein test :only node-webkit-build.core-test/listing-versions

ERROR in (listing-versions) (LispReader.java:1180)
Uncaught exception, not in assertion.
expected: nil
  actual: java.lang.RuntimeException: No reader function for tag vcr-clj/bytes
 at clojure.lang.LispReader$CtorReader.readTagged (LispReader.java:1180)
    clojure.lang.LispReader$CtorReader.invoke (LispReader.java:1164)
    clojure.lang.LispReader$DispatchReader.invoke (LispReader.java:609)
    clojure.lang.LispReader.readDelimitedList (LispReader.java:1138)
    clojure.lang.LispReader$MapReader.invoke (LispReader.java:1081)
    clojure.lang.LispReader.readDelimitedList (LispReader.java:1138)
    clojure.lang.LispReader$MapReader.invoke (LispReader.java:1081)
    clojure.lang.LispReader.readDelimitedList (LispReader.java:1138)
    clojure.lang.LispReader$VectorReader.invoke (LispReader.java:1073)
    clojure.lang.LispReader.readDelimitedList (LispReader.java:1138)
    clojure.lang.LispReader$MapReader.invoke (LispReader.java:1081)
    clojure.lang.LispReader.read (LispReader.java:183)
    clojure.lang.RT.readString (RT.java:1737)
    clojure.core$read_string.invoke (core.clj:3497)
    vcr_clj.cassettes$read_cassette.invoke (cassettes.clj:40)
    vcr_clj.core$with_cassette_fn_STAR_.invoke (core.clj:114)
    node_webkit_build.core_test/fn (core_test.clj:8)
    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)
    clojure.test$default_fixture.invoke (test.clj:674)
    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:2559)
    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)
    user$eval2843$fn__2898$fn__2929.invoke (NO_SOURCE_FILE:-1)
    user$eval2843$fn__2898$fn__2899.invoke (NO_SOURCE_FILE:0)
    user$eval2843$fn__2898.invoke (NO_SOURCE_FILE:0)
    user$eval2843.invoke (NO_SOURCE_FILE:0)
    clojure.lang.Compiler.eval (Compiler.java:6703)
    clojure.lang.Compiler.eval (Compiler.java:6693)
    clojure.lang.Compiler.eval (Compiler.java:6666)
    clojure.core$eval.invoke (core.clj:2927)
    leiningen.core.eval/fn (eval.clj:314)
    clojure.lang.MultiFn.invoke (MultiFn.java:231)
    leiningen.core.eval$eval_in_project.invoke (eval.clj:337)
    leiningen.test$test.doInvoke (test.clj:195)
    clojure.lang.RestFn.invoke (RestFn.java:410)
    clojure.lang.Var.invoke (Var.java:379)
    clojure.lang.AFn.applyToHelper (AFn.java:154)
    clojure.lang.Var.applyTo (Var.java:700)
    clojure.core$apply.invoke (core.clj:626)
    leiningen.core.main$partial_task$fn__4230.doInvoke (main.clj:234)
    clojure.lang.RestFn.invoke (RestFn.java:410)
    clojure.lang.AFn.applyToHelper (AFn.java:154)
    clojure.lang.RestFn.applyTo (RestFn.java:132)
    clojure.lang.AFunction$1.doInvoke (AFunction.java:29)
    clojure.lang.RestFn.applyTo (RestFn.java:137)
    clojure.core$apply.invoke (core.clj:626)
    leiningen.core.main$apply_task.invoke (main.clj:281)
    leiningen.core.main$resolve_and_apply.invoke (main.clj:287)
    leiningen.core.main$_main$fn__4295.invoke (main.clj:357)
    leiningen.core.main$_main.doInvoke (main.clj:344)
    clojure.lang.RestFn.invoke (RestFn.java:408)
    clojure.lang.Var.invoke (Var.java:379)
    clojure.lang.AFn.applyToHelper (AFn.java:154)
    clojure.lang.Var.applyTo (Var.java:700)
    clojure.core$apply.invoke (core.clj:624)
    clojure.main$main_opt.invoke (main.clj:315)
    clojure.main$main.doInvoke (main.clj:420)
    clojure.lang.RestFn.invoke (RestFn.java:436)
    clojure.lang.Var.invoke (Var.java:388)
    clojure.lang.AFn.applyToHelper (AFn.java:160)
    clojure.lang.Var.applyTo (Var.java:700)
    clojure.main.main (main.java:37)

Ran 2 tests containing 2 assertions.
0 failures, 1 errors.
Tests failed.
Tests failed.
gfredericks commented 10 years ago

I don't understand this but I think I just realized that there's no need for a data_readers.clj file, since I'm the one calling read-string and I can control the data readers directly.

So I'm just going to do that and not bother figuring out what's happening for you. I'll post another release shortly.

wilkerlucio commented 10 years ago

@gfredericks if I don't remember wrong, the problem here is that the data_readers.clj are exclusive for a given project, so, since your project is just a dependency, your data_readers doesn't apply for me, so this shouldn't be used like this. The correct thing on the case of VCR here I think would be use EDN so you can set the data readers for a narrow scope, and I guess that's what you are going for, right?

gfredericks commented 10 years ago

Yep.

I thought data reader files got merged somehow, but I might be wrong.

At the very least it should definitely work if your app and the other dependencies don't supply the file.

gfredericks commented 10 years ago

Okay, version 0.3.4-alpha-92bd55e should have no data readers file.

wilkerlucio commented 10 years ago

pure gold :)

[node-webkit-build] ls                                                                                                                                                                                                                                                                                                            11:19:13 [ruby-2.0.0]  ☁  master ☂ ✖ ⚡
#project.clj# LICENSE       README.md     cassettes     project.clj   src           target        test
[node-webkit-build] lein test                                                                                                                                                                                                                                                                                                     11:40:57 [ruby-2.0.0]  ☁  master ☂ ✖ ⚡
(Retrieving com/gfredericks/vcr-clj/0.3.4-alpha-92bd55e/vcr-clj-0.3.4-alpha-92bd55e.pom from clojars)
(Retrieving com/gfredericks/vcr-clj/0.3.4-alpha-92bd55e/vcr-clj-0.3.4-alpha-92bd55e.jar from clojars)

lein test node-webkit-build.core-test

Ran 2 tests containing 2 assertions.
0 failures, 0 errors.
gfredericks commented 10 years ago

Cool; thanks for working through this with me. I'll have a proper release deployed shortly.

wilkerlucio commented 10 years ago

Thank you for replying so quickly, I'm glad you get it working so fast.

gfredericks commented 10 years ago

Okay, 0.4.0 is released.

wilkerlucio commented 10 years ago

:smiley_cat: