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

Defer dereferences of metaconstants in function fakes #390

Merged
merged 11 commits into from Jun 20, 2017
2 changes: 1 addition & 1 deletion src/midje/data/prerequisite_state.clj
Expand Up @@ -91,7 +91,7 @@
map?
(do
(swap! (:call-count-atom action) inc)
((:result-supplier action )))
((:result-supplier action)))

:else
(do
Expand Down
9 changes: 4 additions & 5 deletions src/midje/emission/api.clj
Expand Up @@ -5,10 +5,9 @@
[midje.emission.clojure-test-facade :as ctf]
[midje.emission.levels :as levels]
[midje.emission.state :as state]
[midje.emission.plugins.silence :as silence]
[midje.util.thread-safe-var-nesting :refer [with-altered-roots]]))
[midje.emission.plugins.silence :as silence]))

;;; Level handling
;;; Level handling

(defn level-checker [operation]
(fn [level-name]
Expand All @@ -18,7 +17,7 @@
(def config-at-or-above? (level-checker >=))
(def config-above?(level-checker >))

;;; Plugins
;;; Plugins

(defn load-plugin [location]
(if (symbol? location)
Expand All @@ -32,7 +31,7 @@
(apply function args)
(throw (Error. (str "Your emission plugin does not define " keyword state/emission-functions))))))

;;; The API proper
;;; The API proper

(defn pass []
(state/output-counters:inc:midje-passes!)
Expand Down
4 changes: 2 additions & 2 deletions src/midje/parsing/1_to_explicit_form/facts.clj
Expand Up @@ -102,9 +102,9 @@
recognize/future-fact? (macroexpand form)
;; The `prerequisites` form is not supposed to be used in wrapping style.
wrapping-background-changer? (expand-wrapping-background-changer form)
recognize/expect? (wrapping/multiwrap form (wrapping/forms-to-wrap-around :checks ))
recognize/expect? (wrapping/multiwrap form (wrapping/forms-to-wrap-around :checks))
recognize/fact? (macroexpand form)
recognize/tabular? (macroexpand form)
recognize/tabular? (macroexpand form)
sequential? (preserve-type form (eagerly (map midjcoexpand form)))
:else form))

Expand Down
2 changes: 1 addition & 1 deletion src/midje/parsing/1_to_explicit_form/metaconstants.clj
Expand Up @@ -4,7 +4,7 @@
(:import midje.data.metaconstant.Metaconstant))

