all messages for Emacs-related lists mirrored at yhetil.org
 help / color / mirror / code / Atom feed
* bug#62578: 30.0.50; [PATCH] Add regression tests for synchronous processes in Eshell
@ 2023-04-01  4:41 Jim Porter
  2023-04-01  6:07 ` Eli Zaretskii
  0 siblings, 1 reply; 9+ messages in thread
From: Jim Porter @ 2023-04-01  4:41 UTC (permalink / raw)
  To: 62578

[-- Attachment #1: Type: text/plain, Size: 522 bytes --]

As far as I understand it, Eshell only uses synchronous processes on 
MS-DOS (where 'make-process' doesn't exist). Since I don't build Emacs 
for MS-DOS, and I expect not many others do either, I'm worried that 
this support will regress. To avoid that, here are some regression tests.

It also fixes a small bug with how Eshell updated its internal markers 
when using synchronous processes. This mostly affects the tests, but 
could also cause unexpected behavior when using some of Eshell's 
buffer-navigation commands.

[-- Attachment #2: 0001-Add-tests-for-synchronous-processes-in-Eshell.patch --]
[-- Type: text/plain, Size: 5013 bytes --]

From f6a0f2d74fcac44a4faeb3c0b73184d5498ec525 Mon Sep 17 00:00:00 2001
From: Jim Porter <jporterbugs@gmail.com>
Date: Fri, 31 Mar 2023 21:32:44 -0700
Subject: [PATCH] ; Add tests for synchronous processes in Eshell

Normally, Eshell only uses synchronous processes on MS-DOS, so this is
hard to test.  To get around this, let the tests explicitly request
synchronous processes.

* lisp/eshell/esh-proc.el (eshell-supports-asynchronous-processes):
New variable...
(eshell-gather-process-output): ... use it, and remove some incorrect
code updating Eshell's internal markers (the async code path doesn't
do this, so neither should the sync path).

* lisp/eshell/esh-cmd.el (eshell-execute-pipeline): Use
'eshell-supports-asynchronous-processes'.

* test/lisp/eshell/esh-proc-tests.el
(esh-proc-test/synchronous-proc/simple/interactive)
(esh-proc-test/synchronous-proc/simple/command-result)
(esh-proc-test/synchronous-proc/pipeline/interactive)
(esh-proc-test/synchronous-proc/pipeline/command-result): New tests.
---
 lisp/eshell/esh-cmd.el             |  2 +-
 lisp/eshell/esh-proc.el            |  9 ++++----
 test/lisp/eshell/esh-proc-tests.el | 35 ++++++++++++++++++++++++++++++
 3 files changed, 40 insertions(+), 6 deletions(-)

diff --git a/lisp/eshell/esh-cmd.el b/lisp/eshell/esh-cmd.el
index d5237ee1f04..b80596f2cad 100644
--- a/lisp/eshell/esh-cmd.el
+++ b/lisp/eshell/esh-cmd.el
@@ -892,7 +892,7 @@ eshell-execute-pipeline
      (set headproc nil)
      (set tailproc nil)
      (progn
-       ,(if (fboundp 'make-process)
+       ,(if eshell-supports-asynchronous-processes
 	    `(eshell-do-pipelines ,pipeline)
           `(let ((tail-handles (eshell-duplicate-handles
                                 eshell-current-handles)))
diff --git a/lisp/eshell/esh-proc.el b/lisp/eshell/esh-proc.el
index a86e7502795..2976f5694d7 100644
--- a/lisp/eshell/esh-proc.el
+++ b/lisp/eshell/esh-proc.el
@@ -97,6 +97,9 @@ eshell-kill-hook
 
 ;;; Internal Variables:
 
+(defvar eshell-supports-asynchronous-processes (fboundp 'make-process)
+  "Non-nil if Eshell can create asynchronous processes.")
+
 (defvar eshell-current-subjob-p nil)
 
 (defvar eshell-process-list nil
@@ -296,7 +299,7 @@ eshell-gather-process-output
                 (coding-system-change-eol-conversion locale-coding-system
                                                      'unix))))
     (cond
-     ((fboundp 'make-process)
+     (eshell-supports-asynchronous-processes
       (unless (or ;; FIXME: It's not currently possible to use a
                   ;; stderr process for remote files.
                   (file-remote-p default-directory)
@@ -392,10 +395,6 @@ eshell-gather-process-output
 	    (setq lbeg lend)
 	    (set-buffer proc-buf))
 	  (set-buffer oldbuf))
-        (require 'esh-mode)
-        (declare-function eshell-update-markers "esh-mode" (pmark))
-        (defvar eshell-last-output-end)         ;Defined in esh-mode.el.
-	(eshell-update-markers eshell-last-output-end)
 	;; Simulate the effect of eshell-sentinel.
 	(eshell-close-handles
          (if (numberp exit-status) exit-status -1)
diff --git a/test/lisp/eshell/esh-proc-tests.el b/test/lisp/eshell/esh-proc-tests.el
index 8e02fbb5497..8399fbdeeb9 100644
--- a/test/lisp/eshell/esh-proc-tests.el
+++ b/test/lisp/eshell/esh-proc-tests.el
@@ -191,6 +191,41 @@ esh-proc-test/pipeline-connection-type/last
      (unless (eq system-type 'windows-nt)
        "stdout\nstderr\n"))))
 
+\f
+;; Synchronous processes
+
+(ert-deftest esh-proc-test/synchronous-proc/simple/interactive ()
+  "Test that synchronous processes work in an interactive shell."
+  (skip-unless (executable-find "echo"))
+  (let ((eshell-supports-asynchronous-processes nil))
+    (with-temp-eshell
+     (eshell-match-command-output "*echo hello"
+                                  "\\`hello\n"))))
+
+(ert-deftest esh-proc-test/synchronous-proc/simple/command-result ()
+  "Test that synchronous processes work via `eshell-command-result'."
+  (skip-unless (executable-find "echo"))
+  (let ((eshell-supports-asynchronous-processes nil))
+    (eshell-command-result-equal "*echo hello"
+                                 "hello\n")))
+
+(ert-deftest esh-proc-test/synchronous-proc/pipeline/interactive ()
+  "Test that synchronous pipelines work in an interactive shell."
+  (skip-unless (and (executable-find "echo")
+                    (executable-find "rev")))
+  (let ((eshell-supports-asynchronous-processes nil))
+    (with-temp-eshell
+     (eshell-match-command-output "*echo hello | *rev"
+                                  "\\`olleh\n"))))
+
+(ert-deftest esh-proc-test/synchronous-proc/pipeline/command-result ()
+  "Test that synchronous pipelines work via `eshell-command-result'."
+  (skip-unless (and (executable-find "echo")
+                    (executable-find "rev")))
+  (let ((eshell-supports-asynchronous-processes nil))
+    (eshell-command-result-equal "*echo hello | *rev"
+                                 "olleh\n")))
+
 \f
 ;; Killing processes
 
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* bug#62578: 30.0.50; [PATCH] Add regression tests for synchronous processes in Eshell
  2023-04-01  4:41 bug#62578: 30.0.50; [PATCH] Add regression tests for synchronous processes in Eshell Jim Porter
