[Top][All Lists]

[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: Tue, 04 Jul 2017 18:00:04 -0600
User-agent: Gnus/5.13 (Gnus v5.13) Emacs/26.0.50 (gnu/linux)

tags 24402 patch

Alex <address@hidden> writes:

> Gemini Lasswell <address@hidden> writes:
>> I’ve done some investigating of why this is happening. testcover-start
>> transforms:
>>     (should-error (my-error))
>> into:
>>     (should-error (testcover-after 2 (my-error)))
>> Then the macro expansion of should-error separates the form which it
>> is passed into a function symbol and list of arguments, and applies
>> the function to the arguments inside of a condition-case that traps
>> errors. The problem is that the arguments are evaluated first, outside
>> of the condition-case, so errors in their evaluation do not get
>> caught. This problem is not specific to testcover. In this example,
>> the first test passes and the second fails:
>> (defun div-by (n)
>>   (/ 100 n))
>> (defmacro log-div-by (n)
>>   `(message "div-by: %d" (div-by ,n)))
>> (ert-deftest test-div-by ()
>>   (should-error (div-by 0)))
>> (ert-deftest test-log-div-by ()
>>   (should-error (log-div-by 0)))
> I just ran into this as well. Consider these two forms:
> (should-error (cl-fourth "1234") :type 'wrong-type-argument)
> (should-error (car (cdr (cdr (cdr "1234")))) :type 'wrong-type-argument)
> Only the second raises an error, even though cl-fourth is equivalent to
> the car/cdr chain.

Here's a diff that appears to fix this. It's a bit crude, but I'd rather
not mess around too much with ert's internals.

Attachment: ert-args.diff
Description: first

There's a similar issue with macro-expansions inside of
should/should-error/should-not that could/should be fixed by wrapping
the macroexpand call at the top of ert--expand-should-1 in a similar
condition-case. Here's another diff that does just that:

Attachment: ert-args2.diff
Description: second

I ran "make check" and found only one test that the above diff breaks:

I can't seem to figure out why it doesn't work. The test fails because
of these two:

(let ((test (make-ert-test :body (lambda () (ert-fail "failed")))))
  (should-not (ert-test-result-expected-p test (ert-run-test test))))
(let ((test (make-ert-test :body (lambda () (ert-fail "failed"))
                           :expected-result-type ':failed)))
  (should (ert-test-result-expected-p test (ert-run-test test))))

I tried to re-throw the ert-test-failed signal and still the above two
forms raise error an error.

reply via email to

[Prev in Thread] Current Thread [Next in Thread]