(defn predefine-metaconstants-from-form [form]
(let [metaconstant-symbols (filter data/metaconstant-symbol? (tree-seq coll? seq form))]
(let [metaconstant-symbols (set (filter data/metaconstant-symbol? (tree-seq coll? seq form)))]
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I noticed that (intern *ns* symbol (Metaconstant. symbol {} nil)) was being run multiple times for the same symbol. Figured it makes sense to filter out duplicates.

(doseq [symbol metaconstant-symbols]
(intern *ns* symbol (Metaconstant. symbol {} nil)))
metaconstant-symbols))
Expand Down
13 changes: 7 additions & 6 deletions src/midje/parsing/3_from_lexical_maps/from_fake_maps.clj
Expand Up @@ -38,22 +38,23 @@

(defmulti mkfn:result-supplier (fn [arrow & _] arrow))

(defmethod mkfn:result-supplier => [_arrow_ result] (constantly result))
(defmethod mkfn:result-supplier => [_arrow_ result] result)

(defmethod mkfn:result-supplier =streams=> [_arrow_ result-stream]
(let [the-stream (atom result-stream)]
(let [the-stream (atom (result-stream))]
(fn []
(when (empty? @the-stream)
(throw (exceptions/user-error "Your =stream=> ran out of values.")))
(let [current-result (first @the-stream)]
(swap! the-stream rest)
current-result))))

(defmethod mkfn:result-supplier =throws=> [_arrow_ throwable]
(defmethod mkfn:result-supplier =throws=> [_arrow_ wrapped-throwable]
(fn []
(when-not (instance? Throwable throwable)
(throw (exceptions/user-error "Right side of =throws=> should extend Throwable.")))
(throw throwable)))
(let [throwable (wrapped-throwable)]
(when-not (instance? Throwable throwable)
(throw (exceptions/user-error "Right side of =throws=> should extend Throwable.")))
(throw throwable))))

(defmethod mkfn:result-supplier :default [arrow result-stream]
(throw (exceptions/user-error "It's likely you misparenthesized your metaconstant prerequisite,"
Expand Down
2 changes: 1 addition & 1 deletion src/midje/parsing/lexical_maps.clj
Expand Up @@ -86,7 +86,7 @@
:value-at-time-of-faking (if (bound? ~(fnref/as-var-form fnref))
~(fnref/as-form-to-fetch-var-value fnref))
:arglist-matcher ~(choose-mkfn-for-arglist-matcher args)
:result-supplier (from-fake-maps/mkfn:result-supplier ~arrow ~result)
:result-supplier (from-fake-maps/mkfn:result-supplier ~arrow (fn [] ~result))
:times :default ; Default allows for a more attractive error in the most common case.

:position (pointer/line-number-known ~line)
Expand Down
48 changes: 34 additions & 14 deletions test/as_documentation/metaconstants.clj
Expand Up @@ -145,22 +145,42 @@
(provided
--mc-- =contains=> {:c 50000})))

(unfinished gen-doc)
(fact "Test merging of metaconstant that appear in data and function fakes"
(:header (gen-doc)) => "gamma"
(provided
(gen-doc) => ..doc..
..doc.. =contains=> {:header "gamma"}))

(fact "Test merging of nested metaconstant that appear in data and function fakes"
(:header (:raw (gen-doc))) => "gamma"
(provided
(gen-doc) => {:raw ..doc..}
..doc.. =contains=> {:header "gamma"}))
Copy link
Collaborator

Choose a reason for hiding this comment

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

What would happen in the following:

(fact 
  (:header (gen-doc)) => "gamma"
  (:header (gen-doc)) => "beta"
  (provided
    (gen-doc) =streams=> [..doc1.. ..doc2..]
    ..doc1.. =contains=> {:header "gamma"}
    ..doc2.. =contains=> {:header "beta"})) 

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Aah, nice example! It breaks the code, though I had to tweak it to be valid midje DSL code because provided only applies to the fact immediately above it:

(fact 
  (vector (:header (gen-doc)) (:header (gen-doc))) => [1 2] 
  (provided 
    (gen-doc) =streams=> [..doc1.. ..doc2..] 
    ..doc1.. =contains=> {:header 1} 
    ..doc2.. =contains=> {:header 2}))

results in

FAIL at (form-init5605701913344457958.clj:1)
    Expected: [1 2]
      Actual: (nil nil)
       Diffs: in [0] expected 1, was nil
              in [1] expected 2, was nil

I'll add this as a future-fact test for future implementation

Copy link
Collaborator

@rafaeldff rafaeldff Jun 19, 2017

Choose a reason for hiding this comment

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

Would the following maintain the same behavior as now?

(fact 
  (identical? (gen-doc) (gen-doc)) => true
  (provided
    ..doc.. =contains=> {:header "gamma"}
    (gen-doc) => ..doc..)) 

What about this:

(fact 
  (identical? (gen-doc) (gen-doc)) => true
  (provided
    ..doc.. =contains=> {:header (rand)}
    (gen-doc) => ..doc..)) 

?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

These continue to pass. Before they passed because (identical? nil nil) is true, now they pass because calls to gen-doc dereference ..doc.. after it has been bound to {:header (rand)}

Copy link
Collaborator

Choose a reason for hiding this comment

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

Cool. Do you think it would be worth adding this test to the suite?


(against-background [..doc.. =contains=> {:header "gamma"}]
(fact "Test merging of metaconstant with against-background"
(:header (gen-doc)) => "gamma"
(provided
(gen-doc) => ..doc..)))

(against-background [..doc.. =contains=> {:header ..header..}
..header.. =contains=> {:title "title"}]
(future-fact "Test metaconstants that contain other metaconstants"
(-> (gen-doc) :header :title) => "title"
(provided
(gen-doc) => ..doc..)))

(future-fact "Test metaconstants that contain other metaconstants"
(-> (gen-doc) :header :title) => "title"
(provided
(gen-doc) => ..doc..
..doc.. =contains=> {:header ..header..}
..header.. =contains=> {:title "title"}))
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

the two future-facts are tests elaborating some nice-to-have behavior that currently isn't supported











;;;;

;;; Use with prerequisite functions


(future-fact "Metaconstant merging and streaming"
(vector (:header (gen-doc)) (:header (gen-doc))) => [1 2]
(provided
(gen-doc) =streams=> [..doc1.. ..doc2..]
..doc1.. =contains=> {:header 1}
..doc2.. =contains=> {:header 2}))
9 changes: 9 additions & 0 deletions test/midje/data/t_metaconstant.clj
Expand Up @@ -180,3 +180,12 @@
0
..m..) => 6)

;;; Metaconstant behavior

(unfinished gen-doc)
(fact "=contains=> is evaluated once, even when referenced multiple times"
(identical? (gen-doc) (gen-doc)) => truthy
(provided
..doc.. =contains=> {:header (rand)}
(gen-doc) => ..doc..))

4 changes: 2 additions & 2 deletions test/midje/parsing/3_from_lexical_maps/t_from_fake_maps.clj
Expand Up @@ -57,10 +57,10 @@ odd? 3 falsey)
(facts "about result suppliers used"
"returns identity for =>"
(let [arrow "=>"]
((mkfn:result-supplier arrow [1 2 3])) => [1 2 3])
((mkfn:result-supplier arrow (fn [] [1 2 3]))) => [1 2 3])

"returns stream for =streams=>"
(let [supplier (mkfn:result-supplier "=streams=>" [1 2 3])]
(let [supplier (mkfn:result-supplier "=streams=>" (fn [] [1 2 3]))]
(supplier) => 1
(supplier) => 2
(supplier) => 3))
Expand Down