@ 2023-04-01  6:07 ` Eli Zaretskii
  2023-04-01  7:16   ` Jim Porter
  0 siblings, 1 reply; 9+ messages in thread
From: Eli Zaretskii @ 2023-04-01  6:07 UTC (permalink / raw)
  To: Jim Porter; +Cc: 62578

> Date: Fri, 31 Mar 2023 21:41:09 -0700
> From: Jim Porter <jporterbugs@gmail.com>
> 
> As far as I understand it, Eshell only uses synchronous processes on 
> MS-DOS (where 'make-process' doesn't exist). Since I don't build Emacs 
> for MS-DOS, and I expect not many others do either, I'm worried that 
> this support will regress. To avoid that, here are some regression tests.

Thanks.

> +(ert-deftest esh-proc-test/synchronous-proc/simple/interactive ()
> +  "Test that synchronous processes work in an interactive shell."
> +  (skip-unless (executable-find "echo"))

This will always skip on MS-DOS, since "echo" is not available as an
external program OOTB, only if GNU Coreutils are installed.  And even
if Coreutils _are_ installed, a command that invokes "echo" will most
probably invoke the shell builtin instead.

> +  (skip-unless (and (executable-find "echo")
> +                    (executable-find "rev")))

Likewise here: "rev" is not expected to be available on MS-DOS.

I think you need to rethink these tests in a way that uses different
commands.  My suggestion is to use the Emacs executable, since that is
always available when running these tests.





^ permalink raw reply	[flat|nested] 9+ messages in thread

* bug#62578: 30.0.50; [PATCH] Add regression tests for synchronous processes in Eshell
  2023-04-01  6:07 ` Eli Zaretskii
@ 2023-04-01  7:16   ` Jim Porter
  2023-04-01  7:22     ` Eli Zaretskii
  0 siblings, 1 reply; 9+ messages in thread
From: Jim Porter @ 2023-04-01  7:16 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 62578

On 3/31/2023 11:07 PM, Eli Zaretskii wrote:
>> Date: Fri, 31 Mar 2023 21:41:09 -0700
>> From: Jim Porter <jporterbugs@gmail.com>
>>
>> +(ert-deftest esh-proc-test/synchronous-proc/simple/interactive ()
>> +  "Test that synchronous processes work in an interactive shell."
>> +  (skip-unless (executable-find "echo"))
> 
> This will always skip on MS-DOS, since "echo" is not available as an
> external program OOTB, only if GNU Coreutils are installed.  And even
> if Coreutils _are_ installed, a command that invokes "echo" will most
> probably invoke the shell builtin instead.

My main goal here is to test synchronous subprocesses on platforms 
*other than* MS-DOS, since these new tests aren't really necessary on 
MS-DOS itself: there are plenty of existing Eshell tests that create 
processes, and they'd *all* use synchronous subprocesses on MS-DOS.

As for calling a shell builtin, Eshell should translate "*echo" to 
"/path/to/echo" (or "C:/path/to/echo.exe") before calling it, so it 
shouldn't use a shell builtin here. You're right though that the tests 
probably require GNU Coreutils to be installed (this is also true of 
quite a few existing Eshell tests though).

>> +  (skip-unless (and (executable-find "echo")
>> +                    (executable-find "rev")))
> 
> Likewise here: "rev" is not expected to be available on MS-DOS.

To make this work in more places, I could switch "rev" to something 
that's actually in GNU Coreutils. Maybe "wc".

> I think you need to rethink these tests in a way that uses different
> commands.  My suggestion is to use the Emacs executable, since that is
> always available when running these tests.

Since these tests are meant to check the "synchronous subprocess" code 
in Eshell on non-MS-DOS platforms, I'd say it's ok. However, I can 
change my patch if you prefer. I could either:

1) Add a comment to the tests explaining that they're just meant to 
simulate some of MS-DOS's limitations on non-MS-DOS systems, or

2) Rework these tests so they work the same on both MS-DOS and other 
systems.

Personally, I lean softly towards (1), partly because the Eshell test 
suite probably breaks in quite a few other places on MS-DOS anyway. 
However, it shouldn't be too hard to do (2) instead.

What do you think?





^ permalink raw reply	[flat|nested] 9+ messages in thread

* bug#62578: 30.0.50; [PATCH] Add regression tests for synchronous processes in Eshell
  2023-04-01  7:16   ` Jim Porter
@ 2023-04-01  7:22     ` Eli Zaretskii
  2023-04-01  7:40       ` Jim Porter
  0 siblings, 1 reply; 9+ messages in thread
From: Eli Zaretskii @ 2023-04-01  7:22 UTC (permalink / raw)
  To: Jim Porter; +Cc: 62578

> Date: Sat, 1 Apr 2023 00:16:38 -0700
> Cc: 62578@debbugs.gnu.org
> From: Jim Porter <jporterbugs@gmail.com>
> 
> Since these tests are meant to check the "synchronous subprocess" code 
> in Eshell on non-MS-DOS platforms, I'd say it's ok. However, I can 
> change my patch if you prefer. I could either:
> 
> 1) Add a comment to the tests explaining that they're just meant to 
> simulate some of MS-DOS's limitations on non-MS-DOS systems, or
> 
> 2) Rework these tests so they work the same on both MS-DOS and other 
> systems.
> 
> Personally, I lean softly towards (1), partly because the Eshell test 
> suite probably breaks in quite a few other places on MS-DOS anyway. 
> However, it shouldn't be too hard to do (2) instead.
> 
> What do you think?

