GNU bug report logs - #30745
26.0.91; ert should macros nest strangely

Previous Next

To reply to this bug, email your comments to 30745 AT debbugs.gnu.org.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: phillip.lord <at> russet.org.uk (Phillip Lord)
To: bug-gnu-emacs <at> gnu.org
Subject: 26.0.91; ert should macros nest strangely
Date: Wed, 07 Mar 2018 21:34:22 +0000

ert should macros seem to behave strangely when nested.

Consider the following reproduction:

(require 'cl-lib)
(require 'ert)

(princ
 (cl-cdadr
  (ert-test-result-with-condition-condition
   (ert-run-test
    (make-ert-test
     :body
     (lambda () (should (eq 1 2))))))))

(princ "\nSecond test\n")

(princ
 (should
  (equal
   (cl-cdadr
    (ert-test-result-with-condition-condition
     (ert-run-test
      (make-ert-test
       :body
       (lambda () (should (eq 1 2)))))))
   '(:form (eq 1 2) :value nil))))

(princ "\n")

In Emacs-25 the following behaviour is seen when running this in batch.

~/src/git/emacs-git/emacs-25$ ./src/emacs --batch -l temp.el
(:form (eq 1 2) :value nil)
Second test
t


In Emacs-26, however, we get a different behaviour:


~/src/git/emacs-git/emacs-26$ ./phil_emacs.sh 
(:form (eq 1 2) :value nil)
Second test
Test failed: ((should (equal (cl-cdadr (ert-test-result-with-condition-condition (ert-run-test (make-ert-test :body (lambda nil (should (eq 1 2))))))) (quote (:form (eq 1 2) :value nil)))) :form (equal nil (:form (eq 1 2) :value nil)) :value nil :explanation (different-types nil (:form (eq 1 2) :value nil)))


So, although, the return type of this lot:

 (cl-cdadr
  (ert-test-result-with-condition-condition
   (ert-run-test
    (make-ert-test
     :body
     (lambda () (should (eq 1 2))))))))

remains the same between Emacs-25 and Emacs-26, a test for this breaks.

This regression is caused by:

commit 054c198c120c1f01a8ff753892d52710b740acc6

found by bisection. The regression continues on master.

I will try and find a simpler reproduction.

Message #8 received at 30745 <at> debbugs.gnu.org (full text, mbox):

From: phillip.lord <at> russet.org.uk (Phillip Lord)
To: 30745 <at> debbugs.gnu.org
Subject: Re: bug#30745: 26.0.91; ert should macros nest strangely
Date: Wed, 07 Mar 2018 22:24:38 +0000

This reproduction with a simpler test shows that the problem is nesting.

(require 'ert)

(defun simple-test ()
  (ert-test-result-with-condition-condition
   (ert-run-test
    (make-ert-test
     :body
     (lambda () (should (eq 1 2)))))))

(princ (simple-test))

(princ "\nAnd forcibly unnested\n")

(princ
 (let ((res (simple-test)))
   (should
    (equal
     '(ert-test-failed
       ((should (eq 1 2)) :form (eq 1 2) :value nil))
     res))))


(princ "\nAnd nested\n")

(princ
 (should
  (equal
   '(ert-test-failed
     ((should (eq 1 2)) :form (eq 1 2) :value nil))
   (simple-test))))

(princ "\n")


Emacs-25

(ert-test-failed ((should (eq 1 2)) :form (eq 1 2) :value nil))
And forcibly unnested
t
And nested
t


Emacs-26

(ert-test-failed ((should (eq 1 2)) :form (eq 1 2) :value nil))
And forcibly unnested
t
And nested
Test failed: ((should (equal (quote (ert-test-failed ((should (eq 1 2)) :form (eq 1 2) :value nil))) (simple-test))) :form (equal (ert-test-failed ((should (eq 1 2)) :form (eq 1 2) :value nil)) (((should (eq 1 2)) :form (eq 1 2) :value nil))) :value nil :explanation (proper-lists-of-different-length 2 1 (ert-test-failed ((should (eq 1 2)) :form (eq 1 2) :value nil)) (((should (eq 1 2)) :form (eq 1 2) :value nil)) first-mismatch-at 0))

Message #11 received at 30745 <at> debbugs.gnu.org (full text, mbox):

From: Noam Postavsky <npostavs <at> gmail.com>
To: phillip.lord <at> russet.org.uk (Phillip Lord)
Cc: 30745 <at> debbugs.gnu.org
Subject: Re: bug#30745: 26.0.91; ert should macros nest strangely
Date: Wed, 07 Mar 2018 19:12:09 -0500

phillip.lord <at> russet.org.uk (Phillip Lord) writes:

> This regression is caused by:
>
> commit 054c198c120c1f01a8ff753892d52710b740acc6

Hmm, according to https://debbugs.gnu.org/cgi/bugreport.cgi?bug=24402#56
we thought nesting should still have worked, but apparently that was too
optimistic.

Message #14 received at 30745 <at> debbugs.gnu.org (full text, mbox):

From: phillip.lord <at> russet.org.uk (Phillip Lord)
To: Noam Postavsky <npostavs <at> gmail.com>
Cc: 30745 <at> debbugs.gnu.org, 24402 <at> debbugs.gnu.org
Subject: Re: bug#30745: 26.0.91; ert should macros nest strangely
Date: Thu, 08 Mar 2018 08:54:41 +0000

Noam Postavsky <npostavs <at> gmail.com> writes:

> phillip.lord <at> russet.org.uk (Phillip Lord) writes:
>
>> This regression is caused by:
>>
>> commit 054c198c120c1f01a8ff753892d52710b740acc6
>
> Hmm, according to https://debbugs.gnu.org/cgi/bugreport.cgi?bug=24402#56
> we thought nesting should still have worked, but apparently that was too
> optimistic.


Unfortunately, yes. I didn't try nesting two ert-deftest macros, just
two should's. As the original bug report suggests it's for testing a
test library (https://github.com/phillord/assess).

I do have a work around now (which is to unnest the shoulds); and
fortunately, this work-around is backward compatible which is important
for me. I still have another test failure in my library,
though. Probably caused by the same thing but I haven't worked on it
yet.

Phil

Message #17 received at 30745 <at> debbugs.gnu.org (full text, mbox):

From: phillip.lord <at> russet.org.uk (Phillip Lord)
To: 24402 <at> debbugs.gnu.org
Cc: 30745 <at> debbugs.gnu.org
Subject: Re: bug#30745: 26.0.91; ert should macros nest strangely
Date: Thu, 08 Mar 2018 14:49:27 +0000

Just noting that the fix for this bug has caused a regression.

Message #20 received at 30745 <at> debbugs.gnu.org (full text, mbox):

From: Noam Postavsky <npostavs <at> gmail.com>
To: phillip.lord <at> russet.org.uk (Phillip Lord)
Cc: 30745 <at> debbugs.gnu.org, 24402 <at> debbugs.gnu.org
Subject: Re: bug#30745: 26.0.91; ert should macros nest strangely
Date: Tue, 14 Aug 2018 20:47:21 -0400

[Message part 1 (text/plain, inline)]

block 30745 by 24618
tags 30745 + patch
quit

phillip.lord <at> russet.org.uk (Phillip Lord) writes:

> Unfortunately, yes. I didn't try nesting two ert-deftest macros, just
> two should's. As the original bug report suggests it's for testing a
> test library (https://github.com/phillord/assess).
>
> I do have a work around now (which is to unnest the shoulds); and
> fortunately, this work-around is backward compatible which is important
> for me. I still have another test failure in my library,
> though. Probably caused by the same thing but I haven't worked on it
> yet.

I have a patch for ert which removes it's (ab)use of the debugger, it
seems to fix this case (and also Bug#11218).  Note that it relies on my
patch in #24618 for a catch-all condition-case handler clause.

https://debbugs.gnu.org/cgi/bugreport.cgi?bug=24618;filename=0001-Allow-t-as-a-catch-all-condition-case-handler-Bug-24.patch;msg=8;att=1

[v1-0001-Use-signal-hook-function-in-ert-not-debugger.patch (text/plain, attachment)]

[Message part 3 (text/plain, inline)]

But it bumps into another bug lurking in process.c, which just happened
to work okay previously because ert would bind debug-on-error while
running tests.  There is only one test in the Emacs test suite which
triggers this, we can work around it by binding debug-on-error there:

[v1-0002-Work-around-should-error-.-accept-process-output-.patch (text/plain, attachment)]

[Message part 5 (text/plain, inline)]

This third patch is not really needed to fix the bug, but I had to
simplify the code in order to figure out what was going on anyway.

[v1-0003-Simplify-ert-should-expansion-macro-machinery.patch (text/plain, attachment)]

Message #27 received at 30745 <at> debbugs.gnu.org (full text, mbox):

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Noam Postavsky <npostavs <at> gmail.com>
Cc: 30745 <at> debbugs.gnu.org, 24402 <at> debbugs.gnu.org,
 Phillip Lord <phillip.lord <at> russet.org.uk>
Subject: Re: bug#30745: 26.0.91; ert should macros nest strangely
Date: Mon, 24 Jun 2019 20:54:18 +0200

Noam Postavsky <npostavs <at> gmail.com> writes:

> I have a patch for ert which removes it's (ab)use of the debugger, it
> seems to fix this case (and also Bug#11218).  Note that it relies on my
> patch in #24618 for a catch-all condition-case handler clause.

#24618 was fixed a few weeks later, but this patch was never applied?

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

Message #30 received at 30745 <at> debbugs.gnu.org (full text, mbox):

From: Noam Postavsky <npostavs <at> gmail.com>
To: Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: 30745 <at> debbugs.gnu.org, Phillip Lord <phillip.lord <at> russet.org.uk>,
 24402 <at> debbugs.gnu.org
Subject: Re: bug#30745: 26.0.91; ert should macros nest strangely
Date: Mon, 24 Jun 2019 18:53:00 -0400

Lars Ingebrigtsen <larsi <at> gnus.org> writes:

> Noam Postavsky <npostavs <at> gmail.com> writes:
>
>> I have a patch for ert which removes it's (ab)use of the debugger, it
>> seems to fix this case (and also Bug#11218).  Note that it relies on my
>> patch in #24618 for a catch-all condition-case handler clause.
>
> #24618 was fixed a few weeks later, but this patch was never applied?

The main blocker for this is the other thing:

>> But it bumps into another bug lurking in process.c, which just happened
>> to work okay previously because ert would bind debug-on-error while
>> running tests.

Message #33 received at 30745 <at> debbugs.gnu.org (full text, mbox):

From: Noam Postavsky <npostavs <at> gmail.com>
To: Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: 30745 <at> debbugs.gnu.org, 24402 <at> debbugs.gnu.org,
 Phillip Lord <phillip.lord <at> russet.org.uk>
Subject: Re: bug#30745: 26.0.91; ert should macros nest strangely
Date: Thu, 27 Jun 2019 23:06:00 -0400

Noam Postavsky <npostavs <at> gmail.com> writes:

> The main blocker for this is the other thing:
>
>>> But it bumps into another bug lurking in process.c, which just happened
>>> to work okay previously because ert would bind debug-on-error while
>>> running tests.

Also it seems to be breaking some electric tests

   FAILED  electric-layout-plainer-c-mode-use-c-style
   FAILED  electric-modes-int-main-allman-style

I don't really have any clue why though.

Message #36 received at 30745 <at> debbugs.gnu.org (full text, mbox):

From: Philipp Stephani <p.stephani2 <at> gmail.com>
To: Noam Postavsky <npostavs <at> gmail.com>
Cc: Lars Ingebrigtsen <larsi <at> gnus.org>,
 Phillip Lord <phillip.lord <at> russet.org.uk>, 30745 <at> debbugs.gnu.org,
 24402 <at> debbugs.gnu.org
Subject: Re: bug#30745: 26.0.91; ert should macros nest strangely
Date: Sun, 29 Nov 2020 11:35:43 +0100

Am Di., 25. Juni 2019 um 00:54 Uhr schrieb Noam Postavsky <npostavs <at> gmail.com>:
>
> Lars Ingebrigtsen <larsi <at> gnus.org> writes:
>
> > Noam Postavsky <npostavs <at> gmail.com> writes:
> >
> >> I have a patch for ert which removes it's (ab)use of the debugger, it
> >> seems to fix this case (and also Bug#11218).  Note that it relies on my
> >> patch in #24618 for a catch-all condition-case handler clause.
> >
> > #24618 was fixed a few weeks later, but this patch was never applied?
>
> The main blocker for this is the other thing:
>
> >> But it bumps into another bug lurking in process.c, which just happened
> >> to work okay previously because ert would bind debug-on-error while
> >> running tests.
>

How many tests (besides the jsonrpc one) do we expect to be broken due
to this? It seems that tests relying on such a bug are inherently
brittle, so maybe we can bite the bullet and apply the patch
nevertheless? Or introduce another hack in process.c that treats ERT
tests specially, as if debug-on-error were bound?

Message #39 received at 30745 <at> debbugs.gnu.org (full text, mbox):

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Philipp Stephani <p.stephani2 <at> gmail.com>
Cc: 30745 <at> debbugs.gnu.org, 24402 <at> debbugs.gnu.org,
 Noam Postavsky <npostavs <at> gmail.com>, Phillip Lord <phillip.lord <at> russet.org.uk>
Subject: Re: bug#30745: 26.0.91; ert should macros nest strangely
Date: Thu, 22 Jul 2021 15:13:29 +0200

Philipp Stephani <p.stephani2 <at> gmail.com> writes:

> How many tests (besides the jsonrpc one) do we expect to be broken due
> to this? It seems that tests relying on such a bug are inherently
> brittle, so maybe we can bite the bullet and apply the patch
> nevertheless? Or introduce another hack in process.c that treats ERT
> tests specially, as if debug-on-error were bound?

I wanted to try Noam's patch set, but it's two years old, so it doesn't
apply any more.  Noam, do you have an updated patch set, by any chance?

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

Previous Next

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