GNU bug report logs - #57216
29.0.50; [PATCH] Provide ERT explainers for Eshell test functions

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 57216 in the body.
You can then email your comments to 57216 AT debbugs.gnu.org in the normal way.

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

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

From: Jim Porter <jporterbugs <at> gmail.com>
To: bug-gnu-emacs <at> gnu.org
Cc: eliz <at> gnu.org
Subject: 29.0.50; [PATCH] Provide ERT explainers for Eshell test functions
Date: Sun, 14 Aug 2022 14:20:25 -0700

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

It can be difficult to diagnose test failures in the Eshell ERT test 
suite, since it doesn't print useful details like the command that was 
executed. See bug#57129 for example.

Here are some patches to fix that. The failures now look like this:

----------------------------------------

    (ert-test-failed
     ((should
       (eshell-match-output regexp))
      :form
      (eshell-match-output "wrong")
      :value nil :explanation
      (mismatched-output
       (command "echo $INSIDE_EMACS[, 1]\n")
       (output "eshell\n")
       (regexp "wrong"))))

----------------------------------------

    (ert-test-failed
     ((should
       (eshell-command-result--equal command
                                     (eshell-test-command-result command)
                                     result))
      :form
      (eshell-command-result--equal "echo $LINES" 22 2)
      :value nil :explanation
      (nonequal-result
       (command "echo $LINES")
       (result 22)
       (expected 2))))

----------------------------------------

Most of the first two patches are purely-mechanical changes to update 
function calls. The diffs ended up being pretty big, but I think it 
improves matters. Let me know if I should do it differently (especially 
for the commit message; I didn't see much benefit to listing *every* 
test function I touched, but maybe I should do that?)

While I was testing these, I also found a couple real bugs with the 
'eshell-command-result' function. This throws 'eshell-defer':

  M-: (eshell-command-result "if {[ foo = foo ]} {echo hi}")

That's because 'eshell-do-eval' wasn't forwarding the SYNCHRONOUS-P 
argument in a couple spots.

This is also wrong:

  M-: (eshell-command-result "if (zerop 1) {echo yes} {echo no}")
    => nil

It should return "no".

I fixed these in the third patch. Maybe that could go in a separate bug, 
but it's fairly closely related to this one.

[0001-Provide-ERT-explainer-for-eshell-match-command-outpu.patch (text/plain, attachment)]

[0002-Add-eshell-command-result-equal-with-an-ERT-explaine.patch (text/plain, attachment)]

[0003-Fix-non-interactive-use-of-conditionals-in-Eshell.patch (text/plain, attachment)]

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

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Jim Porter <jporterbugs <at> gmail.com>
Cc: eliz <at> gnu.org, 57216 <at> debbugs.gnu.org
Subject: Re: bug#57216: 29.0.50; [PATCH] Provide ERT explainers for Eshell
 test functions
Date: Mon, 15 Aug 2022 09:01:52 +0200

Jim Porter <jporterbugs <at> gmail.com> writes:

> It can be difficult to diagnose test failures in the Eshell ERT test
> suite, since it doesn't print useful details like the command that was
> executed. See bug#57129 for example.

I'm generally not in favour of making ert tests less trivial (because
that makes dealing with them less easy if you're not familiar with the
specific harness), but in this case, it makes sense, I think.

> Most of the first two patches are purely-mechanical changes to update
> function calls. The diffs ended up being pretty big, but I think it
> improves matters. Let me know if I should do it differently
> (especially for the commit message; I didn't see much benefit to
> listing *every* test function I touched, but maybe I should do that?)

Looks OK to me -- we don't really follow the ChangeLog conventions for
the tests much.

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Jim Porter <jporterbugs <at> gmail.com>
Cc: bug-gnu-emacs <at> gnu.org
Subject: Re: 29.0.50; [PATCH] Provide ERT explainers for Eshell test functions
Date: Mon, 15 Aug 2022 14:12:33 +0300

> From: Jim Porter <jporterbugs <at> gmail.com>
> Cc: eliz <at> gnu.org
> Date: Sun, 14 Aug 2022 14:20:25 -0700
> 
> It can be difficult to diagnose test failures in the Eshell ERT test 
> suite, since it doesn't print useful details like the command that was 
> executed. See bug#57129 for example.
> 
> Here are some patches to fix that. The failures now look like this:

Looks good, thank you very much for working on this important aspect
of the test suite.

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

From: Jim Porter <jporterbugs <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 57216 <at> debbugs.gnu.org
Subject: Re: bug#57216: 29.0.50; [PATCH] Provide ERT explainers for Eshell
 test functions
Date: Mon, 15 Aug 2022 20:49:09 -0700

On 8/15/2022 4:12 AM, Eli Zaretskii wrote:
> Looks good, thank you very much for working on this important aspect
> of the test suite.

Thanks, merged as cf873c1a090b95746cf2f2a9a24a5e4bd7cc2cd6.

Note that you might need to remove the *.elc files in test/lisp/eshell, 
since I updated eshell-tests-helpers.el, and the Makefiles aren't smart 
enough to recompile that before any of the tests.

Message #19 received at 57216-done <at> debbugs.gnu.org (full text, mbox):

From: Jim Porter <jporterbugs <at> gmail.com>
To: Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: eliz <at> gnu.org, 57216-done <at> debbugs.gnu.org
Subject: Re: bug#57216: 29.0.50; [PATCH] Provide ERT explainers for Eshell
 test functions
Date: Mon, 15 Aug 2022 20:52:09 -0700

On 8/15/2022 12:01 AM, Lars Ingebrigtsen wrote:
> Jim Porter <jporterbugs <at> gmail.com> writes:
> 
>> It can be difficult to diagnose test failures in the Eshell ERT test
>> suite, since it doesn't print useful details like the command that was
>> executed. See bug#57129 for example.
> 
> I'm generally not in favour of making ert tests less trivial (because
> that makes dealing with them less easy if you're not familiar with the
> specific harness), but in this case, it makes sense, I think.

Agreed. I've tried to keep the tests as simple as I can, though even for 
the simplest of the Eshell tests, I think it's useful to show this info. 
 99% of the tests are "run some Eshell command and make sure it does 
the right thing", and if it fails, the new failure output is probably 
enough to debug it without even looking at the test code.

(Closing this, since I merged the patch.)

Previous Next

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