Closed AlexBaranosky closed 12 years ago
I've added this working code into Midje, but wanted to give anyone a chance to comment on any aspects of it they don't like before I close this issue.
One minor concern is the name =throws=>
... is it too similar to the checker named throws
? However if it was something like =raises=> I could see people constantly forgetting which was the checker and which was the prerequisite arrow...
I like this. I think throws
is a better name than raises
. I would never remember which was which.
One more concern: is it possible to use this with =streams=> ? A common use for this is testing stateful functions that throw until they succeed:
(fact "foo retries correctly" (foo) => truthy (provided (bar) =streams=> [(throwf "failed") (throwf "failed") "successful"]))
Hi @arohner -
Currently, no, it is not compatible with =streams=>
It could be done, but would require tweaking the API.
somehow =stream=> would need an option that indicated that it should throw anything tht looks like an Exception, instead of simply returning it:
(fact (foo) => anything (provided (bar) =streams=> [:throw (Exception.) :throw (Exception.) :success]))
I can't think of a non crufty way of doing this though.
Any suggestions for the API?
What if =streams=> took unevaluated forms, and lazily evaluated them? Then you could stick arbitrary values in there:
=streams=> [(throwf) (/ 1 0) true]
I wanted to triumphantly show how you can do what you want within the current framework. That looked like this:
(fact
(caller) => 7
(provided
(called) =streams=> (lazily-produce (throw (Exception. "first!"))
(throw (Exception. "second!"))
7)))
lazily-produce
would be a macro that wrapped each of its forms in a function (a "thunk"), which would be evaluated only when called for:
(defn thunkize [forms]
(let [wrapped (map (fn [form] `(fn [] ~form))
forms)]
`(list ~@wrapped)))
(defmacro lazily-produce [& forms]
(let [lazy-forms (thunkize forms)]
`(map (fn [f#] (f#)) ~lazy-forms)))
Unfortunately, an exception blows up lazy sequences:
user=> (def f (lazily-produce (throw (Exception. "first!"))
(throw (Exception. "second!"))
7))
#'user/f
user=> (first f)
Exception first! user/fn--4289 (NO_SOURCE_FILE:341)
user=> (second f)
nil
user=> f
()
... and so, this fact doesn't work:
(unfinished called)
(defn fail-gracefully [on-fn]
(try
(on-fn)
(catch Exception e
:well-that-was-a-big-fail)))
(defn caller []
(first (remove #(= :well-that-was-a-big-fail %)
(repeatedly (fail-gracefully called)))))
(fact
(caller) => 7
(provided
(called) =streams=> (lazily-produce (throw (Exception. "first!"))
(throw (Exception. "second!"))
7)))
The perfect end to a week that profoundly sucked.
In short, I don't understand how Clojure's abstraction of iterative computation into lazy sequences works when an arbitrary computation raises exceptions.
Relatedly: I'm not convinced that the idea of =streams=>
has an obvious connection with "iterate until there are no more exceptions". If Clojure thinks that's a different abstraction, should not Midje think the same?
There are problems with attempting to shoehorn exceptions into lazy seqs
Clojure 1.1 and above 'chunk' seqs, evaluating some number of items ahead of time. Currently, that number is 32. This is a performance optimization. There are ways around it. http://blog.fogus.me/2010/01/22/de-chunkifying-sequences-in-clojure/
A seq in clojure is an immutable list of generated values, plus a fn for generating new values. When the fn throws, the seq's "generator" fn is set to nil, and the seq can no longer generate new values.
It looks like the desired behavior is attainable by modifying the definition of =streams=>. If =streams=> only took a seq of thunks, then =streams=> [(throwf "1) (throwf "2") :success] would work just fine. The implementation would just take the first fn off the streams atom, and invoke it. It would also remove the necessity of =throws=>, thought that could stick around if you like the readability of also having =throws=>.
Here's my implementation of lazily-produce. Feel free to use it, however you like.
(defn stateful-fn*
[fns]
(let [state (atom fns)]
(fn []
(if (not (zero? (count @state)))
(let [f (first @state)]
(try
(apply f [])
(finally
(swap! state rest))))
(throwf "no more values")))))
(defmacro thunk-args
"Takes a seq of unevaluated exprs. Returns a seq of no argument fns, that call each of the exprs in turn"
[exprs]
(into [] (map (fn [v]
`(fn []
~v)) exprs)))
(defmacro stateful-fn
"Takes a seq of exprs. Returns a new fn that calls each expr in turn."
[& args]
`(stateful-fn* (thunk-args ~args)))
Hi guys,
What do we want =streams=> to always thunkize it's right hand side? Or do we want lazily-produce to be something the user has to call explicitly.
I've got something working locally that will always thunkify each element in the sequence on the right hand side of =streams=>
I was going to wait on this for confirmation from @marick, but I might as well push it, and it is easy enough to revert if we want to make the thunking be explicit.
On Feb 21, 2012, at 4:26 AM, AlexBaranosky wrote:
What do we want =streams=> to always thunkize it's right hand side? Or do we want lazily-produce to be something the user has to call explicitly.
I don't know. It makes me nervous, but I can't offhand think of behavior that it would break. Phillip Calçado wrote =streams=> in the first place because he needed it. We should ask him if he sees a problem. I'll tweet him.
Brian Marick, Artisanal Labrador Now working at http://path11.com Contract programming in Ruby and Clojure Occasional consulting on Agile
@marick, have we heard anything from @pcalcado ? I'm tempted to close this issue.
Hey,
I saw this now but am on holidays with very limited web access. Can I come back to you two Sunday/Monday, when I'm back in Berlin?
Cheers
2012/2/23 AlexBaranosky reply@reply.github.com:
@marick, have we heard anything from @pcalcado ? I'm tempted to close this issue.
Reply to this email directly or view it on GitHub: https://github.com/marick/Midje/issues/98#issuecomment-4131241
Phil Calçado http://philcalcado.com
Hi everyone,
Sorry about the ridiculously long time to get back to you.
I read the thread and the patch. As I understand, we want to expand the =streams=> feature to be able to not only return immediate values but also the result of any arbitrary computation. The proposed solution is to thunkify the right side of =streams=>, using a macro to convert the collection items to functions that will be executed in the future.
It makes sense to me, and sounds useful.
Cheers
@pcalcado yes that is right. You can try out the new functionality in 1.3.2-SNAPSHOT as well.
Here's an example: On first call (called) will return 7, on the second call (called) will throw an Exception.
(fact
(caller) => 7
(provided
(called) =streams=> [7 (throw (Exception. "first!"))]))
Code that used to work, but now doesn't, in 1.3.2-SNAPSHOT (20120310):
(provided
(foo) =streams=> (range) :times 10))
returns
;.;. [31mFAIL[0m at (NO_SOURCE_FILE:1) ;.;. These calls were not made the right number of times: ;.;. (foo) [expected :times 10, actually called two times]
For me the question is whether we think it is reasonable for folks to change their facts to?:
(provided
(foo) =streams=> [0 1 2 3 4 5 6 7 8 9] :times 10))
I'd wonder how many cases like that that there are in practice -- I guesstimate not a ton.
While looking at this, I noticed another regression:
(ns scratch.core
(:use midje.sweet))
(unfinished foo)
(defn f [n]
(list (foo) (foo) (foo) (foo)))
(fact
(f 3) => [0 1 2 3]
(provided
(foo) =streams=> (range)))
In Midje 1.3.1, you get a passing test. In the current HEAD, you get this:
FAIL at (t_m.clj:14)
Expected: [0 1 2 3]
Actual: java.lang.Error: Your =stream=> ran out of values.
I've added a future-fact
to t_sweet.clj to document the need to fix this.
In response to Alex's question: I'd want @arohner's original example to work as shown because I never thought of anyone doing what he's doing. If a language/program is winning, combinations of ideas (like an infinite stream and :times) should just naturally work. If you have to say things like "well, if you want to use :times, you have to...", that's a sign you haven't got the right... abstractions? separation of concerns? --- something, anyway.
I'm working on fixing the two =streams=> issues.
I believe the HEAD now handles both =streams=> cases well. (Case 1: you're streaming a lazy seq, case 2: you're streaming a list of forms that you [may] want to be evaluated lazily.)
Note: I've started a new testing convention. There's a directory tests/as_documentation that's intended to describe behavior almost as if the end-user were to read it. That's partly due to a twitter conversation with Alex, but mostly because I saw that the tests for =streams=> were scattered and incomplete. Part of the reason they were incomplete was because they were scattered. (If something is in 8 places, it's harder to realize the 8 pieces don't add up to a coherent whole.) And because there wasn't a good all-in-one-place set of tests that described what =streams=> did, it was easier to change the code without realizing it would produce regressions.
While improving the documentation for =streams=>, I discovered that my fix does not work. That is, given this form:
(foo) =streams=> [ (form) (form) (form) ]
... it successfully evaluates the forms only as demanded. However, my test case used side-effects, not exceptions, to check that. In writing documentation that showed how that worked with exceptions, I discovered it didn't.
The code converts something like [ (throw (Exception. "foo")) ] into a lazy sequence of thunks like:
(fn [] (throw (Exception. "foo")))
Each thunk is evaluated only when its value is demanded. The idea was that the evaluation would throw the exception, the code-under-test would catch it and do whatever it was supposed to do. However, it appears that some mechanism somewhere captures the exception and converts it into an object. So the code-under-test's try
block doesn't get called.
At this point, I'm kind of in despair about the whole thing. I need convincing that (1) Midje can be responsible for esoterica like treating a list of forms as, possibly, not like a list of values, and (2) that this use case is important enough that it ought to be handled by Midje rather than by the person who wants to test the behavior of a function that uses another function that sometimes throws exceptions.
Here is the test code, which has also been checked in as a future-fact in as_documentation/t_streaming_prerequisites.clj
(unfinished a-try)
(defn a-try []
( (fn [] (throw (Exception. "fooooooo"))) ))
(defn msg []
(try
(a-try)
(catch Exception ex
(prn "#'msg going to return " (.getMessage ex))
(.getMessage ex))))
(defn longest-msg []
(reduce (fn [so-far text]
(prn "longest-msg working with " text)
(if (> (count text) (count so-far))
text
so-far))
[(msg) (msg) (msg)]))
; (prn [(msg) (msg) (msg)])
; (prn (longest-msg))
(future-fact "streaming exceptions actually works"
(longest-msg) => "i am long"
(provided
(a-try) =streams=> [ (throw (Exception. "short"))
(throw (Exception. "shorter"))
(throw (Exception. "i am long")) ]))
I looked at this a little earlier. It's really weird. Something seems t be wrapping the thrown exceptions in runtime exceptions. I thought this might be a Clojure 1.3 thing, but it gives the same kid of failures in all versions of Clojure.
@marick @arohner Should we give up on thunking the right side of =streams=> ? Honestly, I think thunking the right hand side is confusing. How is the user supposed to know when to expect thunking behaviour and when to expect immediate evaluation?
@marick what do you think of getting rid of the changes to =streams=>
? After some time to think objcetively, I think they are unintuitive, and of course they don't actually work. The =throws=>
case covers 95% of use cases imo.
@marick I committed changes to remove the code that was thunking the right side of =streams=>
. I submit this to be signed off by you. If you like the idea, we can close this issue. If not, we can either tweak or revert this commit.
Agreed.
Perhaps something like this: