[test] Mark tests with missing dependencies as "expected to fail"

[test] Mark tests with missing dependencies as "expected to fail"

[David Maus]

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

Hi all,

Currently tests with missing dependency are silently skipped -- it
might be worth changing this behavior to not skip them, but mark them
as expected to fail. You can do this in ERT by placing the
keyword :expected-result followed by either :passed or :failed before
the test's body.

Benefit of this is that the tests w/ missing dependencies will show up
in the ERT result page (with a small letter f) but (obviously) don't
count as failures.

The following macro is a first shot at a convenient way to define
tests with dependencies:

#+begin_src emacs-lisp
  (defmacro org-test-deftest-with-dependency (dependency &rest body)
    (let* ((docstring (if (stringp (third body)) (list (third body))))
           (deftest (nthcdr (if docstring 3 2) body))
           (dependency-p (eval dependency)))
      `(ert-deftest ,@(append (list (first body) (second body)) docstring)
           ,@(if dependency-p
                 '(:expected-result :passed)
               '(:expected-result :failed (error "Missing dependency")))
         ,@deftest)))
#+end_src

Here DEPENDENCY is a simple form that evaluates to non-nil if the
dependency is met. If marking the tests this way is the agreed way to
go we can extend the syntax of a dependency to an alist with a
human-readable description of the dependency as key and a list of
forms that all must eval to non-nil as value. E.g.

#+begin_src emacs-lisp
  (defvar test-ob-R-dependencies
    '(("R executable" (org-test-for-executable "R"))
      ("Package ESS" (featurep 'ess))))
#+end_src

And change the expander code to map through this alist and as soon one
dependency evals to nil sets a variable `dependency-missing' to the
respective dependency.

Any comments on this?

Best,
  -- David
--
OpenPGP... 0x99ADB83B5A4478E6
Jabber.... dmjena@jabber.org
Email..... dmaus@ictsoc.de

[-- Attachment #2: Type: application/pgp-signature, Size: 230 bytes --]

Re: [test] Mark tests with missing dependencies as "expected to fail"

[Eric Schulte]

David Maus <dmaus@ictsoc.de> writes:

> Hi all,
>
> Currently tests with missing dependency are silently skipped -- it
> might be worth changing this behavior to not skip them, but mark them
> as expected to fail. You can do this in ERT by placing the
> keyword :expected-result followed by either :passed or :failed before
> the test's body.
>
> Benefit of this is that the tests w/ missing dependencies will show up
> in the ERT result page (with a small letter f) but (obviously) don't
> count as failures.
>
> The following macro is a first shot at a convenient way to define
> tests with dependencies:
>
> #+begin_src emacs-lisp
>   (defmacro org-test-deftest-with-dependency (dependency &rest body)
>     (let* ((docstring (if (stringp (third body)) (list (third body))))
>            (deftest (nthcdr (if docstring 3 2) body))
>            (dependency-p (eval dependency)))
>       `(ert-deftest ,@(append (list (first body) (second body)) docstring)
>            ,@(if dependency-p
>                  '(:expected-result :passed)
>                '(:expected-result :failed (error "Missing dependency")))
>          ,@deftest)))
> #+end_src
>
> Here DEPENDENCY is a simple form that evaluates to non-nil if the
> dependency is met. If marking the tests this way is the agreed way to
> go we can extend the syntax of a dependency to an alist with a
> human-readable description of the dependency as key and a list of
> forms that all must eval to non-nil as value. E.g.
>
> #+begin_src emacs-lisp
>   (defvar test-ob-R-dependencies
>     '(("R executable" (org-test-for-executable "R"))
>       ("Package ESS" (featurep 'ess))))
> #+end_src
>
> And change the expander code to map through this alist and as soon one
> dependency evals to nil sets a variable `dependency-missing' to the
> respective dependency.
>
> Any comments on this?
>

Hi David,

I agree it would be preferable to note that not all tests are run when
dependencies are missing, although I don't think it is extremely
important.  I think some version of the above would be worthwhile if it
could be done in a file-wide manner (as are the current dependency
checks) and wouldn't require duplicating the dependency check or
changing every test form individually.  Perhaps a file-local-variable
could be used to expect failures for every form defined in the file?

Cheers -- Eric

-- 
Eric Schulte
http://cs.unm.edu/~eschulte/

Fwd: [test] Mark tests with missing dependencies as "expected to fail"

[Brian Wightman]

Neglected forwarding to the list - sorry Eric for the double post.

Brian


---------- Forwarded message ----------
From: Brian Wightman <MidLifeXis@wightmanfam.org>
Date: Tue, Oct 18, 2011 at 12:02 PM
Subject: Re: [O] [test] Mark tests with missing dependencies as
"expected to fail"
To: Eric Schulte <schulte.eric@gmail.com>


On Tue, Oct 18, 2011 at 11:22 AM, Eric Schulte <schulte.eric@gmail.com> wrote:
> I agree it would be preferable to note that not all tests are run when
> dependencies are missing, although I don't think it is extremely
> important.  I think some version of the above would be worthwhile if it
> could be done in a file-wide manner (as are the current dependency
> checks) and wouldn't require duplicating the dependency check or
> changing every test form individually.  Perhaps a file-local-variable
> could be used to expect failures for every form defined in the file?

Perl's TAP* (http://testanything.org/) uses SKIP results for tests
that should not be run because some prerequisite is not available, and
TODO tests for those that are expected to fail (due to not being
implemented, known breakage, etc).  They can be reported separately if
the harness wishes.

It sounds like this is what is being proposed.  Perhaps some prior art
could be used.

* I reference TAP because it is what I am familiar with, not because
it is better or worse than alternatives.

Brian

Re: [test] Mark tests with missing dependencies as "expected to fail"

[David Maus]

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

Hi Eric,

At Tue, 18 Oct 2011 10:22:34 -0600,
Eric Schulte wrote:
> Hi David,
>
> I agree it would be preferable to note that not all tests are run when
> dependencies are missing, although I don't think it is extremely
> important.  I think some version of the above would be worthwhile if it
> could be done in a file-wide manner (as are the current dependency
> checks) and wouldn't require duplicating the dependency check or
> changing every test form individually.  Perhaps a file-local-variable
> could be used to expect failures for every form defined in the file?

I tried the approach with a file-local variable but it didn't work
out: A macro can be expanded at any time, i.e. looks like there is no
way to obtain a reference to the buffer where the macro is defined at
expansion time.

But finally came up with this one:

#+begin_src emacs-lisp
    (defmacro org-test-with-dependencies (dependencies &rest body)
      "Mark `ert-deftest' forms in BODY with a expected result
    depending on DEPENDENCIES.  DEPENDENCIES is an alist with a
    human-readable name of the dependency as key. The second element
    of each member should be a form that evaluates to a non-nil value
    if the dependency is met.

    All `ert-deftest' forms in BODY are marked as expected to pass if
    all dependencies are met. Otherwise the expected result is set to
    `:failed' and the test's bodies modified to signal an error with
    an error message indicating the first failing dependency."
      (macrolet ((define-dependencies ()
                   `(cond
                     ,@(mapcar (lambda (dependency)
                                 `((not ,(second dependency)) ,(first dependency))) dependencies))))
        (let ((missing-dependency (define-dependencies)))
          `(progn
             ,@(mapcar (lambda (sexp)
                         (if (and (consp sexp)
                                  (eq (first sexp) 'ert-deftest))
                             (let* ((docstring (if (stringp (fourth sexp)) (list (fourth sexp))))
                                    (deftest-body (nthcdr (if docstring 4 3) sexp)))
                               `(,@(append (list (first sexp) (second sexp) (third sexp)) docstring)
                                 :expected-result ,@(if missing-dependency
                                                        `(:failed (error "Missing dependency: %s" ,missing-dependency))
                                                      '(:passed))
                                 ,@deftest-body))
                           sexp)) body)))))
#+end_src

You wrap it around ert-deftest forms, e.g.

#+begin_src emacs-lisp
(org-test-with-dependencies (("ESS" (featurep 'ess)))
  (ert-deftest test-ob-R/simple-session ()
   (org-test-with-temp-text
       "#+begin_src R :session R\n  paste(\"Yep!\")\n#+end_src\n"
     (should (string= "Yep!" (org-babel-execute-src-block))))))
#+end_src

#+begin_quote
ELISP> (macroexpand '(org-test-with-dependencies (("ESS" (featurep 'ess))) (ert-deftest foo () t)))
(progn
  (ert-deftest foo nil :expected-result :failed
	       (error "Missing dependency: %s" "ESS")
	       t))

ELISP>
#end_quote

If this is acceptable I'd push it and start to adjust the test
definitions.

Best,
  -- David
--
OpenPGP... 0x99ADB83B5A4478E6
Jabber.... dmjena@jabber.org
Email..... dmaus@ictsoc.de

[-- Attachment #2: Type: application/pgp-signature, Size: 230 bytes --]

Re: [test] Mark tests with missing dependencies as "expected to fail"

[Eric Schulte]

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

David Maus <dmaus@ictsoc.de> writes:

> Hi Eric,
>
> At Tue, 18 Oct 2011 10:22:34 -0600,
> Eric Schulte wrote:
>> Hi David,
>>
>> I agree it would be preferable to note that not all tests are run when
>> dependencies are missing, although I don't think it is extremely
>> important.  I think some version of the above would be worthwhile if it
>> could be done in a file-wide manner (as are the current dependency
>> checks) and wouldn't require duplicating the dependency check or
>> changing every test form individually.  Perhaps a file-local-variable
>> could be used to expect failures for every form defined in the file?
>
> I tried the approach with a file-local variable but it didn't work
> out: A macro can be expanded at any time, i.e. looks like there is no
> way to obtain a reference to the buffer where the macro is defined at
> expansion time.
>
> But finally came up with this one:
>

Nice macro,

The only downside I see is the requirement to wrap every single deftest
form which (to me) is too much overhead for too little payoff.  How
about the following which will register a failing test for each file of
tests not loaded due to missing dependencies.

Best -- Eric


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Indicate-tests-with-missing-dependencies-by-adding-a.patch --]
[-- Type: text/x-diff, Size: 2862 bytes --]

From a84259bba32a69ae0b54dc577f1ba19b23625714 Mon Sep 17 00:00:00 2001
From: Eric Schulte <schulte.eric@gmail.com>
Date: Sun, 13 Nov 2011 12:30:25 -0700
Subject: [PATCH] Indicate tests with missing dependencies by adding a expected failing test

* testing/lisp/test-ob-R.el (featurep): Signal missing dependencies
  with an error rather than a throw.
* testing/org-test.el (missing-test-dependency): Define the error
  signal for missing dependencies.
  (org-test-for-executable): Signal missing dependencies with an error
  rather than a throw.
  (org-test-load): Define an expected failing test for each file with
  missing dependencies.
---
 testing/lisp/test-ob-R.el |    2 +-
 testing/org-test.el       |   19 +++++++++++++++----
 2 files changed, 16 insertions(+), 5 deletions(-)

diff --git a/testing/lisp/test-ob-R.el b/testing/lisp/test-ob-R.el
index bc637ff..bb9783f 100644
--- a/testing/lisp/test-ob-R.el
+++ b/testing/lisp/test-ob-R.el
@@ -8,7 +8,7 @@
 
 (org-test-for-executable "R")
 (unless (featurep 'ess)
-  (throw 'missing-test-dependency "ESS"))
+  (signal 'missing-test-dependency "ESS"))
 
 (let ((load-path (cons (expand-file-name
 			".." (file-name-directory
diff --git a/testing/org-test.el b/testing/org-test.el
index 7d2f7e7..0f9cf1a 100644
--- a/testing/org-test.el
+++ b/testing/org-test.el
@@ -102,6 +102,10 @@ org-test searches this directory up the directory tree.")
 
 \f
 ;;; Functions for writing tests
+(put 'missing-test-dependency
+     'error-conditions
+     '(error missing-test-dependency))
+
 (defun org-test-for-executable (exe)
   "Throw an error if EXE is not available.
 This can be used at the top of code-block-language specific test
@@ -111,7 +115,7 @@ executable."
 	   (lambda (acc dir)
 	     (or acc (file-exists-p (expand-file-name exe dir))))
 	   exec-path :initial-value nil)
-    (throw 'missing-test-dependency exe)))
+    (signal 'missing-test-dependency (list exe))))
 
 (defun org-test-buffer (&optional file)
   "TODO:  Setup and return a buffer to work with.
@@ -275,10 +279,17 @@ otherwise place the point at the beginning of the inserted text."
 	       (lambda (path)
 		 (if (file-directory-p path)
 		     (rld path)
-		   (catch 'missing-test-dependency
-		     (when (string-match "^[A-Za-z].*\\.el$"
+		   (condition-case err
+		       (when (string-match "^[A-Za-z].*\\.el$"
 					 (file-name-nondirectory path))
-		       (load-file path)))))
+			 (load-file path))
+		     (missing-test-dependency
+		      (let ((name (intern
+				   (concat "org-missing-dependency/"
+					   (file-name-nondirectory
+					    (file-name-sans-extension path))))))
+			(eval `(ert-deftest ,name ()
+				 :expected-result :failed (should nil))))))))
 	       (directory-files base 'full
 				"^\\([^.]\\|\\.\\([^.]\\|\\..\\)\\).*\\.el$"))))
     (rld (expand-file-name "lisp" org-test-dir))
