GNU bug report logs - #46351
28.0.50; Add convenient way to bypass Eshell's own pipelining

Previous Next

Full log

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

From: Sean Whitton <spwhitton <at> spwhitton.name>
To: Michael Albinus <michael.albinus <at> gmx.de>
Cc: 46351 <at> debbugs.gnu.org
Subject: Re: bug#46351: 28.0.50; Add convenient way to bypass Eshell's own
 pipelining
Date: Wed, 19 Jan 2022 13:54:04 -0700

Hello Michael,

On Wed 19 Jan 2022 at 04:52PM +01, Michael Albinus wrote:

>> --- /dev/null
>> +++ b/test/lisp/eshell/em-extpipe-tests.el
>> @@ -0,0 +1,122 @@
>> +(load (expand-file-name "eshell-tests"
>> +                        (file-name-directory (or load-file-name
>> +                                                 default-directory))))
>
> This is problematic. Loading eshell-tests.el declares also all ert tests
> which are contained in that file. Running em-extpipe-tests.el in batch
> would run also all tests from that file, which is not intended I believe.
>
> A better approach would be to factor out the helper functions from
> eshell-tests.el into an own file, and load it here and in eshell-tests.el.

Good point, I'll factor that out.

>> +(cl-macrolet
>> +    ((deftest (name input expected)
>> +       (let ((result (gensym)))
>> +         `(ert-deftest ,name ()
>> +            (let* ((shell-file-name "sh") (shell-command-switch "-c")
>
> I'm not sure this is the right approach. Why do you change
> shell-file-name  and shell-command-switch? You've spoken in another
> message about Tramp, but I don't understand this. Tramp has its own
> machinery to handle them, via connection-local variables.

The unit tests are all about seeing whether em-extpipe sets up
`eshell-parse-command' to do the right thing.  When it comes to
shell-file-name and shell-command-switch, however, all em-extpipe does
is substitute them in verbatim, using `with-connection-local-variables'.
So there isn't much point in varying the values of the two variables in
the tests.

However, as the values of the two variables show up in the expected
return values of `eshell-parse-command' that are part of the test
definitions, in order to write the tests, I need to know what those two
values will be.  It seemed simplest just to bind them to constants.

I could instead substitute the actual values of those variables into the
expected return values.  It seems to me that would sacrifice readability
of the tests, though.  Am I perhaps missing some other benefit?

>> +  (deftest em-extpipe-test-7
>
> Looks like em-extpipe-test-6 is missing.

Oops, will fix.

Many thanks for the review.

-- 
Sean Whitton

Previous Next

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