---
This patch fixes

  (faith.error-match "oops" (fn [] nil))

passing with flying colors.

At Egghead Games, we test all "reachable"
errors, as per the awesome Test-Driven
Development (TDD), which is why this bug bit
us today.  So, here is a fix!

P.S. #1:

I did not check nor fix 'faith.error', as I
never use it.  Also, do we still need it, now
that we have 'faith.error-match'?  One can
explicitly match against ".*" if they need
to.  That said, either way, this patch is
orthogonal to fixing or removal of the
'error' function.

P.S. #2:

Originally, I wrote

  (faith.error-match "oops" #(nil))

but it tripped the compiler, FYI.

 faith.fnl                | 3 +--
 test/self.fnl            | 8 +++++++-
 test/subtest/failing.fnl | 6 ++++++
 3 files changed, 14 insertions(+), 3 deletions(-)

diff --git a/faith.fnl b/faith.fnl
index 6a0eb60..81352a3 100644
--- a/faith.fnl
+++ b/faith.fnl
@@ -105,8 +105,7 @@

 (fn error-match [pat f ?msg]
   (case (pcall f)
-    (true val) (wrap false ?msg
-                     "Expected an error, got %s" (fennel.view val))
+    (true ?val) (wrap false ?msg "Expected an error, got %s" (fennel.view ?val))
     (_ err) (let [err-string (if (= (type err) :string) err (fennel.view err))]
               (wrap (: err-string :match pat) ?msg
                     "Expected error to match pattern %s, was %s"
diff --git a/test/self.fnl b/test/self.fnl
index 36bbbd7..00a3f65 100644
--- a/test/self.fnl
+++ b/test/self.fnl
@@ -45,7 +45,7 @@
         counts {"s" 0 "." 0 "F" 0 "E" 0}]
     (each [_ t (pairs writes)]
       (tset counts t (+ (. counts t) 1)))
-    (t.= {"s" 1 "." 1 "E" 1 "F" 9} counts)
+    (t.= {"s" 1 "." 1 "E" 1 "F" 10} counts)
     (t.match "FAIL: ./test/subtest/failing.fnl:9: test%-not%-ok" out)
     (t.match "Expected %[\"hello world\"%], got %[\"hello buddy\"%]" out)
     (t.match "that wasn't supposed to happen" out)
@@ -99,6 +99,12 @@
                             "test%-error%-match%-message")
              out)
     (t.match "Expected an error, got false %- aye yay" out)
+    ;; test-error-match-nil
+    (t.match (string.format "FAIL: %s: %s"
+                            "%./test/subtest/failing%.fnl:62"
+                            "test%-error%-match%-nil")
+             out)
+    (t.match "Expected an error, got nil" out)
     (t.= 1 exit)))

 (fn test-setup []
diff --git a/test/subtest/failing.fnl b/test/subtest/failing.fnl
index 6c5d204..1019756 100644
--- a/test/subtest/failing.fnl
+++ b/test/subtest/failing.fnl
@@ -57,6 +57,11 @@
   (t.error-match "dang" #false "aye yay")
   nil)

+(λ test-error-match-nil []
+  ;; NOTE #(nil) gives "Bad code generated - likely a bug with the compiler"
+  (t.error-match "oops" (fn [] nil))
+  nil)
+
 {: test-ok
  : test-not-ok
  : test-very-not-ok
@@ -68,4 +73,5 @@
  : test-error-match-number
  : test-error-match-view
  : test-error-match-message
+ : test-error-match-nil
  : test-fail}
--
2.42.0

Nice catch; thanks. Applied and pushed. Sorry for the delay.

-Phil

Phil Hagelberg <phil@hagelb.org> writes:

> Nice catch; thanks. Applied and pushed. Sorry for the delay.

Fantastic, thanks!

Any comment on this:

> I did not check nor fix 'faith.error', as I
> never use it.  Also, do we still need it, now
> that we have 'faith.error-match'?  One can
> explicitly match against ".*" if they need
> to.  That said, either way, this patch is
> orthogonal to fixing or removal of the
> 'error' function.

Rudy
-- 
"Simplicity is complexity resolved."
-- Constantin Brâncuși, 1876-1957

Rudolf Adamkovič <salutis@me.com> [he/him]
Studenohorská 25
84103 Bratislava
Slovakia

>> I did not check nor fix 'faith.error', as I
>> never use it.  Also, do we still need it, now
>> that we have 'faith.error-match'?  One can
>> explicitly match against ".*" if they need
>> to.  That said, either way, this patch is
>> orthogonal to fixing or removal of the
>> 'error' function.

Oh, I missed this earlier. I think you're right that it can be removed;
we are still in the 0.1.x stage of the project, and on top of that it's
almost always a mistake not to check the contents of the error.

I'll go ahead and push this removal.

-Phil

Phil Hagelberg <phil@hagelb.org> writes:

> I'll go ahead and push this removal.

Fantastic, thank you!

Should we also rename 'error-match' to
'error', now that there is only one way to
assert on errors?

When I compare

  (faith.error-match
    ".*: index out of bounds"
    #(atrium.section 1 -1))

with

  (faith.error
    ".*: index out of bounds"
    #(atrium.section 1 -1))

I see think the latter is sufficiently, if
not perfectly, clear.

WDYT?

Rudy
-- 
"Genius is 1% inspiration and 99% perspiration."
-- Thomas Alva Edison, 1932

Rudolf Adamkovič <salutis@me.com> [he/him]
Studenohorská 25
84103 Bratislava
Slovakia

Rudolf Adamkovič <salutis@me.com> writes:

> Should we also rename 'error-match' to
> 'error', now that there is only one way to
> assert on errors?

Oh, yeah good idea. Better to get these breaking changes out earlier.

-Phil