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

Conversation

philomates
Copy link
Collaborator

@philomates philomates commented Jun 12, 2017

Fix for #159

Idea behind the fix is to, for function fake results ((foo) => ..some-metaconstant..), defer the dereference of variables pointing to metaconstants. These dereferences were occurring at macro-expansion time due to the use of constantly. By deferring to runtime, we are able to dereference these variables in scopes that have been properly manipulated by =contains=> data fakes.

@@ -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.

(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

@philomates philomates changed the title Defer de-references of metaconstants in function fakes Defer dereferences of metaconstants in function fakes Jun 13, 2017
(: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

(:header (:raw (gen-doc))) => "gamma"
(provided
(gen-doc) => {:raw ..doc..}
..doc.. =contains=> {:header "gamma"}))
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?

@rafaeldff rafaeldff merged commit d693670 into marick:master Jun 20, 2017
@philomates philomates deleted the pm/metaconstant-merges branch December 21, 2017 08:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants