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

[Alex]

npostavs@users.sourceforge.net writes:

> Alex <agrambot@gmail.com> writes:
>
>> npostavs@users.sourceforge.net writes:
>>> 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.
>
> I don't think so, just a conditional to decide whether or not to call
> the extra expansion.  Do you think there is anything else?

I was mostly referring to not binding `debugger', but also evaluating
the code "normally" (i.e., not doing expansions first in one
condition-case, evaluating arguments in another, and then the whole form
in a third one).

>> OOC, is there a robust way to check whether or not you're currently
>> byte-compiling?
>
> AFAIK, the usual trick is (bound-and-true-p byte-compile-current-file).
> It's probably good enough for most things.

I believe the below patch does that, though it has some issues.

>> 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?
>
> Nope, I just get the failures on cl-lib-defstruct-record we already
> mentioned.

The segfault appears to have been because I didn't wipe out the elc
files when testing different implementations.

I spent a lot longer than I'd like to admit finding this out. Is there a
reason why "make clean" in the test directory doesn't wipe out elc
files? I don't understand why there's a separate bootstrap-clean that
does this. Can this and TEST_LOAD_EL please be documented in the test
README?

Anyway, I got everything back in order. Sadly, there's a couple extra
tests that now fail for me in the patch that *doesn't* expand inline
functions, and these don't fail for me in a clean master. They are in
eieio-tests (23 and 24).

With the inline expansion, I also get some errors in ert-tests. All of
the errors, with the exception of subr-tests error, seem to be from
cl-defstruct and cl-typep (which is defined by define-inline).

Do you have any ideas? There should be 5 unexpected errors without the
inline expansion, and 6 errors with it. Note that all tests pass in both
cases without "TEST_LOAD_EL=no".

If it's easy to fix the eieio tests and not the other ones, then it
might be better to leave the inline-function expansion out for now.