GNU bug report logs - #24402
25.1.50; testcover-start breaks should-error

Previous Next

Package: emacs;

Reported by: Gemini Lasswell <gazally <at> runbox.com>

Date: Sat, 10 Sep 2016 02:19:01 UTC

Severity: normal

Tags: confirmed, fixed, patch

Found in versions 25.1.50, 26.0.50

Fixed in version 26.1

Done: npostavs <at> users.sourceforge.net

Bug is archived. No further changes may be made.

Full log


View this message in rfc822 format

From: Alex <agrambot <at> gmail.com>
To: npostavs <at> users.sourceforge.net
Cc: Gemini Lasswell <gazally <at> runbox.com>, Tino Calancha <tino.calancha <at> gmail.com>, 24402 <at> debbugs.gnu.org
Subject: bug#24402: should-error doesn't catch all errors
Date: Thu, 13 Jul 2017 16:45:39 -0600
[Message part 1 (text/plain, inline)]
npostavs <at> users.sourceforge.net writes:

> Alex <agrambot <at> 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 (text/x-diff, attachment)]

This bug report was last modified 7 years and 133 days ago.

Previous Next


GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.