Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Recording new episodes with existing cassettes. #15

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 4 additions & 5 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -68,11 +68,10 @@ Each var that is recorded can be customized with options:
fn; if it returns falsy, the call will be passed to
through to the original function both during recording
and playback.

## TODO

* Add a better way to re-record than deleting cassette files.
Maybe an environment variable?
- `:record-new-episodes?`:
a boolean indicating if an existing cassette should be
updated with calls that were not previously recorded.
defaults to false.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The TODO should stay here, as there are still use cases that this feature doesn't address.

## License

Expand Down
11 changes: 9 additions & 2 deletions src/vcr_clj/cassettes.clj
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,15 @@
(binding [*out* writer]
(prn cassette))))

(defn- reattach-http-meta
[cassette]
(let [set-http-meta #(vary-meta % assoc :type :vcr-clj.clj-http/serializable-http-request)
update-episode #(update % :return set-http-meta)]
(update cassette :calls #(mapv update-episode %))))

;; TODO: use clojure.edn?
(defn read-cassette
[name]
(with-open [r (java.io.PushbackReader. (io/reader (cassette-file name)))]
(edn/read {:readers data-readers} r)))
(let [cassette (with-open [r (java.io.PushbackReader. (io/reader (cassette-file name)))]
(edn/read {:readers data-readers} r))]
(reattach-http-meta cassette)))
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This problem would be better solved in the vcr-clj.clj-http namespace, if possible. The print-method for ::serializable-http-request in that namespace could be enhanced so that http requests are recognized when reading, maybe with some help from the data-readers in the serialization namespace.

The reason for this is that everything in vcr-clj outside of the clj-http namespace is meant to be generic, and so attaching this metadata at this point in the code is inappropriate since it would be applied to all function calls, not just http requests. (there is currently a test failure because of this problem)

40 changes: 33 additions & 7 deletions src/vcr_clj/core.clj
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,9 @@
cassette {:calls @calls}]
[func-return cassette]))

(defn indexed-cassette [cassette]
(group-by (juxt :var-name :arg-key) (:calls cassette)))

;; I guess currently we aren't recording actual arguments, just the arg-key.
;; Should that change?
(defn playbacker
Expand All @@ -67,7 +70,7 @@
[cassette order-scope]
;; don't support anything else yet
(case order-scope
:key (let [calls (atom (group-by (juxt :var-name :arg-key) (:calls cassette)))]
:key (let [calls (atom (indexed-cassette cassette))]
(fn [var-name arg-key]
(let [next-val (swap! calls
(fn [x]
Expand All @@ -81,26 +84,43 @@
{:function var-name
:arg-key arg-key}))))))))

(defn record-new-episodes? [specs the-var-name]
(->> specs
(filter #(= (var-name (:var %)) the-var-name))
first
:record-new-episodes?))

;; Assuming that order is only preserved for calls to any var in
;; particular, not necessarily all the vars considered together.
(defn playback
[specs cassette func]
(let [the-playbacker (playbacker cassette :key)
(let [updated-cassette (atom cassette)
record! #(swap! updated-cassette update-in [:calls] conj %)
has-recording? #(get (indexed-cassette cassette) [%1 %2])
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this calls indexed-cassette on each call, which is unnecessary work since it's being called with the same argument each time

the-playbacker (playbacker cassette :key)
redeffings
(into {}
(for [{:keys [var arg-key-fn recordable?]
(for [{:keys [var arg-key-fn recordable? return-transformer]
:or {arg-key-fn vector
recordable? (constantly true)}}
recordable? (constantly true)
return-transformer identity}}
specs
:let [orig (deref var)
the-var-name (var-name var)
wrapped (fn [& args]
(let [k (apply arg-key-fn args)]
(if (apply recordable? args)
(:return (the-playbacker the-var-name k))
(if (and (record-new-episodes? specs the-var-name)
(not (has-recording? the-var-name k)))
(let [result (return-transformer (apply orig args))]
(record! {:var-name the-var-name
:arg-key k
:return result})
result)
(:return (the-playbacker the-var-name k)))
(apply orig args))))]]
[var (add-meta-from wrapped orig)]))]
(with-redefs-fn redeffings func)))
[(with-redefs-fn redeffings func) @updated-cassette]))

;; * TODO
;; ** Handle streams
Expand All @@ -115,7 +135,9 @@
(if (cassette-exists? cassette-name)
(do
(println' "Running test with existing" cassette-name "cassette...")
(playback specs (read-cassette cassette-name) func))
(let [[result cassette] (playback specs (read-cassette cassette-name) func)]
(write-cassette cassette-name cassette)
result))
(do
(println' "Recording new" cassette-name "cassette...")
(let [[return cassette] (record specs func)]
Expand All @@ -139,6 +161,10 @@
a function that the return value will be passed through
while recording, which can be useful for doing things
like ensuring serializability.
:record-new-episodes?
a boolean indicating if an existing cassette should be
updated with calls that were not previously recorded.
defaults to false.
}"
[cname specs & body]
`(with-cassette-fn* ~cname ~specs (fn [] ~@body)))
18 changes: 18 additions & 0 deletions test/vcr_clj/test/clj_http.clj
Original file line number Diff line number Diff line change
Expand Up @@ -123,3 +123,21 @@
(is (= "bar" (get "/foo")))))
(with-cassette :whale
(is (= "bar" (get "/foo")))))

(defn time-server [&args]
{:status 200
:body (str (System/currentTimeMillis))
:headers {}})


(deftest recording-new-http-episodes
(with-jetty-server time-server
(with-local-vars [result-with-a nil
result-with-b nil]
(with-cassette :recording-new-episodes
(var-set result-with-a (get "/a")))
(with-cassette :recording-new-episodes {:record-new-episodes? true}
(var-set result-with-b (get "/b")))
(with-cassette :recording-new-episodes
(is (= (get "/a") @result-with-a))
(is (= (get "/b") @result-with-b))))))
15 changes: 15 additions & 0 deletions test/vcr_clj/test/core.clj
Original file line number Diff line number Diff line change
Expand Up @@ -110,3 +110,18 @@
(is (= 42 (self-caller 41))))
(is (empty? (calls self-caller))
"the recorded call does not result in any self-calls")))

(defn current-time [& args]
"Accepts any arguments and returns the current time"
(System/currentTimeMillis))

(deftest recording-new-episodes
(with-local-vars [result-with-a nil
result-with-b nil]
(with-cassette :recording-new-episodes [{:var #'current-time}] (var-set result-with-a (current-time :a)))
(with-cassette :recording-new-episodes [{:var #'current-time :record-new-episodes? true}]
(var-set result-with-b (current-time :b)))
(with-cassette :recording-new-episodes [{:var #'current-time}]
(is (= (current-time :a) @result-with-a))
(is (= (current-time :b) @result-with-b)))))