-- 
1.7.4.1


[-- Attachment #3: Type: text/plain, Size: 2683 bytes --]


>
> #+begin_src emacs-lisp
>     (defmacro org-test-with-dependencies (dependencies &rest body)
>       "Mark `ert-deftest' forms in BODY with a expected result
>     depending on DEPENDENCIES.  DEPENDENCIES is an alist with a
>     human-readable name of the dependency as key. The second element
>     of each member should be a form that evaluates to a non-nil value
>     if the dependency is met.
>
>     All `ert-deftest' forms in BODY are marked as expected to pass if
>     all dependencies are met. Otherwise the expected result is set to
>     `:failed' and the test's bodies modified to signal an error with
>     an error message indicating the first failing dependency."
>       (macrolet ((define-dependencies ()
>                    `(cond
>                      ,@(mapcar (lambda (dependency)
>                                  `((not ,(second dependency)) ,(first dependency))) dependencies))))
>         (let ((missing-dependency (define-dependencies)))
>           `(progn
>              ,@(mapcar (lambda (sexp)
>                          (if (and (consp sexp)
>                                   (eq (first sexp) 'ert-deftest))
>                              (let* ((docstring (if (stringp (fourth sexp)) (list (fourth sexp))))
>                                     (deftest-body (nthcdr (if docstring 4 3) sexp)))
>                                `(,@(append (list (first sexp) (second sexp) (third sexp)) docstring)
>                                  :expected-result ,@(if missing-dependency
>                                                         `(:failed (error "Missing dependency: %s" ,missing-dependency))
>                                                       '(:passed))
>                                  ,@deftest-body))
>                            sexp)) body)))))
> #+end_src
>
> You wrap it around ert-deftest forms, e.g.
>
> #+begin_src emacs-lisp
> (org-test-with-dependencies (("ESS" (featurep 'ess)))
>   (ert-deftest test-ob-R/simple-session ()
>    (org-test-with-temp-text
>        "#+begin_src R :session R\n  paste(\"Yep!\")\n#+end_src\n"
>      (should (string= "Yep!" (org-babel-execute-src-block))))))
> #+end_src
>
> #+begin_quote
> ELISP> (macroexpand '(org-test-with-dependencies (("ESS" (featurep 'ess))) (ert-deftest foo () t)))
> (progn
>   (ert-deftest foo nil :expected-result :failed
> 	       (error "Missing dependency: %s" "ESS")
> 	       t))
>
> ELISP>
> #end_quote
>
> If this is acceptable I'd push it and start to adjust the test
> definitions.
>
> Best,
>   -- David
> --
> OpenPGP... 0x99ADB83B5A4478E6
> Jabber.... dmjena@jabber.org
> Email..... dmaus@ictsoc.de

-- 
Eric Schulte
http://cs.unm.edu/~eschulte/

Re: [test] Mark tests with missing dependencies as "expected to fail"

[David Maus]

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

At Sun, 13 Nov 2011 12:31:20 -0700,
Eric Schulte wrote:
>
> [1  <text/plain (7bit)>]
> David Maus <dmaus@ictsoc.de> writes:
>
> > Hi Eric,
> >
> > At Tue, 18 Oct 2011 10:22:34 -0600,
> > Eric Schulte wrote:
> >> Hi David,
> >>
> >> I agree it would be preferable to note that not all tests are run when
> >> dependencies are missing, although I don't think it is extremely
> >> important.  I think some version of the above would be worthwhile if it
> >> could be done in a file-wide manner (as are the current dependency
> >> checks) and wouldn't require duplicating the dependency check or
> >> changing every test form individually.  Perhaps a file-local-variable
> >> could be used to expect failures for every form defined in the file?
> >
> > I tried the approach with a file-local variable but it didn't work
> > out: A macro can be expanded at any time, i.e. looks like there is no
> > way to obtain a reference to the buffer where the macro is defined at
> > expansion time.
> >
> > But finally came up with this one:
> >
>
> Nice macro,
>
> The only downside I see is the requirement to wrap every single deftest
> form which (to me) is too much overhead for too little payoff.  How
> about the following which will register a failing test for each file of
> tests not loaded due to missing dependencies.

The macro operates not just on a single `ert-deftest' form, but all
deftests in BODY. It kind-of works like a file local variable.

The current solution does its job, too -- so there's no need to use
this macro. Getting feedback about tests not run because of missing
dependencies is all I every wanted.

Best,
  -- David
--
OpenPGP... 0x99ADB83B5A4478E6
Jabber.... dmjena@jabber.org
Email..... dmaus@ictsoc.de

[-- Attachment #2: Type: application/pgp-signature, Size: 230 bytes --]