[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: |
Wed, 12 Jul 2017 21:04:38 -0600 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/26.0.50 (gnu/linux) |
npostavs@users.sourceforge.net writes:
> Alex <agrambot@gmail.com> writes:
>
>> npostavs@users.sourceforge.net writes:
>>
>>> Does it also work when loading the elc version of the test file? (try
>>> 'make check TEST_LOAD_EL=no')
>>
>> Oh, it doesn't load the elc version by default? That's surprising; I
>> think that should be documented in the test README.
>>
>> I get 3 test failures with TEST_LOAD_EL=no, but I don't believe they're
>> because of me. On a mostly clean master (d014a5e15) those 3 also error.
>> One of them is simple to fix (the (require 'subr-x) should not be inside
>> eval-when-compile in dom-tests).
>
> Ah, the `should' blocks inlining during compilation. Is that necessary?
> Probably yes if we expect to catch errors during macroexpansion I guess.
Can you get errors by expanding inlined functions?
Macros are expanded at compile-time with the current patch. If there are
any macroexpansion errors, then the form is altered to be (error <type>
<data>). Perhaps inline functions could work similarly.
Here's a diff to my patch that uses byte-compile-inline-expand. This
fixes the dom-tests case. Do you think it's worth it?
ert-inline.diff
Description: inline
>> The other failing tests are
>> subr-test-backtrace-integration-test and cl-lib-defstruct-record.
>
> Hmm, I'll see if I can fix these.
Thanks. I noticed when byte-compiling cl-lib-tests, I got this warning:
Unused lexical variable ‘cl-struct-foo’.
>>> What about tests like this?
>>>
>>> (ert-deftest check-error-handling ()
>>> (should
>>> (eq 42
>>> (condition-case ()
>>> (/ 1 0)
>>> (arith-error 42)))))
>>
>> It works for me, yes. As long as `debugger' is set to a symbol. I can
>> make it a bit more robust by using an additional defvar in
>> ert--run-test-internal.
>>
>> Are you asking because it doesn't work for you?
>
> No, I'm just trying to explore the edges of this solution. Isn't
> `debugger' bound to a non-symbol while running the the tests? I'm
> confused as to why this solution works.
Yes, that's why there's the second test that checks for error-symbol to
be ert-test-{failed, skipped}. Basically what's happening is that
ert--signal-hook forces the debugger to trigger even inside a
condition-case, but only with a non-symbol `debugger' (since
ert--run-test-internal binds it to a closure), and one of the above two
errors.
The only time this approach fails is when you bind `debugger' to a
non-symbol and also signal ert-test-{failed, skipped}.
This is relatively rare compared to the problems at hand (macro and
argument errors), so unless there are unforeseen issues I think it's an
acceptable stop-gap solution. Hopefully Someone™ can properly fix this
eventually.
- bug#24402: should-error doesn't catch all errors, (continued)
- 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 <=
- 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
- bug#24402: should-error doesn't catch all errors, Alex, 2017/07/14
- bug#24402: should-error doesn't catch all errors, npostavs, 2017/07/15
- bug#24402: should-error doesn't catch all errors, Alex, 2017/07/15
- bug#24402: should-error doesn't catch all errors, npostavs, 2017/07/16
- bug#24402: should-error doesn't catch all errors, Gemini Lasswell, 2017/07/19
- bug#24402: should-error doesn't catch all errors, Alex, 2017/07/19