Dynamic modules: should should-error work?

Dynamic modules: should should-error work?

[Eli Zaretskii]

It looks like should-error doesn't work with functions implemented in
modules.  For example, try this in modules/mod-test/test.el:

  (ert-deftest mod-test-sum-test ()
    (should-error (mod-test-sum 1 2 3)))

I cannot get this test to succeed, although the error message about
wrong number of arguments is emitted.  What am I missing?

Re: Dynamic modules: should should-error work?

[Philipp Stephani]

[-- Attachment #1.1: Type: text/plain, Size: 605 bytes --]

Eli Zaretskii <eliz@gnu.org> schrieb am So., 22. Nov. 2015 um 19:29 Uhr:

> It looks like should-error doesn't work with functions implemented in
> modules.  For example, try this in modules/mod-test/test.el:
>
>   (ert-deftest mod-test-sum-test ()
>     (should-error (mod-test-sum 1 2 3)))
>
> I cannot get this test to succeed, although the error message about
> wrong number of arguments is emitted.  What am I missing?
>
>
This works for me. Maybe there was some intermittent bug that has since
been fixed?
Anyway, this is a useful thing to test. I've attached a patch to add this
to the test suite.

[-- Attachment #1.2: Type: text/html, Size: 922 bytes --]

[-- Attachment #2: 0001-Add-test-for-argument-count-check.patch --]
[-- Type: application/octet-stream, Size: 1268 bytes --]

From 667fa4cff525604adbee7f5b920536b63987e72d Mon Sep 17 00:00:00 2001
From: Philipp Stephani <phst@google.com>
Date: Mon, 23 Nov 2015 20:27:07 +0100
Subject: [PATCH] Add test for argument count check

---
 modules/mod-test/mod-test.c | 2 ++
 modules/mod-test/test.el    | 3 ++-
 2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/modules/mod-test/mod-test.c b/modules/mod-test/mod-test.c
index 44a14dc..b0c535c 100644
--- a/modules/mod-test/mod-test.c
+++ b/modules/mod-test/mod-test.c
@@ -42,6 +42,8 @@ sum (intmax_t a, intmax_t b)
 static emacs_value
 Fmod_test_sum (emacs_env *env, ptrdiff_t nargs, emacs_value args[], void *data)
 {
+  assert (nargs == 2);
+
   intmax_t a = env->extract_integer (env, args[0]);
   intmax_t b = env->extract_integer (env, args[1]);
 
diff --git a/modules/mod-test/test.el b/modules/mod-test/test.el
index 98ce464..d949fa4 100644
--- a/modules/mod-test/test.el
+++ b/modules/mod-test/test.el
@@ -28,7 +28,8 @@
 ;;
 
 (ert-deftest mod-test-sum-test ()
-  (should (= (mod-test-sum 1 2) 3)))
+  (should (= (mod-test-sum 1 2) 3))
+  (should-error (mod-test-sum 1 2 3) :type 'wrong-number-of-arguments))
 
 (ert-deftest mod-test-sum-docstring ()
   (should (string= (documentation 'mod-test-sum) "Return A + B")))
-- 
2.6.3

Re: Dynamic modules: should should-error work?

[Eli Zaretskii]

> From: Philipp Stephani <p.stephani2@gmail.com>
> Date: Mon, 23 Nov 2015 19:28:09 +0000
> 
> Eli Zaretskii <eliz@gnu.org> schrieb am So., 22. Nov. 2015 um 19:29 Uhr:
> 
>     It looks like should-error doesn't work with functions implemented in
>     modules. For example, try this in modules/mod-test/test.el:
>     
>     (ert-deftest mod-test-sum-test ()
>     (should-error (mod-test-sum 1 2 3)))
>     
>     I cannot get this test to succeed, although the error message about
>     wrong number of arguments is emitted. What am I missing?
>     
> This works for me. Maybe there was some intermittent bug that has since been
> fixed?

Probably.  It works for me now.  Sorry for the noise.

> --- a/modules/mod-test/test.el
> +++ b/modules/mod-test/test.el
> @@ -28,7 +28,8 @@
>  ;;
>  
>  (ert-deftest mod-test-sum-test ()
> -  (should (= (mod-test-sum 1 2) 3)))
> +  (should (= (mod-test-sum 1 2) 3))
> +  (should-error (mod-test-sum 1 2 3) :type 'wrong-number-of-arguments))

Thanks.  I suggest the more thorough test below.  WDYT?

diff --git a/modules/mod-test/test.el b/modules/mod-test/test.el
index 98ce464..7924e3b 100644
--- a/modules/mod-test/test.el
+++ b/modules/mod-test/test.el
@@ -28,7 +28,14 @@
 ;;
 
 (ert-deftest mod-test-sum-test ()
-  (should (= (mod-test-sum 1 2) 3)))
+  (should (= (mod-test-sum 1 2) 3))
+  (let ((descr (should-error (mod-test-sum 1 2 3))))
+    (should (eq (car descr) 'wrong-number-of-arguments))
+    (should (stringp (nth 1 descr)))
+    (should (eq 0
+                (string-match "#<module function at \\(0x\\)?[0-9a-fA-F]+>"
+                              (nth 1 descr))))
+    (should (= (nth 2 descr) 3))))
 
 (ert-deftest mod-test-sum-docstring ()
   (should (string= (documentation 'mod-test-sum) "Return A + B")))

Re: Dynamic modules: should should-error work?

[Philipp Stephani]

[-- Attachment #1: Type: text/plain, Size: 725 bytes --]

Eli Zaretskii <eliz@gnu.org> schrieb am Mo., 23. Nov. 2015 um 21:09 Uhr:

> +  (let ((descr (should-error (mod-test-sum 1 2 3))))
> +    (should (eq (car descr) 'wrong-number-of-arguments))
>

I think testing using :type would also accept subtypes of the given type,
which I think would be preferrable (I'd expect that generally subtypes of
the documented signals are allowed to be thrown).


> +    (should (stringp (nth 1 descr)))
> +    (should (eq 0
> +                (string-match "#<module function at
> \\(0x\\)?[0-9a-fA-F]+>"
> +                              (nth 1 descr))))
> +    (should (= (nth 2 descr) 3))))
>
>
Maybe replace the regex with "\\`#<module function.*>\\'" to make it work
if dladdr is available.

[-- Attachment #2: Type: text/html, Size: 1247 bytes --]

Re: Dynamic modules: should should-error work?

[Eli Zaretskii]

> From: Philipp Stephani <p.stephani2@gmail.com>
> Date: Tue, 24 Nov 2015 19:27:32 +0000
> Cc: emacs-devel@gnu.org
> 
>     + (let ((descr (should-error (mod-test-sum 1 2 3))))
>     + (should (eq (car descr) 'wrong-number-of-arguments))
>     
> 
> I think testing using :type would also accept subtypes of the given type, which
> I think would be preferrable (I'd expect that generally subtypes of the
> documented signals are allowed to be thrown).

I'm not sure I understand what you are saying.  Can you give an
example?

>     + (should (stringp (nth 1 descr)))
>     + (should (eq 0
>     + (string-match "#<module function at \\(0x\\)?[0-9a-fA-F]+>"
>     + (nth 1 descr))))
>     + (should (= (nth 2 descr) 3))))
> 
> Maybe replace the regex with "\\`#<module function.*>\\'" to make it work if
> dladdr is available.

That's what I did in the final commit a few minutes ago.

Thanks.

Re: Dynamic modules: should should-error work?

[Philipp Stephani]

[-- Attachment #1: Type: text/plain, Size: 768 bytes --]

Eli Zaretskii <eliz@gnu.org> schrieb am Di., 24. Nov. 2015 um 20:32 Uhr:

> > From: Philipp Stephani <p.stephani2@gmail.com>
> > Date: Tue, 24 Nov 2015 19:27:32 +0000
> > Cc: emacs-devel@gnu.org
> >
> >     + (let ((descr (should-error (mod-test-sum 1 2 3))))
> >     + (should (eq (car descr) 'wrong-number-of-arguments))
> >
> >
> > I think testing using :type would also accept subtypes of the given
> type, which
> > I think would be preferrable (I'd expect that generally subtypes of the
> > documented signals are allowed to be thrown).
>
> I'm not sure I understand what you are saying.  Can you give an
> example?
>

If it is documented that an error of type X is thrown, does that mean the
error symbol is exactly X or rather that one of its conditions is X?

[-- Attachment #2: Type: text/html, Size: 1248 bytes --]

Re: Dynamic modules: should should-error work?

[Eli Zaretskii]

> From: Philipp Stephani <p.stephani2@gmail.com>
> Date: Mon, 07 Dec 2015 20:09:49 +0000
> Cc: emacs-devel@gnu.org
> 
>     > + (let ((descr (should-error (mod-test-sum 1 2 3))))
>     > + (should (eq (car descr) 'wrong-number-of-arguments))
>     >
>     >
>     > I think testing using :type would also accept subtypes of the given type,
>     which
>     > I think would be preferrable (I'd expect that generally subtypes of the
>     > documented signals are allowed to be thrown).
>     
>     I'm not sure I understand what you are saying. Can you give an
>     example?
> 
> If it is documented that an error of type X is thrown, does that mean the error
> symbol is exactly X or rather that one of its conditions is X? 

In general, the latter; but for wrong-number-of-arguments we know it
has no additional conditions, so it will be exactly it and nothing
else.

Re: Dynamic modules: should should-error work?

[Eli Zaretskii]

> From: Philipp Stephani <p.stephani2@gmail.com>
> Date: Mon, 23 Nov 2015 19:28:09 +0000
> 
> Anyway, this is a useful thing to test. I've attached a patch to add this to
> the test suite. 

I've pushed the first part in your name.

For the test itself, I pushed the variant I suggested in a followup.

Thanks.