GNU bug report logs - #24402
25.1.50; testcover-start breaks should-error

Previous Next

Full log

View this message in rfc822 format

From: Alex <agrambot <at> gmail.com>
To: Tino Calancha <tino.calancha <at> gmail.com>
Cc: Gemini Lasswell <gazally <at> runbox.com>, eliz <at> gnu.org, npostavs <at> users.sourceforge.net, 24402 <at> debbugs.gnu.org
Subject: bug#24402: should-error doesn't catch all errors
Date: Sun, 09 Jul 2017 01:04:24 -0600

Tino Calancha <tino.calancha <at> gmail.com> writes:

> I've being thinking about this, and i cannot find something better than
> your second patch.
>
> My suggestion is:
>
> 1. We apply your 2nd patch.
> 2. We handle the failing tests wrapping '(ert-fail "failed")' into
>   `ignore-errors' as in below patch.
>
> Then, IMO we are in a better situation than we are know:
> That fix this bug, and if i am nt missing something, at a low price: just
> tweaking a bit 2 innocents tests.
>
> What do you think?
>
>
> diff --git a/test/lisp/emacs-lisp/ert-tests.el b/test/lisp/emacs-lisp/ert-tests.el
> index 317838b250..f3e4db192a 100644
> --- a/test/lisp/emacs-lisp/ert-tests.el
> +++ b/test/lisp/emacs-lisp/ert-tests.el
> @@ -413,12 +413,14 @@ ert-test--which-file
>    (let ((test (make-ert-test :body (lambda ()))))
>      (should (ert-test-result-expected-p test (ert-run-test test))))
>    ;; unexpected failure
> -  (let ((test (make-ert-test :body (lambda () (ert-fail "failed")))))
> -    (should-not (ert-test-result-expected-p test (ert-run-test test))))
> -  ;; expected failure
> -  (let ((test (make-ert-test :body (lambda () (ert-fail "failed"))
> -                             :expected-result-type ':failed)))
> +  (let ((test (make-ert-test
> +               :body (lambda () (ignore-errors (ert-fail "failed"))))))
>      (should (ert-test-result-expected-p test (ert-run-test test))))
> +  ;; expected failure
> +  (let ((test (make-ert-test
> +               :body (lambda () (ignore-errors (ert-fail "failed")))
> +               :expected-result-type ':failed)))
> +    (should-not (ert-test-result-expected-p test (ert-run-test test))))
>    ;; `not' expected type
>    (let ((test (make-ert-test :body (lambda ())
>                               :expected-result-type '(not :failed))))

I agree that it's a low price, and that it fixes more than it breaks,
but it does involve switching the logic in a simple basic test.

Also note that ignore-errors expands into a condition-case, so instead
of changing the tests, the patch could be tweaked to return nil if the
test signals ert-test-{failed, skipped}. Or the two tests could use
condition-case directly and make sure that ert-test-failed is/isn't
signalled.

I'd like to get some more opinions on this bug, in the hopes that
there's a better solution. Eli/Noam (or anyone happening upon this), do
you have any ideas on how to solve this?

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.