I don't understand why not use Emacs instead of all those external
commands.  That solves all the problems nicely and portably, and still
allows you to do anything you want.

But it's your call, eventually.





^ permalink raw reply	[flat|nested] 9+ messages in thread

* bug#62578: 30.0.50; [PATCH] Add regression tests for synchronous processes in Eshell
  2023-04-01  7:22     ` Eli Zaretskii
@ 2023-04-01  7:40       ` Jim Porter
  2023-04-02  0:58         ` Jim Porter
  0 siblings, 1 reply; 9+ messages in thread
From: Jim Porter @ 2023-04-01  7:40 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 62578

On 4/1/2023 12:22 AM, Eli Zaretskii wrote:
> I don't understand why not use Emacs instead of all those external
> commands.  That solves all the problems nicely and portably, and still
> allows you to do anything you want.

Mostly just so that the tests are easier to understand, but for these 
newly-added ones, I think they should be easy enough to understand no 
matter what. I'll try using Emacs.

My only concern there is making Emacs do something reasonable when 
piping data into it, but I can just try a few things out, and if it 
really doesn't work, the existing tests in my patch are hopefully still 
better than nothing.





^ permalink raw reply	[flat|nested] 9+ messages in thread

* bug#62578: 30.0.50; [PATCH] Add regression tests for synchronous processes in Eshell
  2023-04-01  7:40       ` Jim Porter
