bug#46351: 28.0.50; Add convenient way to bypass Eshell's own pipelining

[Michael Albinus]

Sean Whitton <spwhitton@spwhitton.name> writes:

> Hello,

Hi Sean,

> I've got it working.  Please let me know what you think of the attached.

Some comments:

> --- /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.

> +(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.

> +  (deftest em-extpipe-test-7

Looks like em-extpipe-test-6 is missing.

> Sean Whitton

Best regards, Michael.