Skip to content

Commit

Permalink
Issue 357: :test conditions could incorrectly be considered to create…
Browse files Browse the repository at this point in the history
… bindings in the compiler (#407)
  • Loading branch information
WilliamParker authored Sep 30, 2018
1 parent d37c1f9 commit 38a48f1
Show file tree
Hide file tree
Showing 3 changed files with 32 additions and 3 deletions.
2 changes: 1 addition & 1 deletion project.clj
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@

:repl-options {;; The large number of ClojureScript tests is causing long compilation times
;; to start the REPL.
:timeout 120000}
:timeout 180000}

;; Factoring out the duplication of this test selector function causes an error,
;; perhaps because Leiningen is using this as uneval'ed code.
Expand Down
7 changes: 5 additions & 2 deletions src/main/clojure/clara/rules/compiler.clj
Original file line number Diff line number Diff line change
Expand Up @@ -749,10 +749,13 @@

constraints (:constraints effective-leaf)

[bound-variables unbound-variables] (if (= :negation (condition-type leaf-condition))
[bound-variables unbound-variables] (if (#{:negation :test} (condition-type leaf-condition))
;; Variables used in a negation should be considered
;; unbound since they aren't usable in another condition,
;; so label all variables as unbound.
;; so label all variables as unbound. Similarly, :test
;; conditions can't bind new variables since they don't
;; have any new facts as input. See:
;; https://github.com/cerner/clara-rules/issues/357
[#{}
(apply s/union (classify-variables constraints))]

Expand Down
26 changes: 26 additions & 0 deletions src/test/common/clara/test_bindings.cljc
Original file line number Diff line number Diff line change
Expand Up @@ -343,3 +343,29 @@
(fire-rules))]

(is (= [{:?x nil}] (query session test-query)))))


;; https://github.com/cerner/clara-rules/issues/357
(def-rules-test test-accumulator-before-equality-test-in-test-node

{:queries [cold-query [[]
[[Cold (= ?t temperature)]]]]

:rules [location-restriction-rule [[[?coldest <- (acc/min :temperature :returns-fact false) :from [Temperature (= ?location location)]]
[:test (= ?location "LHR")]]
(insert! (->Cold ?coldest))]]

:sessions [empty-session [cold-query location-restriction-rule] {}]}

(is (empty? (-> empty-session
(insert (->Temperature 0 "LGW"))
fire-rules
(query cold-query)))
"The query results should be empty if the location fails the :test equality check")

(is (= [{:?t 0}]
(-> empty-session
(insert (->Temperature 0 "LHR"))
fire-rules
(query cold-query)))
"The query results should not be empty for a matching location"))

0 comments on commit 38a48f1

Please sign in to comment.