bug#24402: should-error doesn't catch all errors

[Alex]

npostavs@users.sourceforge.net writes:

> Alex <agrambot@gmail.com> writes:
>
>> 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?
>
> It would be nice if we can make code inside tests behave the same as
> outside.  But we should make it conditional on whether the code is being
> compiled, otherwise code inside tests would behave differently when
> being interpreted.  Anyway, we can leave this for a separate bug.

I agree, but that sounds like it'll require a fair bit of refactoring
and knowledge of ert internals.

OOC, is there a robust way to check whether or not you're currently
byte-compiling?

>> 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}.
>
> Okay, it sounds like we would only hit problems when running an ert test
> from inside an ert test.  That should be pretty rare outside of ert's
> test suite, and we already have workarounds for that case, right?

I tried a nested case using two ert-deftests and it worked, so it looks
okay here too.

>> 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.
>
> Yes, I think this approach is acceptable.

I was going to ask if you would merge in a few days, but it appears that
what should have been a simple rebase to master caused unforeseen
consequences. For instance, for some reason I now get a segmentation
fault when executing 'make cl-lib-tests TEST_LOAD_EL=no'. I even reset
to the commit I was at before and it still segfaults. Can you reproduce
this with the following patch on master?

0001-Catch-argument-and-expansion-errors-in-ert.patch
Description: ert