~technomancy/fennel

This thread contains a patchset. You're looking at the original emails, but you may wish to use the patch review UI. Review patch
1

[PATCH v2] fix ?. behavior introduced in 18c6fcf and fix tests not being ran

Details
Message ID
<20210228133342.476522-1-andreyorst@gmail.com>
DKIM signature
pass
Download raw message
Patch: +53 -52
The change introduced in 18c6fcf made ?. behave the same as .
This was not caught by the tests, because tests were not ran at all,
because "expected" result was nil, and assigning nil to table key
effectively removes that key from the table, thus making tests
completely ignored. I've noticed this and fixed in some other tests in
core.fnl as well.
---
 src/fennel/macros.fnl |  8 +++---
 test/core.fnl         | 30 +++++++++----------
 test/macro.fnl        | 67 ++++++++++++++++++++++---------------------
 3 files changed, 53 insertions(+), 52 deletions(-)

diff --git a/src/fennel/macros.fnl b/src/fennel/macros.fnl
index fd59a05..5f35919 100644
--- a/src/fennel/macros.fnl
+++ b/src/fennel/macros.fnl
@@ -60,13 +60,13 @@ Same as ->> except will short-circuit with nil when it encounters a nil value."
               (-?>> ,el ,(unpack els))
               ,tmp)))))