@ 2023-04-02  0:58         ` Jim Porter
  2023-04-02  5:29           ` Eli Zaretskii
  0 siblings, 1 reply; 9+ messages in thread
From: Jim Porter @ 2023-04-02  0:58 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 62578

[-- Attachment #1: Type: text/plain, Size: 325 bytes --]

On 4/1/2023 12:40 AM, Jim Porter wrote:
> My only concern there is making Emacs do something reasonable when 
> piping data into it, but I can just try a few things out, and if it 
> really doesn't work, the existing tests in my patch are hopefully still 
> better than nothing.

Ok, that wasn't too hard. How does this look?

[-- Attachment #2: 0001-Add-tests-for-synchronous-processes-in-Eshell.patch --]
[-- Type: text/plain, Size: 5975 bytes --]

From f5e8fad60c00acbb8cd7cd82acf1bae21e01aafb Mon Sep 17 00:00:00 2001
From: Jim Porter <jporterbugs@gmail.com>
Date: Fri, 31 Mar 2023 21:32:44 -0700
Subject: [PATCH] ; Add tests for synchronous processes in Eshell

Normally, Eshell only uses synchronous processes on MS-DOS, so this is
hard to test.  To get around this, let the tests explicitly request
synchronous processes.

* lisp/eshell/esh-proc.el (eshell-supports-asynchronous-processes):
New variable...
(eshell-gather-process-output): ... use it, and remove some incorrect
code updating Eshell's internal markers (the async code path doesn't
do this, so neither should the sync path).

* lisp/eshell/esh-cmd.el (eshell-execute-pipeline): Use
'eshell-supports-asynchronous-processes'.

* test/lisp/eshell/esh-proc-tests.el
(esh-proc-test/emacs-command): New function.
(esh-proc-test/emacs-echo, esh-proc-test/emacs-upcase): New variables.
(esh-proc-test/synchronous-proc/simple/interactive)
(esh-proc-test/synchronous-proc/simple/command-result)
(esh-proc-test/synchronous-proc/pipeline/interactive)
(esh-proc-test/synchronous-proc/pipeline/command-result): New tests.
---
 lisp/eshell/esh-cmd.el             |  2 +-
 lisp/eshell/esh-proc.el            |  9 +++---
 test/lisp/eshell/esh-proc-tests.el | 52 ++++++++++++++++++++++++++++++
 3 files changed, 57 insertions(+), 6 deletions(-)

diff --git a/lisp/eshell/esh-cmd.el b/lisp/eshell/esh-cmd.el
index d5237ee1f04..b80596f2cad 100644
--- a/lisp/eshell/esh-cmd.el
+++ b/lisp/eshell/esh-cmd.el
@@ -892,7 +892,7 @@ eshell-execute-pipeline
      (set headproc nil)
      (set tailproc nil)
      (progn
-       ,(if (fboundp 'make-process)
+       ,(if eshell-supports-asynchronous-processes
 	    `(eshell-do-pipelines ,pipeline)
           `(let ((tail-handles (eshell-duplicate-handles
                                 eshell-current-handles)))
diff --git a/lisp/eshell/esh-proc.el b/lisp/eshell/esh-proc.el
index a86e7502795..2976f5694d7 100644
--- a/lisp/eshell/esh-proc.el
+++ b/lisp/eshell/esh-proc.el
@@ -97,6 +97,9 @@ eshell-kill-hook
 
 ;;; Internal Variables:
 
+(defvar eshell-supports-asynchronous-processes (fboundp 'make-process)
+  "Non-nil if Eshell can create asynchronous processes.")
+
 (defvar eshell-current-subjob-p nil)
 
 (defvar eshell-process-list nil
@@ -296,7 +299,7 @@ eshell-gather-process-output
                 (coding-system-change-eol-conversion locale-coding-system
                                                      'unix))))
     (cond
-     ((fboundp 'make-process)
+     (eshell-supports-asynchronous-processes
       (unless (or ;; FIXME: It's not currently possible to use a
                   ;; stderr process for remote files.
                   (file-remote-p default-directory)
@@ -392,10 +395,6 @@ eshell-gather-process-output
 	    (setq lbeg lend)
 	    (set-buffer proc-buf))
 	  (set-buffer oldbuf))
-        (require 'esh-mode)
-        (declare-function eshell-update-markers "esh-mode" (pmark))
-        (defvar eshell-last-output-end)         ;Defined in esh-mode.el.
-	(eshell-update-markers eshell-last-output-end)
 	;; Simulate the effect of eshell-sentinel.
 	(eshell-close-handles
          (if (numberp exit-status) exit-status -1)
diff --git a/test/lisp/eshell/esh-proc-tests.el b/test/lisp/eshell/esh-proc-tests.el
index 8e02fbb5497..6395bdf4f71 100644
--- a/test/lisp/eshell/esh-proc-tests.el
+++ b/test/lisp/eshell/esh-proc-tests.el
@@ -191,6 +191,58 @@ esh-proc-test/pipeline-connection-type/last
      (unless (eq system-type 'windows-nt)
        "stdout\nstderr\n"))))
 
+\f
+;; Synchronous processes
+
+;; These tests check that synchronous subprocesses (only used on
+;; MS-DOS by default) work correctly.  To help them run on MS-DOS as
+;; well, we use the Emacs executable as our subprocess to test
+;; against; that way, users don't need to have GNU coreutils (or
+;; similar) installed.
+
+(defsubst esh-proc-test/emacs-command (command)
+  "Evaluate COMMAND in a new Emacs batch instance."
+  (mapconcat #'shell-quote-argument
+             `(,(expand-file-name invocation-name invocation-directory)
+               "-Q" "--batch" "--eval" ,(prin1-to-string command))
+             " "))
+
+(defvar esh-proc-test/emacs-echo
+  (esh-proc-test/emacs-command '(message "hello"))
+  "A command that prints \"hello\" to stdout using Emacs.")
+
+(defvar esh-proc-test/emacs-upcase
+  (esh-proc-test/emacs-command '(message "%s" (upcase (read-string ""))))
+  "A command that upcases the text from stdin using Emacs.")
+
+(ert-deftest esh-proc-test/synchronous-proc/simple/interactive ()
+  "Test that synchronous processes work in an interactive shell."
+  (let ((eshell-supports-asynchronous-processes nil))
+    (with-temp-eshell
+     (eshell-match-command-output esh-proc-test/emacs-echo
+                                  "\\`hello\n"))))
+
+(ert-deftest esh-proc-test/synchronous-proc/simple/command-result ()
+  "Test that synchronous processes work via `eshell-command-result'."
+  (let ((eshell-supports-asynchronous-processes nil))
+    (eshell-command-result-equal esh-proc-test/emacs-echo
+                                 "hello\n")))
+
+(ert-deftest esh-proc-test/synchronous-proc/pipeline/interactive ()
+  "Test that synchronous pipelines work in an interactive shell."
+  (let ((eshell-supports-asynchronous-processes nil))
+    (with-temp-eshell
+     (eshell-match-command-output (concat esh-proc-test/emacs-echo " | "
+                                          esh-proc-test/emacs-upcase)
+                                  "\\`HELLO\n"))))
+
+(ert-deftest esh-proc-test/synchronous-proc/pipeline/command-result ()
+  "Test that synchronous pipelines work via `eshell-command-result'."
+  (let ((eshell-supports-asynchronous-processes nil))
+    (eshell-command-result-equal (concat esh-proc-test/emacs-echo " | "
+                                          esh-proc-test/emacs-upcase)
+                                 "HELLO\n")))
+
 \f
 ;; Killing processes
 
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* bug#62578: 30.0.50; [PATCH] Add regression tests for synchronous processes in Eshell
  2023-04-02  0:58         ` Jim Porter
@ 2023-04-02  5:29           ` Eli Zaretskii
  2023-04-02  5:51             ` Jim Porter
  0 siblings, 1 reply; 9+ messages in thread
From: Eli Zaretskii @ 2023-04-02  5:29 UTC (permalink / raw)
  To: Jim Porter; +Cc: 62578

> Date: Sat, 1 Apr 2023 17:58:59 -0700
> From: Jim Porter <jporterbugs@gmail.com>
> Cc: 62578@debbugs.gnu.org
> 
> On 4/1/2023 12:40 AM, Jim Porter wrote:
> > My only concern there is making Emacs do something reasonable when 
> > piping data into it, but I can just try a few things out, and if it 
> > really doesn't work, the existing tests in my patch are hopefully still 
> > better than nothing.
> 
> Ok, that wasn't too hard. How does this look?

LGTM, thanks.

One minor nit: 'message' in a batch session prints to stderr, not to
stdout.





^ permalink raw reply	[flat|nested] 9+ messages in thread

* bug#62578: 30.0.50; [PATCH] Add regression tests for synchronous processes in Eshell
  2023-04-02  5:29           ` Eli Zaretskii
@ 2023-04-02  5:51             ` Jim Porter
  2023-04-02 21:25               ` Jim Porter
  0 siblings, 1 reply; 9+ messages in thread
From: Jim Porter @ 2023-04-02  5:51 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 62578

[-- Attachment #1: Type: text/plain, Size: 556 bytes --]

On 4/1/2023 10:29 PM, Eli Zaretskii wrote:
> One minor nit: 'message' in a batch session prints to stderr, not to
> stdout.

!! Thanks for catching that. That actually means there's a small bug (or 
a missing feature) in Eshell: it doesn't support redirecting stdout and 
stderr to separate places when using synchronous subprocesses.

I've added a note about this to esh-proc.el (fixing it would probably be 
tricky), and changed 'message' in the tests to be 'princ'. See attached; 
I'll probably merge this tomorrow unless you catch any remaining issues.

[-- Attachment #2: 0001-Add-tests-for-synchronous-processes-in-Eshell.patch --]
[-- Type: text/plain, Size: 6337 bytes --]

From ec1a5b3cda09945cb4b74273f9568267de5bd96f Mon Sep 17 00:00:00 2001
From: Jim Porter <jporterbugs@gmail.com>
Date: Fri, 31 Mar 2023 21:32:44 -0700
Subject: [PATCH] ; Add tests for synchronous processes in Eshell

Normally, Eshell only uses synchronous processes on MS-DOS, so this is
hard to test.  To get around this, let the tests explicitly request
synchronous processes.

* lisp/eshell/esh-proc.el (eshell-supports-asynchronous-processes):
New variable...
(eshell-gather-process-output): ... use it, and remove some incorrect
code updating Eshell's internal markers (the async code path doesn't
do this, so neither should the sync path).

* lisp/eshell/esh-cmd.el (eshell-execute-pipeline): Use
'eshell-supports-asynchronous-processes'.

* test/lisp/eshell/esh-proc-tests.el
(esh-proc-test/emacs-command): New function.
(esh-proc-test/emacs-echo, esh-proc-test/emacs-upcase): New variables.
(esh-proc-test/synchronous-proc/simple/interactive)
(esh-proc-test/synchronous-proc/simple/command-result)
(esh-proc-test/synchronous-proc/pipeline/interactive)
(esh-proc-test/synchronous-proc/pipeline/command-result): New tests.
---
 lisp/eshell/esh-cmd.el             |  2 +-
 lisp/eshell/esh-proc.el            | 11 ++++---
 test/lisp/eshell/esh-proc-tests.el | 53 ++++++++++++++++++++++++++++++
 3 files changed, 60 insertions(+), 6 deletions(-)

diff --git a/lisp/eshell/esh-cmd.el b/lisp/eshell/esh-cmd.el
index d5237ee1f04..b80596f2cad 100644
--- a/lisp/eshell/esh-cmd.el
+++ b/lisp/eshell/esh-cmd.el
@@ -892,7 +892,7 @@ eshell-execute-pipeline
      (set headproc nil)
      (set tailproc nil)
      (progn
-       ,(if (fboundp 'make-process)
+       ,(if eshell-supports-asynchronous-processes
 	    `(eshell-do-pipelines ,pipeline)
           `(let ((tail-handles (eshell-duplicate-handles
                                 eshell-current-handles)))
diff --git a/lisp/eshell/esh-proc.el b/lisp/eshell/esh-proc.el
index a86e7502795..00e0c8014e1 100644
--- a/lisp/eshell/esh-proc.el
+++ b/lisp/eshell/esh-proc.el
@@ -97,6 +97,9 @@ eshell-kill-hook
 
 ;;; Internal Variables:
 
+(defvar eshell-supports-asynchronous-processes (fboundp 'make-process)
+  "Non-nil if Eshell can create asynchronous processes.")
+
 (defvar eshell-current-subjob-p nil)
 
 (defvar eshell-process-list nil
@@ -296,7 +299,7 @@ eshell-gather-process-output
                 (coding-system-change-eol-conversion locale-coding-system
                                                      'unix))))
     (cond
-     ((fboundp 'make-process)
+     (eshell-supports-asynchronous-processes
       (unless (or ;; FIXME: It's not currently possible to use a
                   ;; stderr process for remote files.
                   (file-remote-p default-directory)
@@ -367,6 +370,8 @@ eshell-gather-process-output
 	(erase-buffer)
 	(set-buffer oldbuf)
 	(run-hook-with-args 'eshell-exec-hook command)
+        ;; XXX: This doesn't support sending stdout and stderr to
+        ;; separate places.
 	(setq exit-status
 	      (apply #'call-process-region
 		     (append (list eshell-last-sync-output-start (point)
@@ -392,10 +397,6 @@ eshell-gather-process-output
 	    (setq lbeg lend)
 	    (set-buffer proc-buf))
 	  (set-buffer oldbuf))
-        (require 'esh-mode)
-        (declare-function eshell-update-markers "esh-mode" (pmark))
-        (defvar eshell-last-output-end)         ;Defined in esh-mode.el.
-	(eshell-update-markers eshell-last-output-end)
 	;; Simulate the effect of eshell-sentinel.
 	(eshell-close-handles
          (if (numberp exit-status) exit-status -1)
diff --git a/test/lisp/eshell/esh-proc-tests.el b/test/lisp/eshell/esh-proc-tests.el
index 8e02fbb5497..fa20efa71e1 100644
--- a/test/lisp/eshell/esh-proc-tests.el
+++ b/test/lisp/eshell/esh-proc-tests.el
@@ -191,6 +191,59 @@ esh-proc-test/pipeline-connection-type/last
      (unless (eq system-type 'windows-nt)
        "stdout\nstderr\n"))))
 
+\f
+;; Synchronous processes
+
+;; These tests check that synchronous subprocesses (only used on
+;; MS-DOS by default) work correctly.  To help them run on MS-DOS as
+;; well, we use the Emacs executable as our subprocess to test
+;; against; that way, users don't need to have GNU coreutils (or
+;; similar) installed.
+
+(defsubst esh-proc-test/emacs-command (command)
+  "Evaluate COMMAND in a new Emacs batch instance."
+  (mapconcat #'shell-quote-argument
+             `(,(expand-file-name invocation-name invocation-directory)
+               "-Q" "--batch" "--eval" ,(prin1-to-string command))
+             " "))
+
+(defvar esh-proc-test/emacs-echo
+  (esh-proc-test/emacs-command '(princ "hello\n"))
+  "A command that prints \"hello\" to stdout using Emacs.")
+
+(defvar esh-proc-test/emacs-upcase
+  (esh-proc-test/emacs-command
+   '(princ (upcase (concat (read-string "") "\n"))))
+  "A command that upcases the text from stdin using Emacs.")
+
+(ert-deftest esh-proc-test/synchronous-proc/simple/interactive ()
+  "Test that synchronous processes work in an interactive shell."
+  (let ((eshell-supports-asynchronous-processes nil))
+    (with-temp-eshell
+     (eshell-match-command-output esh-proc-test/emacs-echo
+                                  "\\`hello\n"))))
+
+(ert-deftest esh-proc-test/synchronous-proc/simple/command-result ()
+  "Test that synchronous processes work via `eshell-command-result'."
+  (let ((eshell-supports-asynchronous-processes nil))
+    (eshell-command-result-equal esh-proc-test/emacs-echo
+                                 "hello\n")))
+
+(ert-deftest esh-proc-test/synchronous-proc/pipeline/interactive ()
+  "Test that synchronous pipelines work in an interactive shell."
+  (let ((eshell-supports-asynchronous-processes nil))
+    (with-temp-eshell
+     (eshell-match-command-output (concat esh-proc-test/emacs-echo " | "
+                                          esh-proc-test/emacs-upcase)
+                                  "\\`HELLO\n"))))
+
+(ert-deftest esh-proc-test/synchronous-proc/pipeline/command-result ()
+  "Test that synchronous pipelines work via `eshell-command-result'."
+  (let ((eshell-supports-asynchronous-processes nil))
+    (eshell-command-result-equal (concat esh-proc-test/emacs-echo " | "
+                                          esh-proc-test/emacs-upcase)
+                                 "HELLO\n")))
+
 \f
 ;; Killing processes
 
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* bug#62578: 30.0.50; [PATCH] Add regression tests for synchronous processes in Eshell
  2023-04-02  5:51             ` Jim Porter
@ 2023-04-02 21:25               ` Jim Porter
  0 siblings, 0 replies; 9+ messages in thread
From: Jim Porter @ 2023-04-02 21:25 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 62578-done

On 4/1/2023 10:51 PM, Jim Porter wrote:
> I'll probably merge this tomorrow unless you catch any remaining issues.

Merged as 00144fa287e. Closing this now.





^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2023-04-02 21:25 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-04-01  4:41 bug#62578: 30.0.50; [PATCH] Add regression tests for synchronous processes in Eshell Jim Porter
2023-04-01  6:07 ` Eli Zaretskii
2023-04-01  7:16   ` Jim Porter
2023-04-01  7:22     ` Eli Zaretskii
2023-04-01  7:40       ` Jim Porter
2023-04-02  0:58         ` Jim Porter
2023-04-02  5:29           ` Eli Zaretskii
2023-04-02  5:51             ` Jim Porter
2023-04-02 21:25               ` Jim Porter

Code repositories for project(s) associated with this external index

	https://git.savannah.gnu.org/cgit/emacs.git
	https://git.savannah.gnu.org/cgit/emacs/org-mode.git

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.