[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
bug#24402: should-error doesn't catch all errors
From: |
Alex |
Subject: |
bug#24402: should-error doesn't catch all errors |
Date: |
Sun, 09 Jul 2017 01:04:24 -0600 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/26.0.50 (gnu/linux) |
Tino Calancha <tino.calancha@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?
- bug#24402: should-error doesn't catch all errors (Was:bug#24402: More Information), Alex, 2017/07/03
- bug#24402: should-error doesn't catch all errors, Alex, 2017/07/04
- bug#24402: should-error doesn't catch all errors, Tino Calancha, 2017/07/05
- bug#24402: should-error doesn't catch all errors, npostavs, 2017/07/11
- bug#24402: should-error doesn't catch all errors, Alex, 2017/07/11
- bug#24402: should-error doesn't catch all errors, npostavs, 2017/07/12
- bug#24402: should-error doesn't catch all errors, Alex, 2017/07/12
- bug#24402: should-error doesn't catch all errors, npostavs, 2017/07/12
- bug#24402: should-error doesn't catch all errors, Alex, 2017/07/12
- bug#24402: should-error doesn't catch all errors, npostavs, 2017/07/12
- bug#24402: should-error doesn't catch all errors, Alex, 2017/07/13
- bug#24402: should-error doesn't catch all errors, npostavs, 2017/07/13
- bug#24402: should-error doesn't catch all errors, Alex, 2017/07/14