(fn ?dot [tbl k1 ...]
(fn ?dot [tbl k ...]
  "Nil-safe table look up.
Same as . (dot), except will short-circuit with nil when it encounters
a nil value in any of subsequent keys."
  (if (= nil k1)
      tbl
      `(?. (. ,tbl ,k1) ,...)))
  (if (= nil k) tbl
      `(let [res# (. ,tbl ,k)]
         (and res# (?. res# ,...)))))

(fn doto* [val ...]
  "Evaluates val and splices it into the first argument of subsequent forms."
diff --git a/test/core.fnl b/test/core.fnl
index c0f23bf..992174c 100644
--- a/test/core.fnl
+++ b/test/core.fnl
@@ -107,20 +107,20 @@
      (l.assertEquals (fennel.eval code {:correlate true}) expected code))))

(fn test-conditionals []
  (let [cases {"(if _G.non-existent 1 (* 3 9))" 27
               "(if false \"yep\" \"nope\")" "nope"
               "(if false :y true :x :trailing :condition)" "x"
               "(let [b :original b (if false :not-this)] (or b :nil))" "nil"
               "(let [x 1 y 2] (if (= (* 2 x) y) \"yep\"))" "yep"
               "(let [x 3 res (if (= x 1) :ONE (= x 2) :TWO true :???)] res)" "???"
               "(let [x {:y 2}] (if false \"yep\" (< 1 x.y 3) \"uh-huh\" \"nope\"))" "uh-huh"
               "(var [a z] [0 0]) (when true (set a 192) (set z 12)) (+ z a)" 204
               "(var a 884) (when nil (set a 192)) a" 884
               "(var i 0) (var s 0) (while (let [l 11] (< i l)) (set s (+ s i)) (set i (+ 1 i))) s" 55
               "(var x 12) (if true (set x 22) 0) x" 22
               "(when (= 12 88) (os.exit 1)) false" false
               "(while (let [f false] f) (lua :break))" nil}]
    (each [code expected (pairs cases)]
  (let [cases [["(if _G.non-existent 1 (* 3 9))" 27]
               ["(if false \"yep\" \"nope\")" "nope"]
               ["(if false :y true :x :trailing :condition)" "x"]
               ["(let [b :original b (if false :not-this)] (or b nil))" nil]
               ["(let [x 1 y 2] (if (= (* 2 x) y) \"yep\"))" "yep"]
               ["(let [x 3 res (if (= x 1) :ONE (= x 2) :TWO true :???)] res)" "???"]
               ["(let [x {:y 2}] (if false \"yep\" (< 1 x.y 3) \"uh-huh\" \"nope\"))" "uh-huh"]
               ["(var [a z] [0 0]) (when true (set a 192) (set z 12)) (+ z a)" 204]
               ["(var a 884) (when nil (set a 192)) a" 884]
               ["(var i 0) (var s 0) (while (let [l 11] (< i l)) (set s (+ s i)) (set i (+ 1 i))) s" 55]
               ["(var x 12) (if true (set x 22) 0) x" 22]
               ["(when (= 12 88) (os.exit 1)) false" false]
               ["(while (let [f false] f) (lua :break))" nil]]]
    (each [_ [code expected] (ipairs cases)]
      (l.assertEquals (fennel.eval code {:correlate true}) expected code))))

(fn test-core []
@@ -219,7 +219,7 @@
               "(tostring (. {} 12))" "nil"
               "(let [(_ m) (pcall #(. 1 1))] (m:match \"attempt to index a number\"))"
               "attempt to index a number"
               "(let [t {:st {:v 5 :f #(+ $.v $2)}} x (#(+ $ $2) 1 3)] (t.st:f x) nil)" nil
               "(tostring (let [t {:st {:v 5 :f #(+ $.v $2)}} x (#(+ $ $2) 1 3)] (t.st:f x) nil))" "nil"
               "(let [x (if 3 4 5)] x)" 4
               "(select \"#\" (if (= 1 (- 3 2)) (values 1 2 3 4 5) :onevalue))" 5
               (.. "(do (local c1 20) (local c2 40) (fn xyz [A B] (and A B)) "
diff --git a/test/macro.fnl b/test/macro.fnl
index 45d6b3d..64338d5 100644
--- a/test/macro.fnl
+++ b/test/macro.fnl
@@ -2,42 +2,43 @@
(local fennel (require :fennel))

(fn test-arrows []
  (let [cases {"(-> (+ 85 21) (+ 1) (- 99))" 8
               "(-> 1234 (string.reverse) (string.upper))" "4321"
               "(-> 1234 string.reverse string.upper)" "4321"
               "(->> (+ 85 21) (+ 1) (- 99))" (- 8)
               "(-?> [:a :b] (table.concat \" \"))" "a b"
               "(-?> {:a {:b {:c :z}}} (. :a) (. :b) (. :c))" "z"
               "(-?> {:a {:b {:c :z}}} (. :a) (. :missing) (. :c))" nil
               "(-?>> \" \" (table.concat [:a :b]))" "a b"
               "(-?>> :w (. {:w :x}) (. {:x :missing}) (. {:y :z}))" nil
               "(-?>> :w (. {:w :x}) (. {:x :y}) (. {:y :z}))" "z"}]
    (each [code expected (pairs cases)]
  (let [cases [["(-> (+ 85 21) (+ 1) (- 99))" 8]
               ["(-> 1234 (string.reverse) (string.upper))" "4321"]
               ["(-> 1234 string.reverse string.upper)" "4321"]
               ["(->> (+ 85 21) (+ 1) (- 99))" (- 8)]
               ["(-?> [:a :b] (table.concat \" \"))" "a b"]
               ["(-?> {:a {:b {:c :z}}} (. :a) (. :b) (. :c))" "z"]
               ["(-?> {:a {:b {:c :z}}} (. :a) (. :missing) (. :c))" nil]
               ["(-?>> \" \" (table.concat [:a :b]))" "a b"]
               ["(-?>> :w (. {:w :x}) (. {:x :missing}) (. {:y :z}))" nil]
               ["(-?>> :w (. {:w :x}) (. {:x :y}) (. {:y :z}))" "z"]]]
    (each [_ [code expected] (ipairs cases)]
      (l.assertEquals (fennel.eval code) expected code))))

(fn test-?. []
  (let [cases {"(?. {:a 1})" {:a 1}
               "(?. {:a 1} :a)" 1
               "(?. {:a 1} :b)" nil
               "(?. [-1 -2])" [-1 -2]
               "(?. [-1 -2] 1)" -1
               "(?. [-1 -2] 3)" nil
               "(?. {:a {:b {:c 3}}} :a :b :c)" 3
               "(?. {:a {:b {:c 3}}} :d :b :c)" nil
               "(?. {:a {:b {:c 3}}} :a :d :c)" nil
               "(?. {:a {:b {:c 3}}} :a :b :d)" nil
               "(?. [-1 [-2 [-3] [-4]]] 2 3 1)" -4
               "(?. [-1 [-2 [-3] [-4]]] 0 3 1)" nil
               "(?. [-1 [-2 [-3] [-4]]] 2 5 1)" nil
               "(?. [-1 [-2 [-3] [-4]]] 2 3 2)" nil
               "(?. {:a [{} {:b {:c 4}}]} :a 2 :b :c)" 4
               "(?. {:a [{} {:b {:c 4}}]} :a 1 :b :c)" nil
               "(?. {:a [{} {:b {:c 4}}]} :a 3 :b :c)" nil
               "(?. {:a [[{:b {:c 5}}]]} :a 1 :b :c)" nil
               "(?. {:a [[{:b {:c 5}}]]} :a 1 1 :b :c)" 5
               "(local t {:a [[{:b {:c 5}}]]}) (?. t :a 1 :b :c)" nil
               "(local t {:a [[{:b {:c 5}}]]}) (?. t :a 1 1 :b :c)" 5}]
    (each [code expected (pairs cases)]
  (let [cases [["(?. {:a 1})" {:a 1}]
               ["(?. {:a 1} :a)" 1]
               ["(?. {:a 1} :b)" nil]
               ["(?. [-1 -2])" [-1 -2]]
               ["(?. [-1 -2] 1)" -1]
               ["(?. [-1 -2] 3)" nil]
               ["(?. {:a {:b {:c 3}}} :a :b :c)" 3]
               ["(?. {:a {:b {:c 3}}} :d :b :c)" nil]
               ["(?. {:a {:b {:c 3}}} :a :d :c)" nil]
               ["(?. {:a {:b {:c 3}}} :a :b :d)" nil]
               ["(?. [-1 [-2 [-3] [-4]]] 2 3 1)" -4]
               ["(?. [-1 [-2 [-3] [-4]]] 0 3 1)" nil]
               ["(?. [-1 [-2 [-3] [-4]]] 2 5 1)" nil]
               ["(?. [-1 [-2 [-3] [-4]]] 2 3 2)" nil]
               ["(?. {:a [{} {:b {:c 4}}]} :a 2 :b :c)" 4]
               ["(?. {:a [{} {:b {:c 4}}]} :a 1 :b :c)" nil]
               ["(?. {:a [{} {:b {:c 4}}]} :a 3 :b :c)" nil]
               ["(?. {:a [[{:b {:c 5}}]]} :a 1 :b :c)" nil]
               ["(?. {:a [[{:b {:c 5}}]]} :a 1 1 :b :c)" 5]
               ["(local t {:a [[{:b {:c 5}}]]}) (?. t :a 1 :b :c)" nil]
               ["(local t {:a [[{:b {:c 5}}]]}) (?. t :a 1 1 :b :c)" 5]
               ["(?. {:a [[{:b {:c false}}]]} :a 1 1 :b :c)" false]]]
    (each [_ [code expected] (ipairs cases)]
      (l.assertEquals (fennel.eval code) expected code))))

(fn test-comprehensions []
-- 
2.29.2
Details
Message ID
<87eeh0m650.fsf@whirlwind>
In-Reply-To
<20210228133342.476522-1-andreyorst@gmail.com> (view parent)
DKIM signature
missing
Download raw message
Andrey Listopadov <andreyorst@gmail.com> writes:

> The change introduced in 18c6fcf made ?. behave the same as .
> This was not caught by the tests, because tests were not ran at all,
> because "expected" result was nil, and assigning nil to table key
> effectively removes that key from the table, thus making tests
> completely ignored. I've noticed this and fixed in some other tests in
> core.fnl as well.

Thanks for catching this! The problem with the tests was a hold-over
from back before we were using a proper testing library and were just
rolling our own functions. It's great to see this patch take care of
that so it won't bite us in the future.

-Phil
Reply to thread Export thread (mbox)