[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
bug#24402: should-error doesn't catch all errors
From: |
Tino Calancha |
Subject: |
bug#24402: should-error doesn't catch all errors |
Date: |
Fri, 07 Jul 2017 19:15:13 +0900 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/26.0.50 (gnu/linux) |
Alex <agrambot@gmail.com> writes:
> I don't think your tweak will work in all cases, namely whenever
> (list ,@arg-forms) has side-effects. Luckily this doesn't happen in any
> current tests, but I think those types of tests should be supported.
Good point.
> I believe the following is why my previous diff doesn't work. Consider:
>
> (let ((test (make-ert-test :body (lambda () (ert-fail "failed")))))
> (ert-run-test test))
>
> The above returns a struct/record and does not error.
>
> (let ((test (make-ert-test :body (lambda () (ert-fail "failed")))))
> (condition-case err
> (ert-run-test test)
> (error "woops")))
>
> Even though ert-run-test by itself does not error, the error handler is
> ran. I believe this is because `ert--run-test-internal' binds `debugger'
> around the evaluation of the test to somehow suppress this error.
>
> Using condition-case-unless-debug gets around this, but now anytime
> debug-on-error is non-nil the condition-case won't catch the error. I'm
> not sure how to solve this.
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 ammend the failing tests wrapping '> I don't think your tweak will work
in all cases, namely whenever
> (list ,@arg-forms) has side-effects. Luckily this doesn't happen in any
> current tests, but I think those types of tests should be supported.
Good point.
> I believe the following is why my previous diff doesn't work. Consider:
>
> (let ((test (make-ert-test :body (lambda () (ert-fail "failed")))))
> (ert-run-test test))
>
> The above returns a struct/record and does not error.
>
> (let ((test (make-ert-test :body (lambda () (ert-fail "failed")))))
> (condition-case err
> (ert-run-test test)
> (error "woops")))
>
> Even though ert-run-test by itself does not error, the error handler is
> ran. I believe this is because `ert--run-test-internal' binds `debugger'
> around the evaluation of the test to somehow suppress this error.
>
> Using condition-case-unless-debug gets around this, but now anytime
> debug-on-error is non-nil the condition-case won't catch the error. I'm
> not sure how to solve this.
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?
--8<-----------------------------cut here---------------start------------->8---
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))))
--8<-----------------------------cut here---------------end--------------->8---
- 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